From: Alexander Aring <aahringo@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Will Deacon <will@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Boqun Feng <boqun.feng@gmail.com>,
Mark Rutland <mark.rutland@arm.com>,
thunder.leizhen@huawei.com, jacob.e.keller@intel.com,
Andrew Morton <akpm@linux-foundation.org>,
Sparse Mailing-list <linux-sparse@vger.kernel.org>,
cluster-devel <cluster-devel@redhat.com>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RFC 0/2] refcount: attempt to avoid imbalance warnings
Date: Fri, 1 Jul 2022 08:07:01 -0400 [thread overview]
Message-ID: <CAK-6q+jkNbotWK7cFsNGO+B+ApcdUd7+_4mdcF8=00YsDAATTA@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wjZfO9hGqJ2_hGQG3U_XzSh9_XaXze=HgPdvJbgrvASfA@mail.gmail.com>
Hi,
On Thu, Jun 30, 2022 at 12:34 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Jun 30, 2022 at 6:59 AM Alexander Aring <aahringo@redhat.com> wrote:
> >
> > I send this patch series as RFC because it was necessary to do a kref
> > change after adding __cond_lock() to refcount_dec_and_lock()
> > functionality.
>
> Can you try something like this instead?
>
> This is two separate patches - one for sparse, and one for the kernel.
>
> This is only *very* lightly tested (ie I tested it on a single kernel
> file that used refcount_dec_and_lock())
>
yes that avoids the warnings for fs/dlm.c by calling unlock() when the
kref_put_lock() returns true.
However there exists other users of kref_put_lock() which drops a
sparse warning now after those patches e.g. net/sunrpc/svcauth.c.
I think I can explain why. It is that kref_put_lock() has a release
callback and it's _optional_ that this release callback calls the
unlock(). If the release callback calls unlock() then the user of
kref_put_lock() signals this with a releases() annotation of the
passed release callback.
It seems that sparse is not detecting this annotation anymore when
it's passed as callback and the function pointer parameter declaration
of kref_put_lock() does not have such annotation. The annotation gets
"dropped" then.
If I change the parameter order and add a annotation to the release
callback, like:
__kref_put_lock(struct kref *kref, spinlock_t *lock,
void (*release)(struct kref *kref) __releases(lock))
#define kref_put_lock(kref, release, lock) __kref_put_lock(kref, lock, release)
the problem is gone but forces every user to release the lock in the
release callback which isn't required and also cuts the API because
the lock which you want to call unlock() on can be not part of your
container_of(kref) struct.
Then I did a similar thing before which would solve it for every user
because there is simply no function pointer passed as parameter and
the annotation gets never "dropped":
#define kref_put_lock(kref, release, lock) \
(refcount_dec_and_lock(&(kref)->refcount, lock) ? ({ release(kref); 1; }) : 0)
Maybe a functionality of forwarding function annotation if passed as a
function pointer (function pointer declared without annotations) as in
e.g. kref_put_lock() can be added into sparse?
- Alex
next prev parent reply other threads:[~2022-07-01 12:07 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 ` [RFC 2/2] kref: move kref_put_lock() callback to caller Alexander Aring
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 [this message]
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='CAK-6q+jkNbotWK7cFsNGO+B+ApcdUd7+_4mdcF8=00YsDAATTA@mail.gmail.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 read-only IMAP folder(s) and NNTP newsgroup(s).