From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 32359288514; Wed, 4 Mar 2026 19:51:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772653904; cv=none; b=PqDTXQT3Rp5gSV1tE5Sbd2eitTIIhINlDYUEVofziy51WjoZ36ikgRLgGRICWyQZPdSdFUWw15a0nXIFQMC9qKIHPrfmepb0baU0SSCyA6QiAH6fwVw2iY+l4v/GdaSzBfyDmjo8ORh6HUryYo45r2Ic2HUC4NDL9PrO0uNMtog= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772653904; c=relaxed/simple; bh=rFmGkgUVzMPgco4e0S5JV2te/mCdrmp4u2/QNzOWFLQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Pc1V5ySnqmXa74FNfEVFj2NZVyDSipvyJvYkC756wMhwFSlfhQ7zrVCqC1I5aLzMfuu9LlXOrHMx0Q1/CXs9WeUAQ1KUhOzmrDpbN1zQrN1DdaT2ARywPGsqxTmA0J5NhduUjKsVDGzuSkW+7dMxfv+JmLaKMxZ5tAqh4PTDlrc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=pX3IkKnI; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="pX3IkKnI" 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=08NDQAJVBDz3lIIuOFusP0LgtmohkQzXTukv2/ZPiuU=; b=pX3IkKnIC6cLxQzOMp1keovyop TS2FsJXeNEXcXPyHii0WzqT16tB7+HKWNEYn4gfnSiwdF56V6oURJkrFM4H1S5cw0aP1xs7Gzn/Hh a4cIHjyoh+MT6RaBjHJsLGoh27Nh0+VUzrrzx5A0gwtaawFXofRqt5t/zRwpYmNCWAX9grnCmg2Rd yqKXD6BNK7nHMWAq6F8xna0er6PulXNsXMl3odNU49mICLB55Z06AuLFO+c7wY8gdE+uiMGCGWRSz zf1GVo8kheJkJPGtaKJ8/Fi7j4+20XbJtolnazCwkngNRLLRaRSJxsTCzEBbxC+tJDID+V05ajLBi DbLr8P+Q==; Received: from willy by casper.infradead.org with local (Exim 4.98.2 #2 (Red Hat Linux)) id 1vxsFu-0000000Doa8-1hV2; Wed, 04 Mar 2026 19:51:38 +0000 Date: Wed, 4 Mar 2026 19:51:38 +0000 From: Matthew Wilcox To: Linus Torvalds Cc: Peter Zijlstra , Ingo Molnar , Will Deacon , Boqun Feng , Waiman Long , linux-kernel@vger.kernel.org, Christoph Hellwig , linux-fsdevel@vger.kernel.org Subject: Re: [RFC 1/1] rwsem: Shrink rwsem by one pointer Message-ID: References: <20260217190835.1151964-1-willy@infradead.org> <20260217190835.1151964-2-willy@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Wed, Feb 18, 2026 at 02:52:29PM -0800, Linus Torvalds wrote: > On Wed, 18 Feb 2026 at 14:45, Linus Torvalds > wrote: > > > > Anyway, this is all from just looking at the patch, so maybe I missed > > something, but it does look very wrong. > > Bah. And immediately after sending, I went "maybe I should look at the > code" more closely. > > I think my suggestion to just remove the check was right, but the > return value of rwsem_del_waiter() needs to be fixed to be the "I used > to be the first waiter, but there are other waiters and I updated the > first waiter pointer". I tried to make this work, but it's a bit ugly. rwsem_del_waiter() needs to know whether there are remaining waiters, and rwsem_del_wake_waiter() needs to know whether we deleted the first waiter _and_ there are remaining ones. So we end up returning a tristate from __rwsem_del_waiter() and I find it less clear. static inline -bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter) +int __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter) { + int ret = 1; + if (list_empty(&waiter->list)) { sem->first_waiter = NULL; - return true; + return 0; } - if (sem->first_waiter == waiter) + if (sem->first_waiter == waiter) { sem->first_waiter = list_first_entry(&waiter->list, struct rwsem_waiter, list); + ret = 2; + } list_del(&waiter->list); - return false; + return ret; } [...] -static inline bool -rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter) +static inline +bool rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter) { + int del_case; + lockdep_assert_held(&sem->wait_lock); - if (__rwsem_del_waiter(sem, waiter)) - return true; - atomic_long_andnot(RWSEM_FLAG_HANDOFF | RWSEM_FLAG_WAITERS, &sem->count); - return false; + del_case = __rwsem_del_waiter(sem, waiter); + if (del_case > 0) + atomic_long_andnot(RWSEM_FLAG_HANDOFF | RWSEM_FLAG_WAITERS, + &sem->count); + return del_case == 2; } [...] { - bool first = sem->first_waiter == waiter; - wake_q_init(wake_q); /* - * If the wait_list isn't empty and the waiter to be deleted is - * the first waiter, we wake up the remaining waiters as they may - * be eligible to acquire or spin on the lock. + * If the deleted waiter was the first one and there are other + * waiters, we wake them up as they may be eligible to acquire + * or spin on the lock. */ - if (rwsem_del_waiter(sem, waiter) && first) + if (rwsem_del_waiter(sem, waiter)) rwsem_mark_wake(sem, RWSEM_WAKE_ANY, wake_q); Even if we use a nice enum instead of 0/1/2 for the return value, I don't think this is an improvement. I played around with a couple of other ways to refactor this and didn't come up with anything pretty.