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

[Peter Zijlstra <peterz@infradead.org>]

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