From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0298C47073 for ; Tue, 2 Jan 2024 09:15:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 579026B01A5; Tue, 2 Jan 2024 04:15:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 502FF6B01A6; Tue, 2 Jan 2024 04:15:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3A3976B01A7; Tue, 2 Jan 2024 04:15:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 268B66B01A5 for ; Tue, 2 Jan 2024 04:15:09 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id D91DB8075E for ; Tue, 2 Jan 2024 09:15:08 +0000 (UTC) X-FDA: 81633811896.20.298571E Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf25.hostedemail.com (Postfix) with ESMTP id 7B6FBA0014 for ; Tue, 2 Jan 2024 09:15:06 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=qbjbK+L5; spf=none (imf25.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1704186907; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=tlEfMQFt5gK7xTvm4OYyFW6oyrGcWRnYRgh3+dnfWXQ=; b=j26ZAAzD07mJD5EJVLJSrnaCtsloUo9r1BF4IJ00Wjdc3+LrJSVE4EqkXZXk3H+8YYpC+0 BmuCuBuaRXzV24dMggp5l1McsVXfOcA3u/XXUqvq4r9pcU3EEEqKTwrujkgXweIFVS9UKl o/NvxmG6T/BHDitxVrcWEXDzzZZ9iAg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704186907; a=rsa-sha256; cv=none; b=PHKjP2K15Q5ZmyOPVJDg0ZbTkkCGsU6HuFzOEXMKXVvJQe6iA+0ZjOys286oV01iJ8lGnO vADeflcNEgIgK5wlRFxsPFQ4hFtVyTN57c6/UpLjyMxUnMBGNkSbjYhKztf9+dXIPjGgtL FyWEw52RVxDQvJwmzooUa1IvTPCcsxQ= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=qbjbK+L5; spf=none (imf25.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=tlEfMQFt5gK7xTvm4OYyFW6oyrGcWRnYRgh3+dnfWXQ=; b=qbjbK+L5aYe/lDZscjtenI5cjW dZL/n71TVQj2zphfxq3RZ+cNggYEWOSaYIL6eYvVx5t5XbeQtXC5MfvkPEbeyWhD7ffyRT3M3U/Vd VVSFZvNRo70tslTpjdD38Sg+/jUCGJ+dlQkKzla59Tw98eiFplHqpLYGh8x1cESshIjA8i2C7Sn5M muL2EGVBlVq2u1U2wLBjpcL4+E4cCtxzD6oY/ijyD7c2z9wI6Hy3ySQPY+FZTT4GhfgPtObTsn+Mf 4GMgFwlfji7YX5rkT/+U0nnZrO6GArNuSqp3UnB6eP/3Dq/eakRRSpeSc0fIN9D8KevfYWMzxiVW4 BH4OtoIw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1rKaqw-009pns-In; Tue, 02 Jan 2024 09:14:26 +0000 Date: Tue, 2 Jan 2024 09:14:26 +0000 From: Matthew Wilcox To: "Aiqun Yu (Maria)" Cc: "Eric W. Biederman" , Hillf Danton , kernel@quicinc.com, quic_pkondeti@quicinc.com, keescook@chromium.or, viro@zeniv.linux.org.uk, brauner@kernel.org, oleg@redhat.com, dhowells@redhat.com, jarkko@kernel.org, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, keyrings@vger.kernel.org, linux-security-module@vger.kernel.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH] kernel: Introduce a write lock/unlock wrapper for tasklist_lock Message-ID: References: <20231213101745.4526-1-quic_aiquny@quicinc.com> <87o7eu7ybq.fsf@email.froward.int.ebiederm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 7B6FBA0014 X-Rspam-User: X-Stat-Signature: tcc91fwz45ykwb15pngna8zcrs79x4cp X-Rspamd-Server: rspam03 X-HE-Tag: 1704186906-847790 X-HE-Meta: U2FsdGVkX1/UU/i5vhGgy/kFrtnMQtjOCzkJfLI3rvGvkxFzbyEJiHL/WgyzYS0PDC9TnLBGnPXsp0X38AXn0mBCb8puMA1xQEALQ76UbEF4JN9C2UQgP/CvwUGfre6jB8K76X8A+G7AlfrulHBtGYzt2HsPcPh0qAnBlcwtUgITxVHCfWtq2zjC2X6+l88KpyBOr1k1WgsfR9OGqQjdtYs6rKt0rKj3CRshv3trkEfJV/07P/P6hdHidBpYxAewsreC+CFFr79+m/fVyFRyyBKxVGmYJcitwp//f8iH2vPYmMyFIF+OP0xlHPqgGKAojMqrPAYprUmdAuHVCl0GHB/GV6jEXAOg5jfmuI9sx2HgABxxQxbdW2v0LTBXCeiKaKaAE1hUG6URkcLM7uAwSQsLHexKlPuKT2H9CDdz2DoTlLqkR0KAvQOgLdbyiJBfaW20rfr7JJkIyNnKqJRLSEePrvvnz2Wky/b0aoN0a6n7V7+iTHYklcs+IyYadM6gIdvbhJw/ca2i9kcx6145ULpQnSLa+V1D7rg3j7sjAp1NS2MUrY0oGa8+jokJBlKEnKZxAzJPBslouZ8Uz8UQDUnTf8d349Kzycpchv5TnyJKdRtYRAbCLBwnjiEkovHqVNalQv0DW7zut1ytmc2x5TXcopfAVEhxbU3Q5pRtF0rsDo21oDOiVldAYaF4NstKJdSHjEvXnbMgatf0OmgC29bGWMUuP8P8zTAedjc+i889DxXs4qNJdfb7Ray225GLJ4FqAyNc15orNC9ijaw4xQVeWzZwXq6jal8xTu0eSf6V0DqJ5oW9K1pqGACwZK9d4THGi7Npv/xWc+CGVO+NMD+EmHhf4RHtpw1aC7wXgFDtaEKoJkz20wq56Zi6bp0Icwg0RIb6fi42YapTW+o7l3Odu4w+xMzPTXfCUAHBw7mArKnX93ExHtd+MFycpJq5amfAsterD/xCiLSi9Ep 6MCWktDV MTMhv+RWRdfokA1S3C62XB1muJB5wrmViACgnRxIPs5fJAB7JgyuH+5ovwbnLUUdGXmzEGDRoPhjW9mdyGdPFoGyRMlxwCuxKsU3rlL0a9u9bnhyL13jwLoAXsTeS4nGqIWsHf+nnUEeayjJiY5VDFIjE0DKrGNkdUiyfi5qThXR1liFDupZRMsXg3Eh7xjn2TpdmME9wijVovPWUr0vxz76E1kJCj+hmyRk1qiqGRXQ/2YoO90tUGPowdA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Jan 02, 2024 at 10:19:47AM +0800, Aiqun Yu (Maria) wrote: > On 12/29/2023 6:20 AM, Matthew Wilcox wrote: > > On Wed, Dec 13, 2023 at 12:27:05PM -0600, Eric W. Biederman wrote: > > > Matthew Wilcox writes: > > > > I think the right way to fix this is to pass a boolean flag to > > > > queued_write_lock_slowpath() to let it know whether it can re-enable > > > > interrupts while checking whether _QW_WAITING is set. > > > > > > Yes. It seems to make sense to distinguish between write_lock_irq and > > > write_lock_irqsave and fix this for all of write_lock_irq. > > > > I wasn't planning on doing anything here, but Hillf kind of pushed me into > > it. I think it needs to be something like this. Compile tested only. > > If it ends up getting used, > Happy new year! Thank you! I know your new year is a few weeks away still ;-) > > -void __lockfunc queued_write_lock_slowpath(struct qrwlock *lock) > > +void __lockfunc queued_write_lock_slowpath(struct qrwlock *lock, bool irq) > > { > > int cnts; > > @@ -82,7 +83,11 @@ void __lockfunc queued_write_lock_slowpath(struct qrwlock *lock) > Also a new state showed up after the current design: > 1. locked flag with _QW_WAITING, while irq enabled. > 2. And this state will be only in interrupt context. > 3. lock->wait_lock is hold by the write waiter. > So per my understanding, a different behavior also needed to be done in > queued_write_lock_slowpath: > when (unlikely(in_interrupt())) , get the lock directly. I don't think so. Remember that write_lock_irq() can only be called in process context, and when interrupts are enabled. > So needed to be done in release path. This is to address Hillf's concern on > possibility of deadlock. Hillf's concern is invalid. > > /* When no more readers or writers, set the locked flag */ > > do { > > + if (irq) > > + local_irq_enable(); > I think write_lock_irqsave also needs to be take account. So > loal_irq_save(flags) should be take into account here. If we did want to support the same kind of spinning with interrupts enabled for write_lock_irqsave(), we'd want to pass the flags in and do local_irq_restore(), but I don't know how we'd support write_lock_irq() if we did that -- can we rely on passing in 0 for flags meaning "reenable" on all architectures? And ~0 meaning "don't reenable" on all architectures? That all seems complicated, so I didn't do that.