linux-kernel - Re: [PATCH v3 05/46] perf/x86/intel/cmt: add per-package locks

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Nov 2016 08:32:48 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     David Carrillo-Cisneros <davidcc@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>, Ingo Molnar <mingo@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kan Liang <kan.liang@...el.com>,
        Vegard Nossum <vegard.nossum@...il.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Nilay Vaish <nilayvaish@...il.com>,
        Borislav Petkov <bp@...e.de>,
        Vikas Shivappa <vikas.shivappa@...ux.intel.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Paul Turner <pjt@...gle.com>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v3 05/46] perf/x86/intel/cmt: add per-package locks


* Peter Zijlstra <peterz@...radead.org> wrote:

> On Thu, Nov 10, 2016 at 06:22:17PM -0800, David Carrillo-Cisneros wrote:
> > A possible alternative to nested spinlocks is to use a single rw_lock to
> > protect changes to the monr hierarchy.
> > 
> > This works because, when manipulating pmonrs, the monr hierarchy (monr's
> > parent and children) is read but never changed (reader path).
> > Changes to the monr hierarchy do change pmonrs (writer path).
> > 
> > It would be implemented like:
> > 
> >   // system-wide lock for monr hierarchy
> >   rwlock_t monr_hrchy_rwlock = __RW_LOCK_UNLOCKED(monr_hrchy_rwlock);
> > 
> > 
> > Reader Path: Access pmonrs in same pkgd. Used in pmonr state transitions
> > and when reading cgroups (read subtree of pmonrs in same package):
> > 
> > v3 (old):
> > 
> >   raw_spin_lock(&pkgd->lock);
> >   // read monr data, read/write pmonr data
> >   raw_spin_unlock(&pkgd->lock);
> > 
> > next version:
> > 
> >   read_lock(&monr_hrchy_rwlock);
> >   raw_spin_lock(&pkgd->lock);
> >   // read monr data, read/write pmonr data
> >   raw_spin_unlock(&pkgd->lock);
> >   read_unlock(&monr_hrchy_rwlock);
> > 
> > 
> > Writer Path: Modifies monr hierarchy (add/remove monr in
> > creation/destruction of events and start/stop monitoring
> > of cgroups):
> > 
> > v3 (old):
> > 
> >   monr_hrchy_acquire_locks(...); // acquires all pkg->lock
> >   // read/write monr data, potentially read/write pmonr data
> >   monr_hrchy_release_locks(...); // release all pkg->lock
> > 
> > v4:
> > 
> >   write_lock(&monr_hrchy_rwlock);
> >   // read/write monr data, potentially read/write pmonr data
> >   write_unlock(&monr_hrchy_rwlock);
> > 
> > 
> > The write-path should occur infrequently. The reader-path
> > is slower due to the additional read_lock(&monr_hrchy_rwlock).
> > The number of locks to acquire remains constant on the number of
> > packages in both paths. There would be no need to ever nest pkgd->lock's.
> > 
> > For v3, I discarded this idea because, due to the additional
> > read_lock(&monr_hrchy_rwlock), the pmonr state transitions that may
> > occur in a context switch would be slower than in the current version.
> > But it may be ultimately better considering the scalability issues
> > that you mention.
> 
> Well, its very hard to suggest alternatives, because there simply isn't
> anything of content here. This patch just adds locks, and the next few
> patches don't describe much either.
> 
> Why can't this be RCU?
> 
> Also, "monr" is a horribly 'word'.

And 'hrchy' is a hrbbl wrd as wll!

Thanks,

	Ingo