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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 26 Apr 2011 12:26:09 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Matt Fleming <matt@...sole-pimps.org>
cc:	Dave Hansen <dave@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [RFC][PATCH 2/3] track numbers of pagetable pages

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@...ux.vnet.ibm.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/