From: Alexander Aring <aahringo@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
jacob.e.keller@intel.com,
Andrew Morton <akpm@linux-foundation.org>,
thunder.leizhen@huawei.com,
Sparse Mailing-list <linux-sparse@vger.kernel.org>,
cluster-devel <cluster-devel@redhat.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: sparse warnings related to kref_put_lock() and refcount_dec_and_lock()
Date: Wed, 29 Jun 2022 10:42:48 -0400 [thread overview]
Message-ID: <CAK-6q+jngywC7UZ6XG=yww16nJQgz0Z=VYKH8Fhg0kbZCkaypA@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wie9HSY-MmFr9zHeizRTgSfdONop0-4ezBiq5hYNp4U6g@mail.gmail.com>
Hi,
On Tue, Jun 28, 2022 at 1:27 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Tue, Jun 28, 2022 at 1:58 AM Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > I would certainly not recommend this but ...
> > if it's OK to cheat and lie then you can do:
> > + bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock) __acquires(lock);
>
> Actually, we have "__cond_lock()" in the kernel to actually document
> that something takes a lock only in certain conditions.
>
> It needs to be declared as a macro in the header file, with this as an example:
>
> #define raw_spin_trylock(lock) __cond_lock(lock, _raw_spin_trylock(lock))
>
I added a prefix of "raw_" to refcount_dec_and_lock() and similar
functions and replaced the original functions with the __cond_lock()
macro to redirect to their raw_ functions. Similar to how the
raw_spinlock_trylock() naming scheme is doing it. The "raw_"
functionality should never be called by the user then.
> ie that says that "raw_spin_trylock() takes 'lock' when
> _raw_spin_trylock() returned true".
>
> That then makes it possible to write code like
>
> if (raw_spin_trylock(lock)) {..
> raw_spin_unlock(lock));
> }
>
> and sparse will get the nesting right.
>
> But that does *not* solve the issue of people then writing this as
>
> locked = raw_spin_trylock(lock);
> .. do_something ..
> if (locked)
> raw_spin_unlock(lock));
>
> and you end up with the same thing again.
>
Yes it mostly removed all sparse warnings I see. I needed to move at
one call of the refcount_dec_and_lock() function inside the if
condition and the sparse warning was gone. It should not be a problem
to change it in this way.
If there are no other complaints I will send a patch for the raw_
prefix to all those conditional refcount lock functions and the add a
__cond_lock() macro for the original function calls.
Thanks!
- Alex
prev parent reply other threads:[~2022-06-29 14:43 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAK-6q+hd-L54cqOiFNuufS2_VF5XS0R8cjQL7es8921+2u3uwQ@mail.gmail.com>
[not found] ` <20220627184232.tjfuzeir57l3h5ll@mail>
2022-06-28 0:56 ` sparse warnings related to kref_put_lock() and refcount_dec_and_lock() Alexander Aring
2022-06-28 1:06 ` Alexander Aring
2022-06-28 8:58 ` Luc Van Oostenryck
2022-06-28 13:26 ` Alexander Aring
2022-06-28 17:27 ` Linus Torvalds
2022-06-29 14:42 ` Alexander Aring [this message]
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+jngywC7UZ6XG=yww16nJQgz0Z=VYKH8Fhg0kbZCkaypA@mail.gmail.com' \
--to=aahringo@redhat.com \
--cc=akpm@linux-foundation.org \
--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=thunder.leizhen@huawei.com \
--cc=torvalds@linux-foundation.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).