Re: data loss in 2.6.9-rc1-mm1

[Hugh Dickins]

On 27 Aug 2004, Ram Pai wrote:
> On Fri, 2004-08-27 at 06:56, Hugh Dickins wrote:
> > 
> > Hmm, 2.6.9-rc1-mm1 looks like not a release to trust your (page
> > size multiple) data to!  You should find the patch below fixes it
> > (and, I hope, the issue the erroneous patches were trying to fix).
> 
> Hmm.. now I fail to understand how this code works.
> 
> assuming page size is 4096, if the size of the file is 4096, is the
> end_index 0 or is it 1?

Before your change and after mine, 1; with your change, 0.

> I had this assumption:  
> 
> 	file size in bytes			end_index
> 	-----------------			---------
> 		1 to 4096			0
> 		4097 to 2*4096			1
> 		2*4096+1 to 3*4096		2
> 		...				..

Well, that's what you changed it to, when you patched from the original
		end_index = isize >> PAGE_CACHE_SHIFT;
to		end_index = (isize - 1) >> PAGE_CACHE_SHIFT;

But the "nr <= offset" check(s) relies on the original convention:
		0 to 4095			0
		4096 to 8191			1
		...				..

> or is the isize value reported by i_size_read(inode) one less than the
> size of the real file?

No!

> What am I missing?

You're expecting end_index to be the index of the last (possibly
incomplete) page of the file.  And that might be a reasonable way
of working it (though the special case of an empty file hints not).
But the nr,offset checks (I say checks because I added another like
the one further down, hopefully to fix the extra readahead issue)
require the original convention.  Just try it out with numbers.

Hugh

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/