Re: sparse warnings related to kref_put_lock() and refcount_dec_and_lock()

[Linus Torvalds]

On Tue, Jun 28, 2022 at 1:58 AM Luc Van Oostenryck
<luc.vanoostenryck@xxxxxxxxx> 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))

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.

Anyway, for things like refcount_dec_and_lock(), I suspect that first
pattern should work, because you really shouldn't have that second
pattern. If you just decremented without any locking, the actions are
completely different from the "oh, got the last decrement and now it's
locked and I need to free things" or whatever.

                Linus