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 DE6B91EB29 for ; Mon, 6 Nov 2023 11:11:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (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="Jm/0eX5X" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=vUpyzL3z2/C5RrFQiyOF3f2JCk8iw10/Zg7NgkeCw/E=; b=Jm/0eX5XG+QccwGne3HBFtqhrU 10Q6vmKKfAMRz0hNIwIUexTPMf8YXBciHhbYi9nGyExHExOSTAYXeB4MeyKPLcwgPYdpPGch3p0ro VfYf1cDs/9WsASgvvzfHX8+sy90Hxvs2xw7Rwaxm1Q4NF2TuckCW0Chfh1ce4Ea0tc4xlW1UQDP8v XCZrdLVP/7sS1s6YsHF6P5n/L4nuLcXpVIVlTnwhqk5TnTGvW3BCwai99KrQdr6rauhyo8T4ud8Oh ZurA3r9gCyKQMhngIYtn4nSsQC+p61meKsLzTeya9R7XbLXjfG/kqKzpsbl3sNqv4d5s6Su659qyH q38D5haw==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1qzxVY-005ZUp-Ac; Mon, 06 Nov 2023 11:11:05 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id A899430049D; Mon, 6 Nov 2023 12:11:04 +0100 (CET) Date: Mon, 6 Nov 2023 12:11:04 +0100 From: Peter Zijlstra To: Alexander Aring Cc: will@kernel.org, gfs2@lists.linux.dev, boqun.feng@gmail.com, mark.rutland@arm.com, linux-kernel@vger.kernel.org Subject: Re: [RFC 1/2] refcount: introduce generic lockptr funcs Message-ID: <20231106111104.GK8262@noisy.programming.kicks-ass.net> References: <20231103161635.1902667-1-aahringo@redhat.com> <20231103185414.GD8262@noisy.programming.kicks-ass.net> Precedence: bulk X-Mailing-List: gfs2@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Fri, Nov 03, 2023 at 03:20:08PM -0400, Alexander Aring wrote: > Hi, > > On Fri, Nov 3, 2023 at 2:54 PM Peter Zijlstra wrote: > > > > On Fri, Nov 03, 2023 at 12:16:34PM -0400, Alexander Aring wrote: > > > > > diff --git a/lib/refcount.c b/lib/refcount.c > > > index a207a8f22b3c..e28678f0f473 100644 > > > --- a/lib/refcount.c > > > +++ b/lib/refcount.c > > > @@ -94,6 +94,34 @@ bool refcount_dec_not_one(refcount_t *r) > > > } > > > EXPORT_SYMBOL(refcount_dec_not_one); > > > > > > +bool refcount_dec_and_lockptr(refcount_t *r, void (*lock)(void *lockptr), > > > + void (*unlock)(void *lockptr), void *lockptr) > > > +{ > > > + if (refcount_dec_not_one(r)) > > > + return false; > > > + > > > + lock(lockptr); > > > + if (!refcount_dec_and_test(r)) { > > > + unlock(lockptr); > > > + return false; > > > + } > > > + > > > + return true; > > > +} > > > +EXPORT_SYMBOL(refcount_dec_and_lockptr); > > > > This is terrible, you're forcing indirect calls on everything. > > > > Okay, I see. How about introducing a macro producing all the code at > preprocessor time? __always_inline should work, then you get constant propagation for the function pointer. But indeed, perhaps a macro is more convenient vs the irq flags argument. You'll then end up with something like: #define __refcount_dec_and_lock(_ref, _lock, _unlock) \ ({ bool _ret = false; \ if (!refcount_dec_not_one(_ref)) { \ _lock; \ if (!refcount_dec_and_test(_ref)) { \ _unlock; \ } else { \ _ret = true; \ } \ } \ _ret; \ }) bool refcount_dec_and_spinlock_irqsave(refcount_t *r, spinlock_t *lock, unsigned long *flags) { return __refcount_dec_and_lock(r, spin_lock_irqsave(*lock, *flags), spin_unlock_irqrestore(*lock, *flags)); }