From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750895AbdCNHj1 (ORCPT ); Tue, 14 Mar 2017 03:39:27 -0400 Received: from LGEAMRELO12.lge.com ([156.147.23.52]:53397 "EHLO lgeamrelo12.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750796AbdCNHj0 (ORCPT ); Tue, 14 Mar 2017 03:39:26 -0400 X-Original-SENDERIP: 156.147.1.127 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 165.244.249.25 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 10.177.222.33 X-Original-MAILFROM: byungchul.park@lge.com Date: Tue, 14 Mar 2017 16:36:30 +0900 From: Byungchul Park To: Peter Zijlstra CC: , , , , , , , , , , Subject: Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Message-ID: <20170314073630.GG11100@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> MIME-Version: 1.0 In-Reply-To: <20170228181547.GM5680@worktop> User-Agent: Mutt/1.5.21 (2010-09-15) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB02/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/03/14 16:36:58, Serialize by Router on LGEKRMHUB02/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/03/14 16:36:58, Serialize complete at 2017/03/14 16:36:58 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 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. > > 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. Yes. I agree with that each syscall is irrelevant to others. But should we do that? Is it a problem if we don't distinguish between each syscall context in crossrelease check? IMHO, it's ok to perform commit if the target crosslock can be seen when releasing it. No? (As you know, in case of work queue, each work should be distinguished. See the comment in code.) If we have to do it.. do you mean to modify architecture code for syscall entry? Or is there architecture independent code where we can be aware of the entry? It would be appriciated if you answer them. > > (And we should not be returning to userspace with locks held anyway -- > lockdep already has a check for that).