On Thu, 12 Mar 2020 00:21:56 +0000 Jules Irenge <jbi.octave@xxxxxxxxx> wrote:
> wb_congested_put() was written in such a way that made it difficult
> for Sparse tool not to complain
> Expanding the function locking block in the if statement improves on
> the readability of the code. Rewritting it comes with one add-on:
>
> It fixes a warning reported by Sparse tool at wb_congested_put()
>
> warning: context imbalance in wb_congested_put() - unexpected unlock
>
> Refactor the function wb_congested_put()
>
> ...
>
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -464,18 +464,21 @@ void wb_congested_put(struct bdi_writeback_congested *congested)
> {
> unsigned long flags;
>
> - if (!refcount_dec_and_lock_irqsave(&congested->refcnt, &cgwb_lock, &flags))
> - return;
> -
> + if (!refcount_dec_not_one(&congested->refcnt)) {
> + spin_lock_irqsave(&cgwb_lock, flags);
> + if (!refcount_dec_and_test(&congested->refcnt)) {
> + spin_unlock_irqrestore(&cgwb_lock, flags);
> + return;
> + }
> /* bdi might already have been destroyed leaving @congested unlinked */
> - if (congested->__bdi) {
> - rb_erase(&congested->rb_node,
> - &congested->__bdi->cgwb_congested_tree);
> - congested->__bdi = NULL;
> + if (congested->__bdi) {
> + rb_erase(&congested->rb_node,
> + &congested->__bdi->cgwb_congested_tree);
> + congested->__bdi = NULL;
> + }
> + spin_unlock_irqrestore(&cgwb_lock, flags);
> + kfree(congested);
> }
> -
> - spin_unlock_irqrestore(&cgwb_lock, flags);
> - kfree(congested);
> }
hm, it's hard to get excited over this. Open-coding the
refcount_dec_and_lock_irqsave() internals at a callsite in order to
make sparse happy.
Is there some other way, using __acquires (for example)?
