lkml.org 
[lkml]   [2004]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: data loss in 2.6.9-rc1-mm1
On Sat, 28 Aug 2004, Nick Piggin wrote:
>
> Ahh, yep - Hugh just forgot to also move the "nr" calculation
> into the ->readpage path, so it hits twice on the fast path.

Yes, your patch is better than mine.

> What's more, it looks like mine handles the corner case of reading off the
> end of a non-PAGE_SIZE file (but within the same page). I think yours will
> drop through and do the ->readpage, while mine doesn't...?

It's a common case, not a corner case: a short read to end of file,
then app does another read which returns 0 for the end of file: that
wouldn't normally fall through to readpage in Ram's case, but would
do unnecessary page_cache_readahead (won't do much) and find_get_page.

> I agree. We'll leave it to someone else to decide, then ;)

I vote for Nick's patch.

I do have one reservation on do_generic_mapping_read,
common to all these versions, unrelated to the current issue.

I'm surprised to notice that you're careful to re-i_size_read
after readpage, but otherwise rely on the initial i_size_read.
We could go around this loop many many times, faulting user pages
in actor, rescheduling away: the old (e.g. 2.4 or 2.6.0) code was
deficient after readpage, but at least it reassessed i_size each
time around the loop. I guess if the file is truncated meanwhile,
the common case would be for a find_get_page to fail, and then the
situation be corrected after readpage; perhaps it's more likely to
show up as read returning too little on a large file being steadily
appended. Maybe you already ruled these cases out as not worth the
overhead of handling, but it does surprise me that the old code was
safer in this respect.

Hugh

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

\
 
 \ /
  Last update: 2005-03-22 14:05    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site