Re: [PATCH V3 13/13] dm: support IO polling for bio-based dm device

From: Mike Snitzer <snitzer@redhat.com>
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org,
	Jeffle Xu <jefflexu@linux.alibaba.com>,
	dm-devel@redhat.com
Subject: Re: [PATCH V3 13/13] dm: support IO polling for bio-based dm device
Date: Thu, 25 Mar 2021 14:45:19 -0400	[thread overview]
Message-ID: <20210325184519.GB17820@redhat.com> (raw)
In-Reply-To: <20210324121927.362525-14-ming.lei@redhat.com>

On Wed, Mar 24 2021 at  8:19am -0400,
Ming Lei <ming.lei@redhat.com> wrote:

> From: Jeffle Xu <jefflexu@linux.alibaba.com>
> 
> IO polling is enabled when all underlying target devices are capable
> of IO polling. The sanity check supports the stacked device model, in
> which one dm device may be build upon another dm device. In this case,
> the mapped device will check if the underlying dm target device
> supports IO polling.
> 
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/md/dm-table.c         | 24 ++++++++++++++++++++++++
>  drivers/md/dm.c               | 14 ++++++++++++++
>  include/linux/device-mapper.h |  1 +
>  3 files changed, 39 insertions(+)
> 

...

> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 50b693d776d6..fe6893b078dc 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1720,6 +1720,19 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
>  	return ret;
>  }
>  
> +static bool dm_bio_poll_capable(struct gendisk *disk)
> +{
> +	int ret, srcu_idx;
> +	struct mapped_device *md = disk->private_data;
> +	struct dm_table *t;
> +
> +	t = dm_get_live_table(md, &srcu_idx);
> +	ret = dm_table_supports_poll(t);
> +	dm_put_live_table(md, srcu_idx);
> +
> +	return ret;
> +}
> +

I know this code will only get called by blk-core if bio-based but there
isn't anything about this method's implementation that is inherently
bio-based only.

So please rename from dm_bio_poll_capable to dm_poll_capable

Other than that:

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

>  /*-----------------------------------------------------------------
>   * An IDR is used to keep track of allocated minor numbers.
>   *---------------------------------------------------------------*/
> @@ -3132,6 +3145,7 @@ static const struct pr_ops dm_pr_ops = {
>  };
>  
>  static const struct block_device_operations dm_blk_dops = {
> +	.poll_capable = dm_bio_poll_capable,
>  	.submit_bio = dm_submit_bio,
>  	.open = dm_blk_open,
>  	.release = dm_blk_close,