Re: [PATCH v2 1/2] block: prevent freeing a zone write plug too early

[Jens Axboe <axboe@kernel.dk>]

On 4/20/24 1:58 AM, Damien Le Moal wrote:
> The submission of plugged BIOs is done using a work struct executing the
> function blk_zone_wplug_bio_work(). This function gets and submits a
> plugged zone write BIO and is guaranteed to operate on a valid zone
> write plug (with a reference count higher than 0) on entry as plugged
> BIOs hold a reference on their zone write plugs. However, once a BIO is
> submitted with submit_bio_noacct_nocheck(), the BIO may complete before
> blk_zone_wplug_bio_work(), with the BIO completion trigering a release
> and freeing of the zone write plug if the BIO is the last write to a
> zone (making the zone FULL). This potentially can result in the zone
> write plug being freed while the work is still active.
> 
> Avoid this by calling flush_work() from disk_free_zone_wplug_rcu().
> 
> Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  block/blk-zoned.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 3befebe6b319..685f0b9159fd 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -526,6 +526,8 @@ static void disk_free_zone_wplug_rcu(struct rcu_head *rcu_head)
>  	struct blk_zone_wplug *zwplug =
>  		container_of(rcu_head, struct blk_zone_wplug, rcu_head);
>  
> +	flush_work(&zwplug->bio_work);
> +
>  	mempool_free(zwplug, zwplug->disk->zone_wplugs_pool);
>  }

This is totally backwards. First of all, if you actually had work that
needed flushing at this point, the kernel would bomb spectacularly.
Secondly, what's the point of using RCU to protect this, if you're now
needing to flush work from the RCU callback? That's a clear sign that
something is very wrong here with your references / RCU usage.. The work
item should hold a reference to it, trying to paper around it like this
is not going to work at all.

Why is the work item racing with RCU freeing?!

-- 
Jens Axboe