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 EE259C3DA6E for ; Thu, 28 Dec 2023 22:20:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2901E6B0104; Thu, 28 Dec 2023 17:20:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 23F886B0105; Thu, 28 Dec 2023 17:20:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 107376B0106; Thu, 28 Dec 2023 17:20:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id F13616B0104 for ; Thu, 28 Dec 2023 17:20:41 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id C634514020E for ; Thu, 28 Dec 2023 22:20:41 +0000 (UTC) X-FDA: 81617647482.07.F3DF865 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf24.hostedemail.com (Postfix) with ESMTP id 88F1918000D for ; Thu, 28 Dec 2023 22:20:38 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=KHXXKQZp; spf=none (imf24.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=1703802040; 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=kQ+uKuyxhmcjSMG7LN+mZVJSxMAB8MaVFuhiW7JiPfc=; b=VfW5Y9kXGThoI9SfyZVohGIKNnX1sXwrIp2v3t1KXjoiqojopCxZcyO7dlI9i7FGm0ae/V 3ac/zeV3NrSCisQx49Z08Km15E4xI5/mQnQDHZIdpuhFbQdy6sKytc5rVo3fA8KcH5gKlW hKpeMYztzVvPBndDtqw/DREej/fJTLU= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=KHXXKQZp; spf=none (imf24.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-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1703802040; a=rsa-sha256; cv=none; b=NbmNBFq6LiE3gCoY8duzg5k+5f6InEm+1J2OenhJQMDZzMot06thuZF90w38CfYUibjH56 gZoo6i8j+hAmgswE2KGXe9XV0Ywd/sjfdN/iG9+/oBCNTZTz34jI54r/3lSUwAFIv1YWcY k9wSx6OJDtPBbMfTarzCUc7FKbxY3tQ= 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=kQ+uKuyxhmcjSMG7LN+mZVJSxMAB8MaVFuhiW7JiPfc=; b=KHXXKQZpOt9etWodUQCdGaQE2b 9KJWxMzCTUUh3sxe3Y75CnkuzFp+l8AW3f2sSmKRCwanS720bp2Un6aHnAfX/z9/JtIRsMh/ypcC5 HGPs9M3qxsME+51W0rachnSOIb1ljosSCeax34cSZhyDvMN9xXfGsgmfoJ07Tp04fdpCh7WOs94G3 lDJy6Gv/cJRx890dse2nws3lBy7fN4fiz2GxWU+PvZ9Z1byTi2FrmfL2YqA9W/iWUnz91UC/s61J2 rSp8EQVu0uwUogN9IGCIUxZNxIetjAEg0lVuPElVdTrxavbxOqCYBBf5UhihqmCqxUVyPc7jTGZDF 27uLB1Mg==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1rIyjT-005dpL-R0; Thu, 28 Dec 2023 22:20:03 +0000 Date: Thu, 28 Dec 2023 22:20:03 +0000 From: Matthew Wilcox To: "Eric W. Biederman" Cc: Maria Yu , 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: <87o7eu7ybq.fsf@email.froward.int.ebiederm.org> X-Rspamd-Queue-Id: 88F1918000D X-Rspam-User: X-Stat-Signature: 19sgrrf8p45d3ofbxkdjadk5daup3e4z X-Rspamd-Server: rspam01 X-HE-Tag: 1703802038-281622 X-HE-Meta: U2FsdGVkX1/4fLz9rv78B4iQ1O9zDj+NA+syAkKfVcQ69ZUXedTryRfDIOlRexx53KMbea4xIB+O2258/81e4zRYKZ4nW3HVpmqa8fAm+i/AA3PBdNVPrR3z+JrxTWhTS8DwFM+okkXZXnoRG2OTM6U/OVRXRpFZq/zdBebShkYwe7m27wK7uQiPfOByZWEexPD++xjzOZvVLCN7/baDgNRHdrtdTKdqDuBLNbEtLy9cd2RHwWk5x/HLtyCMY81M9WIa7Uv97/G8466tGo8LVHjHIicxjYLkeeexnSebU5nIwOpYooAnln/0Q1qKXO29nGCebVKlTCa4S4/IMs2YS2j9rlxReK7pYtnxoE5u8FrPZCGgmQBg59YWnbqvtdmOAT3WM1LFoBcyVNTp220VhJ0v+4KJk8qZ2drrlVuVbWvEuPJCFFWHyHVWPRLDFEahWd/VFJayPCpWAvL1/NDpbyXxy6dO/DhNqOeSN0URC8e+ZqDXB5O6bQuTHVYpZGAM4je2dCDCG6vlUIGL4Uc3L1wzJEzvmxEhgRWG96BlmS5Yx4gFZNGjzhp+c/Iwqv3H6QG36AFZXe47jffC2FmOd49Ef5gEt5NwLc/4bsTneMHNiWL6AsK43a3KxjHCtQfTMcDN8pHcdrwn+8kvwQj1SmC5qUlc/0ROQDbGhIIMz105RM1QAfJiSqJN0G0OOMtB/ue/MftuCAU5IDZU1IJitqQRtCKMTNmSJ4BuGJ8NAXsBPvoufB7tdFzTfwsk+n8dxfSEV3Ok6wyC4CNcO87svl/x5ViPeDoe2JfIqcaBtPJBqFrj5XzR40+DvF53nPmJD4USBWU5ENeVTelnNXZRdE6aiJnzna51HB35teRkEHbsoRWvCCR+vwygSsPvRiQlhOqtOLSDXEPaDwI6ZCtxwjC9hEw46zukruwlYRJW/Mudejdz3zbQRXcHbeswbgql2LwILy53LWLwfIl6Sc8 7DIZnMnC 39LKz3fy+juggAlrN9c2Oqf758HC+0XtjHwMYdcrfUx2S7T+DERnajagoWJmpvEqa0/xEu40gb2+FmaZw8e7J4+NzcbR5fvFD8BCQ1UqMThknJqQk1jhyniSGzuucIbJWJE6Dj8T7FFUNLXFWVnlzd4Qm9AN7sskbhxTXaHWi3+fZjGOHnihGYhHW81d9yd2G63ojLt8bFdNf2az/uObYQg7skBP2KHxF9ouDuGeBLldZbBuu7XXsxjCGbw== 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 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, Signed-off-by: Matthew Wilcox (Oracle) diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h index 75b8f4601b28..1152e080c719 100644 --- a/include/asm-generic/qrwlock.h +++ b/include/asm-generic/qrwlock.h @@ -33,8 +33,8 @@ /* * External function declarations */ -extern void queued_read_lock_slowpath(struct qrwlock *lock); -extern void queued_write_lock_slowpath(struct qrwlock *lock); +void queued_read_lock_slowpath(struct qrwlock *lock); +void queued_write_lock_slowpath(struct qrwlock *lock, bool irq); /** * queued_read_trylock - try to acquire read lock of a queued rwlock @@ -98,7 +98,21 @@ static inline void queued_write_lock(struct qrwlock *lock) if (likely(atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED))) return; - queued_write_lock_slowpath(lock); + queued_write_lock_slowpath(lock, false); +} + +/** + * queued_write_lock_irq - acquire write lock of a queued rwlock + * @lock : Pointer to queued rwlock structure + */ +static inline void queued_write_lock_irq(struct qrwlock *lock) +{ + int cnts = 0; + /* Optimize for the unfair lock case where the fair flag is 0. */ + if (likely(atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED))) + return; + + queued_write_lock_slowpath(lock, true); } /** @@ -138,6 +152,7 @@ static inline int queued_rwlock_is_contended(struct qrwlock *lock) */ #define arch_read_lock(l) queued_read_lock(l) #define arch_write_lock(l) queued_write_lock(l) +#define arch_write_lock_irq(l) queued_write_lock_irq(l) #define arch_read_trylock(l) queued_read_trylock(l) #define arch_write_trylock(l) queued_write_trylock(l) #define arch_read_unlock(l) queued_read_unlock(l) diff --git a/include/linux/rwlock.h b/include/linux/rwlock.h index c0ef596f340b..897010b6ba0a 100644 --- a/include/linux/rwlock.h +++ b/include/linux/rwlock.h @@ -33,6 +33,7 @@ do { \ extern int do_raw_read_trylock(rwlock_t *lock); extern void do_raw_read_unlock(rwlock_t *lock) __releases(lock); extern void do_raw_write_lock(rwlock_t *lock) __acquires(lock); + extern void do_raw_write_lock_irq(rwlock_t *lock) __acquires(lock); extern int do_raw_write_trylock(rwlock_t *lock); extern void do_raw_write_unlock(rwlock_t *lock) __releases(lock); #else @@ -40,6 +41,7 @@ do { \ # define do_raw_read_trylock(rwlock) arch_read_trylock(&(rwlock)->raw_lock) # define do_raw_read_unlock(rwlock) do {arch_read_unlock(&(rwlock)->raw_lock); __release(lock); } while (0) # define do_raw_write_lock(rwlock) do {__acquire(lock); arch_write_lock(&(rwlock)->raw_lock); } while (0) +# define do_raw_write_lock_irq(rwlock) do {__acquire(lock); arch_write_lock_irq(&(rwlock)->raw_lock); } while (0) # define do_raw_write_trylock(rwlock) arch_write_trylock(&(rwlock)->raw_lock) # define do_raw_write_unlock(rwlock) do {arch_write_unlock(&(rwlock)->raw_lock); __release(lock); } while (0) #endif diff --git a/include/linux/rwlock_api_smp.h b/include/linux/rwlock_api_smp.h index dceb0a59b692..6257976dfb72 100644 --- a/include/linux/rwlock_api_smp.h +++ b/include/linux/rwlock_api_smp.h @@ -193,7 +193,7 @@ static inline void __raw_write_lock_irq(rwlock_t *lock) local_irq_disable(); preempt_disable(); rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_); - LOCK_CONTENDED(lock, do_raw_write_trylock, do_raw_write_lock); + LOCK_CONTENDED(lock, do_raw_write_trylock, do_raw_write_lock_irq); } static inline void __raw_write_lock_bh(rwlock_t *lock) diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c index d2ef312a8611..6c644a71b01d 100644 --- a/kernel/locking/qrwlock.c +++ b/kernel/locking/qrwlock.c @@ -61,9 +61,10 @@ EXPORT_SYMBOL(queued_read_lock_slowpath); /** * queued_write_lock_slowpath - acquire write lock of a queued rwlock - * @lock : Pointer to queued rwlock structure + * @lock: Pointer to queued rwlock structure + * @irq: True if we can enable interrupts while spinning */ -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) /* When no more readers or writers, set the locked flag */ do { + if (irq) + local_irq_enable(); cnts = atomic_cond_read_relaxed(&lock->cnts, VAL == _QW_WAITING); + if (irq) + local_irq_disable(); } while (!atomic_try_cmpxchg_acquire(&lock->cnts, &cnts, _QW_LOCKED)); unlock: arch_spin_unlock(&lock->wait_lock); diff --git a/kernel/locking/spinlock_debug.c b/kernel/locking/spinlock_debug.c index 87b03d2e41db..bf94551d7435 100644 --- a/kernel/locking/spinlock_debug.c +++ b/kernel/locking/spinlock_debug.c @@ -212,6 +212,13 @@ void do_raw_write_lock(rwlock_t *lock) debug_write_lock_after(lock); } +void do_raw_write_lock_irq(rwlock_t *lock) +{ + debug_write_lock_before(lock); + arch_write_lock_irq(&lock->raw_lock); + debug_write_lock_after(lock); +} + int do_raw_write_trylock(rwlock_t *lock) { int ret = arch_write_trylock(&lock->raw_lock);