On Tue, Jan 08, 2013 at 10:46:38AM +0800, Liu Bo wrote:
> On Mon, Jan 07, 2013 at 05:20:50PM +0100, David Sterba wrote:
> > On Mon, Jan 07, 2013 at 11:53:08AM +0800, Liu Bo wrote:
> > Fix looks ok. I think this should be caught at runtime as well, the
> > number of ways how the lock start and end are passed is not small and it
> > need not be always possible to catch it from reading sources. The range
> > is inclusive, so it's 'lock end % 2 == 1' right? (in the bit
> > manipulating primitives).
> 
> Hmm, not always here, lockend = inode->i_size - 1, so lockend % 2 == 1 may not
> be true.

Yeah, that's the case that must be handled,

> But this check can worked in those places where we've rounded the range up to
> PAGE_SIZE :)

context:
2233         u64 lockend = i_size_read(inode);
...
2236         u64 len = i_size_read(inode);
...
2240         lockend = max_t(u64, root->sectorsize, lockend);
^^^^

lockend will be always at least root->sectorsize == PAGE_SIZE, so the simple
% 2 == 1 test should work generally. Am I missing something? Do we really lock
sub-PAGE_SIZE ranges?

2241         if (lockend <= lockstart)
2242                 lockend = lockstart + root->sectorsize;
2243
2244         len = lockend - lockstart + 1;
2245
2246         len = max_t(u64, len, root->sectorsize);
2247         if (inode->i_size == 0)
2248                 return -ENXIO;

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to