From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752326AbdCAM3Y (ORCPT ); Wed, 1 Mar 2017 07:29:24 -0500 Received: from LGEAMRELO12.lge.com ([156.147.23.52]:42369 "EHLO lgeamrelo12.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101AbdCAM3W (ORCPT ); Wed, 1 Mar 2017 07:29:22 -0500 X-Original-SENDERIP: 156.147.1.121 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 165.244.249.23 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 10.177.222.33 X-Original-MAILFROM: byungchul.park@lge.com Date: Wed, 1 Mar 2017 21:27:23 +0900 From: Byungchul Park To: Peter Zijlstra CC: , , , , , , , , , , Subject: Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Message-ID: <20170301122723.GK11663@X58A-UD3R> References: <1484745459-2055-1-git-send-email-byungchul.park@lge.com> <1484745459-2055-7-git-send-email-byungchul.park@lge.com> <20170228181547.GM5680@worktop> <20170301072128.GH11663@X58A-UD3R> <20170301104328.GD6515@twins.programming.kicks-ass.net> MIME-Version: 1.0 In-Reply-To: <20170301104328.GD6515@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB04/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/03/01 21:27:37, Serialize by Router on LGEKRMHUB04/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/03/01 21:27:38, Serialize complete at 2017/03/01 21:27:38 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 01, 2017 at 11:43:28AM +0100, Peter Zijlstra wrote: > On Wed, Mar 01, 2017 at 04:21:28PM +0900, Byungchul Park wrote: > > On Tue, Feb 28, 2017 at 07:15:47PM +0100, Peter Zijlstra wrote: > > > On Wed, Jan 18, 2017 at 10:17:32PM +0900, Byungchul Park wrote: > > > > + /* > > > > + * Each work of workqueue might run in a different context, > > > > + * thanks to concurrency support of workqueue. So we have to > > > > + * distinguish each work to avoid false positive. > > > > + * > > > > + * TODO: We can also add dependencies between two acquisitions > > > > + * of different work_id, if they don't cause a sleep so make > > > > + * the worker stalled. > > > > + */ > > > > + unsigned int work_id; > > > > > > > +/* > > > > + * Crossrelease needs to distinguish each work of workqueues. > > > > + * Caller is supposed to be a worker. > > > > + */ > > > > +void crossrelease_work_start(void) > > > > +{ > > > > + if (current->xhlocks) > > > > + current->work_id++; > > > > +} > > > > > > So what you're trying to do with that 'work_id' thing is basically wipe > > > the entire history when we're at the bottom of a context. > > > > Sorry, but I do not understand what you are trying to say. > > > > What I was trying to do with the 'work_id' is to distinguish between > > different works, which will be used to check if history locks were held > > in the same context as a release one. > > The effect of changing work_id is that history disappears, yes? That is, > by changing it, all our hist_locks don't match the context anymore and > therefore we have no history. Right. Now I understood your words. > This is a useful operation. > > You would want to do this at points where you know there will not be any > dependencies on prior action, and typically at the same points we want > to not be holding any locks. > > Hence my term: 'bottom of a context', referring to an empty (held) lock > stack. Right. > I would say this needs to be done for all 'work-queue' like things, and Of course. > there are quite a few outside of the obvious ones, smpboot threads and > many other kthreads fall into this category. Where can I check those? > Similarly the return to userspace point that I already mentioned. > > I would propose something like: > > lockdep_assert_empty(); > > Or something similar, which would verify the lock stack is indeed empty > and wipe our entire hist_lock buffer when cross-release is enabled. Right. I should do that. > > > Which is a useful operation, but should arguably also be done on the > > > return to userspace path. Any historical lock from before the current > > > syscall is irrelevant. Let me think more. It looks not a simple problem. > > > > Sorry. Could you explain it more? > > Does the above make things clear? Perfect. Thank you very much.