Re: [RFC][PATCH 2/3] track numbers of pagetable pages

[Hugh Dickins]

On Tue, 26 Apr 2011, Matt Fleming wrote:

> [Added Hugh Dickins to the CC list]
> 
> Sorry it's taken me so long to reply Dave.
> 
> On Mon, 18 Apr 2011 08:02:04 -0700
> Dave Hansen <dave@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > On Sat, 2011-04-16 at 10:44 +0100, Matt Fleming wrote:
> > > >  static inline void pgtable_page_dtor(struct mm_struct *mm, struct page *page)
> > > >  {
> > > >  	pte_lock_deinit(page);
> > > > +	dec_mm_counter(mm, MM_PTEPAGES);
> > > >  	dec_zone_page_state(page, NR_PAGETABLE);
> > > >  }
> > > 
> > > I'm probably missing something really obvious but...
> > > 
> > > Is this safe in the non-USE_SPLIT_PTLOCKS case? If we're not using
> > > split-ptlocks then inc/dec_mm_counter() are only safe when done under
> > > mm->page_table_lock, right? But it looks to me like we can end up doing,
> > > 
> > >   __pte_alloc()
> > >       pte_alloc_one()
> > >           pgtable_page_ctor()
> > > 
> > > before acquiring mm->page_table_lock in __pte_alloc().
> > 
> > No, it's probably not safe.  We'll have to come up with something a bit
> > different in that case.  Either that, or just kill the non-atomic case.
> > Surely there's some percpu magic counter somewhere in the kernel that is
> > optimized for fast (unlocked?) updates and rare, slow reads.
> 
> It seems it was Hugh that added these atomics in f412ac08c986 ("[PATCH]
> mm: fix rss and mmlist locking").
> 
> Hugh, what was the reason that you left the old counters around (the
> ones protected by page_table_lock)? It seems to me that we could
> delete those and just have the single case that uses the atomic_t
> operations.

The only reason was to avoid adding costly atomic operations into a
configuration that had no need for them there: the page_table_lock
sufficed.

Certainly it would be simpler just to delete the non-atomic variant.

And I think it's fair to say that any configuration on which we're
measuring performance to that degree (rather than "does it boot fast?"
type measurements), would already be going the split ptlocks route.

> 
> Would anyone object to a patch that removed the non-atomic case?

Not I.

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