On 07/30/2014 10:45 PM, Matthew Wilcox wrote:
<>

quoted

+ 	if (sector & (PAGE_SECTORS-1))
+ 		return -EINVAL;

Mmm.  PAGE_SECTORS is private to brd (and also private to bcache!) at
this point.  We've got a real mess of defines of SECTOR_SIZE, SECTORSIZE,
SECTOR_SHIFT and so on, dotted throughout various random include files.
I am not the river to flush those Augean stables today.

I'll go with this, from the dcssblk driver:

        if (sector % (PAGE_SIZE / 512))
                return -EINVAL;

Sigh, right, sure I did not mean to make that fight. Works as well

<>

quoted

Style: Need a space between declaration and code (have you check-patch)

That's a bullshit check.  I don't know why it's in checkpatch.

I did not invent the rules. But I do respect them. I think the merit
of sticking to some common style is much higher then any particular
style choice. Though this particular one I do like, because of the
C rule that forces all declarations before code, so it makes it easier
on the maintenance. In any way Maintainers are suppose to run checkpatch
before submission, some do ;-)

<>

quoted

quoted

+	if (size < 0)

	if(size < PAGE_SIZE), No?

No, absolutely not.  PAGE_SIZE is unsigned long, which (if I understand
my C integer promotions correctly) means that 'size' gets promoted to
an unsigned long, and we compare them unsigned, so errors will never be
caught by this check.

Good point I agree that you need a cast ie.

 	if(size < (long)PAGE_SIZE)

The reason I'm saying this is because of a bug I actually hit when
playing with partitioning and fdisk, it came out that the last partition's
size was not page aligned, and code that checked for (< 0) crashed because
prd returned the last two sectors of the partition, since your API is sector
based this can happen for you here, before you are memseting a PAGE_SIZE
you need to test there is space, No? 

Thanks
Boaz

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>