lkml.org 
[lkml]   [2000]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: page->mapping == 0


    On Sun, 29 Oct 2000, Alexander Viro wrote:
    >
    > One possible way is to access page->mapping _only_ under the page lock
    > and in cases when we call ->i_mapping->a_ops->foo check the ->mapping
    > before the method call.

    I'm leaning towards this for a 2.4.x solution.

    As far as I can tell, page->mapping is _already_ only accessed and
    modified with the page lock held. It's just that we don't test it for
    NULL, in case an earlier lock holder decided to clear it.

    (No, I didn't look through all the users, but at least conceptually it
    _should_ be true that we only look at "mapping" with the lock held: it's
    mainly used for pagein, and pageout, buth with the lock held for other
    reasons already. Certainly all the places where we have had bug-reports
    have been of this type).

    Making it policy that we have to re-test page->mapping after aquireing the
    page lock might be the simplest fix for 2.4.x. It still means that we
    might end up allowing people to have a "bad" page in the VM space due to
    the "test->insert" race condition, but it woul dmake that event pretty
    much a harmless bug (and thus move it to the "beauty wart - to be fixed
    later" category).

    And the places where we get the page lock and use page->mapping are not
    that many, I think.

    (And notice how we actually _have_ this approach already in
    do_buffer_fdatasync(), for similar reasons - we use the "re-test the
    page->buffers" thing there. Of course, there we do it because the clearing
    of page->buffers is easier to see, and can happen as a result of memory
    pressure, and not just truncate()).

    So, for example, in __find_lock_page() we should re-test the mapping after
    we aquired the page lock. Which is fairly easy, just add something like

    /* Race: did the mapping go away before we got the page lock? */
    if (page && page->mapping != mapping) {
    page_cache_release(page);
    goto repeat;
    }

    to the end of __find_lock_page(). Add similar logic to
    do_generic_file_read(), filemap_nopage() and filemap_sync_pte() and
    read_cache_page(), and you're pretty much done.

    Linus

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

    \
     
     \ /
      Last update: 2005-03-22 12:45    [W:0.022 / U:3.768 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site