linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Aring <aahringo@redhat.com>
To: will@kernel.org
Cc: peterz@infradead.org, boqun.feng@gmail.com, mark.rutland@arm.com,
	thunder.leizhen@huawei.com, jacob.e.keller@intel.com,
	akpm@linux-foundation.org, linux-sparse@vger.kernel.org,
	cluster-devel@redhat.com, luc.vanoostenryck@gmail.com,
	torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
	aahringo@redhat.com
Subject: [RFC 2/2] kref: move kref_put_lock() callback to caller
Date: Thu, 30 Jun 2022 09:59:34 -0400	[thread overview]
Message-ID: <20220630135934.1799248-3-aahringo@redhat.com> (raw)
In-Reply-To: <20220630135934.1799248-1-aahringo@redhat.com>

This patch moves the release callback call to the caller of kref_put_lock()
functionality. Since refcount_dec_and_lock() uses __cond_lock() we get
the following warning for e.g. net/sunrpc/svcauth.c:

warning: context imbalance in 'auth_domain_put' - wrong count at exit

The warning occurs now because it seems that before __cond_lock() change
sparse was able to detect the correct locking behaviour. Now it thinks
there is an additional lock acquire. However the __cond_lock()
instrumentation in refcount_dec_and_lock() was making it possible to
avoid sparse warnings by evaluating the return value and unlock the lock
if conditional necessary.

This patch solves the problem by just do the passed release callback
call based by the return value of kref_put_lock() and not inside of
kref_put_lock() and evaluating the return value of
refcount_dec_and_lock() that surprisingly sparse can recognize.

It seems it's only possible to have the one way or the other. This patch
changes the kref_put_lock() in way that it works like
refcount_dec_and_lock() way with __cond_lock().

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 include/linux/kref.h | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/include/linux/kref.h b/include/linux/kref.h
index d32e21a2538c..a70d45940d55 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -68,27 +68,19 @@ static inline int kref_put(struct kref *kref, void (*release)(struct kref *kref)
 	return 0;
 }
 
-static inline int kref_put_mutex(struct kref *kref,
-				 void (*release)(struct kref *kref),
-				 struct mutex *lock)
+static inline bool raw_kref_put_mutex(struct kref *kref, struct mutex *lock)
 {
-	if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) {
-		release(kref);
-		return 1;
-	}
-	return 0;
+	return refcount_dec_and_mutex_lock(&kref->refcount, lock);
 }
+#define kref_put_mutex(kref, release, lock) \
+	((raw_kref_put_mutex(kref, lock)) ? ({ release(kref); 1; }) : 0)
 
-static inline int kref_put_lock(struct kref *kref,
-				void (*release)(struct kref *kref),
-				spinlock_t *lock)
+static inline bool raw_kref_put_lock(struct kref *kref, spinlock_t *lock)
 {
-	if (refcount_dec_and_lock(&kref->refcount, lock)) {
-		release(kref);
-		return 1;
-	}
-	return 0;
+	return refcount_dec_and_lock(&kref->refcount, lock);
 }
+#define kref_put_lock(kref, release, lock) \
+	((raw_kref_put_lock(kref, lock)) ? ({ release(kref); 1; }) : 0)
 
 /**
  * kref_get_unless_zero - Increment refcount for object unless it is zero.
-- 
2.31.1


  parent reply	other threads:[~2022-06-30 14:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-30 13:59 [RFC 0/2] refcount: attempt to avoid imbalance warnings Alexander Aring
2022-06-30 13:59 ` [RFC 1/2] refcount: add __cond_lock() for conditional lock refcount API Alexander Aring
2022-06-30 15:43   ` Peter Zijlstra
2022-06-30 13:59 ` Alexander Aring [this message]
2022-06-30 16:34 ` [RFC 0/2] refcount: attempt to avoid imbalance warnings Linus Torvalds
2022-07-01  8:47   ` Peter Zijlstra
2022-07-01 12:07   ` Alexander Aring
2022-07-01 19:09     ` Alexander Aring
2025-02-08  2:52   ` Bart Van Assche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220630135934.1799248-3-aahringo@redhat.com \
    --to=aahringo@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=cluster-devel@redhat.com \
    --cc=jacob.e.keller@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=luc.vanoostenryck@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --cc=thunder.leizhen@huawei.com \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).