From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936378AbcKKHWK (ORCPT ); Fri, 11 Nov 2016 02:22:10 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:37006 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932428AbcKKHWJ (ORCPT ); Fri, 11 Nov 2016 02:22:09 -0500 Date: Fri, 11 Nov 2016 08:21:56 +0100 From: Peter Zijlstra To: David Carrillo-Cisneros Cc: 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: <20161111072156.GS3568@worktop.programming.kicks-ass.net> References: <1477787923-61185-1-git-send-email-davidcc@google.com> <1477787923-61185-6-git-send-email-davidcc@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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'.