From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936459AbcKKHcy (ORCPT ); Fri, 11 Nov 2016 02:32:54 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35154 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934577AbcKKHcx (ORCPT ); Fri, 11 Nov 2016 02:32:53 -0500 Date: Fri, 11 Nov 2016 08:32:48 +0100 From: Ingo Molnar To: Peter Zijlstra Cc: David Carrillo-Cisneros , Thomas Gleixner , linux-kernel , "x86@kernel.org" , Ingo Molnar , Andi Kleen , Kan Liang , Vegard Nossum , Marcelo Tosatti , Nilay Vaish , Borislav Petkov , Vikas Shivappa , Ravi V Shankar , Fenghua Yu , Paul Turner , Stephane Eranian Subject: Re: [PATCH v3 05/46] perf/x86/intel/cmt: add per-package locks Message-ID: <20161111073248.GA23156@gmail.com> References: <1477787923-61185-1-git-send-email-davidcc@google.com> <1477787923-61185-6-git-send-email-davidcc@google.com> <20161111072156.GS3568@worktop.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161111072156.GS3568@worktop.programming.kicks-ass.net> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Peter Zijlstra 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