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:21:56 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     David Carrillo-Cisneros <davidcc@...gle.com>
Cc:     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

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'.