lkml.org 
[lkml]   [2008]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 2.6.26-rc5-mm2
    On Tue, 10 Jun 2008, Lee Schermerhorn wrote:
    > On Tue, 2008-06-10 at 17:28 +1000, Nick Piggin wrote:
    > > mm/memory.c:do_wp_page
    > > //TODO: is this safe? do_anonymous_page() does it this way.
    > >
    > > That's a bit disheartening. Surely a question like that has to
    > > be answered definitively? (hopefully whatever is doing the
    > > asking won't get merged until answered)
    >
    > I put those C++ TODO comments in there specifically to raise their
    > visibility in hopes that someone [like you :)] would notice and maybe
    > have an answer to the question. I noted the issue in the change log as
    > well--i.e., that I had moved set_pte_at() to after the lru_cache_add and
    > 'new_rmap. The existing order may be that way for a reason, but it's
    > not clear [to me] what that reason is. As I noted, do_anonymous_page()
    > sets the pte after the lru_add and new_rmap.
    >
    > I agree, these questions need to be answered and the TODO's resolved
    > before merging. Any thoughts as to the ordering?

    The ordering of lru_cache_add*, page_add_*_rmap and set_pte_at does
    not matter (but update_mmu_cache must come after set_pte_at not before).

    Even if the page table lock were not held across them (it is), I think
    their ordering would not matter much (just benign races); though it's
    always worth keeping in mind that once you've done the lru_cache_add,
    that page is now visible to vmscan.c.

    But I'm all in favour of you imposing consistency there (as part of
    a wider patch? perhaps not; and do_swap_page does now look out of step).
    It can sometimes help when inserting debug checks e.g. on page_mapcount.

    I think you'll find the lru_cache_add_active_or_noreclaim could
    actually be moved into page_add_new_rmap - I found that helpful when
    working on eliminating the PageSwapCache flag (work now grown out of
    date, I'm afraid), to know that the page was not publicly visible
    until I did lru_cache_add_active at the end of page_add_new_rmap.

    Hugh


    \
     
     \ /
      Last update: 2008-06-10 18:53    [W:0.022 / U:30.944 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site