lkml.org 
[lkml]   [2006]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC] Event counters [1/3]: Basic counter functionality
    From
    Date
    On Sat, 2005-12-31 at 18:26 -0200, Marcelo Tosatti wrote:
    > Hi Nick!
    >

    Hey Marcelo!

    > On Sat, Dec 31, 2005 at 06:54:25PM +1100, Nick Piggin wrote:
    > >
    > > Hi Guys,
    > >
    > > I've been waiting for some mm/ patches to clear from -mm before commenting
    > > too much... however I see that this patch is actually against -mm itself,
    > > with my __mod_page_state stuff in it... that makes the page state accounting
    > > much lighter weight AND is not racy.
    >
    > It is racy with reference to preempt (please refer to the race condition
    > described above):
    >
    > diff -puN mm/rmap.c~mm-page_state-opt mm/rmap.c
    > --- devel/mm/rmap.c~mm-page_state-opt 2005-12-13 22:25:01.000000000 -0800
    > +++ devel-akpm/mm/rmap.c 2005-12-13 22:25:01.000000000 -0800
    > @@ -451,7 +451,11 @@ static void __page_set_anon_rmap(struct
    >
    > page->index = linear_page_index(vma, address);
    >
    > - inc_page_state(nr_mapped);
    > + /*
    > + * nr_mapped state can be updated without turning off
    > + * interrupts because it is not modified via interrupt.
    > + */
    > + __inc_page_state(nr_mapped);
    > }
    >
    > And since "nr_mapped" is not a counter for debugging purposes only, you
    > can't be lazy with reference to its consistency.
    >
    > I would argue that you need a preempt save version for this important
    > counters, surrounded by preempt_disable/preempt_enable (which vanish
    > if one selects !CONFIG_PREEMPT).
    >

    I think it should not be racy because the function should always be
    called with the page table lock held, which disables preempt. I guess
    the comment should be explicit about that as well.

    There were some runtime warnings that come up when this patch first
    went into -mm because of a silly typo, however that should now be
    resolved too.

    > As Christoph notes, debugging counter consistency can be lazy, not even
    > requiring correct preempt locking (hum, this is debatable, needs careful
    > verification).
    >
    > > So I'm not exactly sure why such a patch as this is wanted now? Are there
    > > any more xxx_page_state hotspots? (I admit to only looking at page faults,
    > > page allocator, and page reclaim).
    >
    > A consolidation of the good parts of both would be interesting.
    >
    > I don't see much point in Christoph's naming change to "event_counter",
    > why are you doing that?
    >
    > And follows an addition to your's mm-page_state-opt-docs.patch. Still
    > need to verify "nr_dirty" and "nr_unstable".
    >
    > Happy new year!
    >

    Thanks, happy new year to you too!

    --
    SUSE Labs, Novell Inc.



    Send instant messages to your online friends http://au.messenger.yahoo.com

    -
    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: 2006-01-03 06:03    [W:0.031 / U:1.224 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site