Re: data loss in 2.6.9-rc1-mm1
From: Nick Piggin (nickpiggin_at_yahoo.com.au)
Date: 08/29/04
- Previous message: Peter Williams: "Re: Scheduler fairness problem on 2.6 series (Attn: Nick Piggin and others)"
- In reply to: Hugh Dickins: "Re: data loss in 2.6.9-rc1-mm1"
- Next in thread: Ram Pai: "Re: data loss in 2.6.9-rc1-mm1"
- Reply: Ram Pai: "Re: data loss in 2.6.9-rc1-mm1"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Sun, 29 Aug 2004 11:30:08 +1000 To: Hugh Dickins <hugh@veritas.com>
Hugh Dickins wrote:
> 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.
>
Ahh, yeah I guess that would be the more common case. I was just
thinking of just randomly reading past the end of the file - in
that case it *would* fall through to ->readpage if the page wasn't
in cache.
But anyway, I think we agree my (our) version should cover that.
>
>>I agree. We'll leave it to someone else to decide, then ;)
>
>
> I vote for Nick's patch.
>
OK - maybe that can go for a spin in the next -mm. Andrew did you
get it?
> 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.
>
Yeah I guess it is a case of doing the minimum that is really
needed.
Although considering page_cache_readahead call can do an i_size_read,
it might be nicer to probably change the interface to have it passed down
instead
-
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/
- Previous message: Peter Williams: "Re: Scheduler fairness problem on 2.6 series (Attn: Nick Piggin and others)"
- In reply to: Hugh Dickins: "Re: data loss in 2.6.9-rc1-mm1"
- Next in thread: Ram Pai: "Re: data loss in 2.6.9-rc1-mm1"
- Reply: Ram Pai: "Re: data loss in 2.6.9-rc1-mm1"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
