Re: [PATCH] dm: fix race between dm_mq_queue_rq and __dm_destroy

[Mikulas Patocka <mpatocka@redhat.com>]

Hi

Please explain why are there in-progress requests when the device is being 
destroyed. What kind of workload does trigger this crash?

The device can only be destroyed if it's open count is zero. And if no one 
has the device open, there should be no requests in flight.

If some kernel code is sending requests to a closed block device, it 
should be fixed to not do that.

Mikulas



On Wed, 25 Mar 2026, Libo Chen wrote:

> The existing dm_get() calls BUG_ON if DMF_FREEING is already set. This
> can be triggered when a request is dispatched via blk_mq_requeue_work()
> racing with __dm_destroy():
> 
>   CPU0                           CPU1
>   ----                           ----
>   __dm_destroy()
>     set_bit(DMF_FREEING)
>                                  blk_mq_requeue_work()
>                                    dm_mq_queue_rq()
>                                      dm_start_request()
>                                        dm_get(md)
>                                          BUG_ON(DMF_FREEING) <-- crash
> 
> Introduce dm_try_get() which increments holders first, then checks
> DMF_FREEING. If set, it decrements holders and returns false. The
> smp_mb__after_atomic() ensures the increment is visible before reading
> the flag, providing correct synchronization with __dm_destroy().
> 
> dm_hold() was not used because it acquires the global _minor_lock,
> which would cause contention in the hot I/O path.
> 
> Move dm_try_get() before blk_mq_start_request() in dm_start_request().
> This ensures we don't start the block layer timer or mark the request
> MQ_RQ_IN_FLIGHT for a request that will immediately fail due to device
> destruction. If dm_try_get() fails, we return early without calling
> blk_mq_start_request(), providing cleaner error semantics.
> 
> Signed-off-by: Libo Chen <libchen@purestorage.com>
> ---
>  drivers/md/dm-rq.c            | 26 ++++++++++++++++----------
>  drivers/md/dm.c               | 12 ++++++++++++
>  include/linux/device-mapper.h |  1 +
>  3 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> index a6ca92049c10..dc6ba46cad92 100644
> --- a/drivers/md/dm-rq.c
> +++ b/drivers/md/dm-rq.c
> @@ -430,8 +430,20 @@ ssize_t dm_attr_rq_based_seq_io_merge_deadline_store(struct mapped_device *md,
>  	return count;
>  }
>  
> -static void dm_start_request(struct mapped_device *md, struct request *orig)
> +static bool dm_start_request(struct mapped_device *md, struct request *orig)
>  {
> +	/*
> +	 * Hold the md reference here for the in-flight I/O.
> +	 * We can't rely on the reference count by device opener,
> +	 * because the device may be closed during the request completion
> +	 * when all bios are completed.
> +	 * See the comment in rq_completed() too.
> +	 *
> +	 * Fail if DMF_FREEING is set to avoid racing with __dm_destroy().
> +	 */
> +	if (!dm_try_get(md))
> +		return false;
> +
>  	blk_mq_start_request(orig);
>  
>  	if (unlikely(dm_stats_used(&md->stats))) {
> @@ -444,14 +456,7 @@ static void dm_start_request(struct mapped_device *md, struct request *orig)
>  				    &tio->stats_aux);
>  	}
>  
> -	/*
> -	 * Hold the md reference here for the in-flight I/O.
> -	 * We can't rely on the reference count by device opener,
> -	 * because the device may be closed during the request completion
> -	 * when all bios are completed.
> -	 * See the comment in rq_completed() too.
> -	 */
> -	dm_get(md);
> +	return true;
>  }
>  
>  static int dm_mq_init_request(struct blk_mq_tag_set *set, struct request *rq,
> @@ -508,7 +513,8 @@ static blk_status_t dm_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	if (ti->type->busy && ti->type->busy(ti))
>  		return BLK_STS_RESOURCE;
>  
> -	dm_start_request(md, rq);
> +	if (!dm_start_request(md, rq))
> +		return BLK_STS_IOERR;
>  
>  	/* Init tio using md established in .init_request */
>  	init_tio(tio, rq, md);
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 6c83ab940af7..bd75b34de333 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -2673,6 +2673,18 @@ void dm_get(struct mapped_device *md)
>  	BUG_ON(test_bit(DMF_FREEING, &md->flags));
>  }
>  
> +bool dm_try_get(struct mapped_device *md)
> +{
> +	atomic_inc(&md->holders);
> +	smp_mb__after_atomic();
> +
> +	if (unlikely(test_bit(DMF_FREEING, &md->flags))) {
> +		atomic_dec(&md->holders);
> +		return false;
> +	}
> +	return true;
> +}
> +
>  int dm_hold(struct mapped_device *md)
>  {
>  	spin_lock(&_minor_lock);
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index 84fdc3a6a19a..8df81c180f2f 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -500,6 +500,7 @@ int dm_create(int minor, struct mapped_device **md);
>   */
>  struct mapped_device *dm_get_md(dev_t dev);
>  void dm_get(struct mapped_device *md);
> +bool dm_try_get(struct mapped_device *md);
>  int dm_hold(struct mapped_device *md);
>  void dm_put(struct mapped_device *md);
>  
> -- 
> 2.34.1
>