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    [W:0.020 / U:33.092 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site