Linux SPARSE checker discussions
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Alexander Aring <aahringo@redhat.com>
Cc: linux-sparse@vger.kernel.org, cluster-devel <cluster-devel@redhat.com>
Subject: Re: sparse warnings related to kref_put_lock() and refcount_dec_and_lock()
Date: Mon, 27 Jun 2022 20:42:32 +0200	[thread overview]
Message-ID: <20220627184232.tjfuzeir57l3h5ll@mail> (raw)
In-Reply-To: <CAK-6q+hd-L54cqOiFNuufS2_VF5XS0R8cjQL7es8921+2u3uwQ@mail.gmail.com>

On Mon, Jun 27, 2022 at 11:15:17AM -0400, Alexander Aring wrote:
> Hi,
> 
> I recently converted to use kref_put_lock() in fs/dlm subsystem and
> now I get the following warning in sparse:
> 
> warning: context imbalance in 'put_rsb' - unexpected unlock
> 
> It seems sparse is not able to detect that there is a conditional
> requirement that the lock passed to kref_put_lock() (or also
> refcount_dec_and_lock()) is locked or not. I evaluate the return value
> to check if kref_put_lock() helds the lock and unlock it then. The
> idea is that the lock needs only to be held when the refcount is going
> to be zero.
> 
> It seems other users unlock the lock at the release callback of
> kref_put_lock() and annotate the callback with "__releases()" which
> seems to work to avoid the sparse warning. However this works if you
> don't have additional stack pointers which you need to pass to the
> release callback.
> 
> My question would be is this a known problem and a recommended way to
> avoid this sparse warning (maybe just for now)?

Hi,

I suppose that your case here can be simplified into something like:

	if (some_condition)
		take(some_lock);

	do_stuff();

	if (some_condition)
		release(some_lock);

Sparse issues the 'context imbalance' warning because, a priori,
it can't exclude that some execution will takes the lock and not
releases it (or the opposite). In some case, when do_stuff() is
very simple, sparse can see that everything is OK, but generally
it won't (some whole kernel analysis but the general case is
undecidable anyway).

The recommended way would be to write things rather like this:

	if (some_condition) {
		take(some_lock);
		do_stuff();
		release(some_lock);
	} else {
		do_stuff();
	}


The __acquires() and __releases() annotations are needed for sparse
to know that the annotated function always take or always release
some lock but won't handle conditional locks.

I hope that this is helpful to you.

-- Luc

  reply	other threads:[~2022-06-27 18:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-27 15:15 sparse warnings related to kref_put_lock() and refcount_dec_and_lock() Alexander Aring
2022-06-27 18:42 ` Luc Van Oostenryck [this message]
2022-06-28  0:56   ` 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

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=20220627184232.tjfuzeir57l3h5ll@mail \
    --to=luc.vanoostenryck@gmail.com \
    --cc=aahringo@redhat.com \
    --cc=cluster-devel@redhat.com \
    --cc=linux-sparse@vger.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