On Mon, Feb 26 2018 at  5:56pm -0500,
Mikulas Patocka <[email protected]> wrote:

> The block layer provides a function direct_make_request - but it doesn't
> provide any test whether this function may be used or not.
> 
> Device mapper currently uses a dirty trick - it compares the result of
> bdevname against the string "nvme" to test if direct_make_request can be
> used.

dm's DM_TYPE_NVME_BIO_BASED is backed by that crude "nvme" check _but_
it is a means to an end.  Alternative would be to have NVMe (and other
devices) set something like QUEUE_FLAG_NO_PARTIAL_COMPLETION or
something -- and then DM would need to verify all underlying devices set
that flag.

> The information whether the driver will or won't recurse can be easily
> obtained in generic_make_request (by comparing make_request_fn - or
> alternatively, we could introduce a queue flag for this), so my suggestion
> is to just delete direct_make_request and fold this "norecurse"
> optimization directly into generic_make_request This will allow us to
> simplify device mapper paths and get rid of device name matching.

Sorry, Nack from me as implemented.  Please see below.

> Index: linux-2.6/block/blk-core.c
> ===================================================================
> --- linux-2.6.orig/block/blk-core.c   2018-02-26 20:37:19.088499000 +0100
> +++ linux-2.6/block/blk-core.c        2018-02-26 20:43:57.839999000 +0100
> @@ -2265,6 +2265,8 @@ end_io:
>       return false;
>  }
>  
> +blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio);
> +
>  /**
>   * generic_make_request - hand a buffer to its device driver for I/O
>   * @bio:  The bio describing the location in memory and on the device.
> @@ -2300,10 +2302,14 @@ blk_qc_t generic_make_request(struct bio
>        */
>       struct bio_list bio_list_on_stack[2];
>       blk_qc_t ret = BLK_QC_T_NONE;
> +     bool may_recurse;
>  
>       if (!generic_make_request_checks(bio))
>               goto out;
>  
> +     may_recurse = bio->bi_disk->queue->make_request_fn != blk_queue_bio &&
> +                   bio->bi_disk->queue->make_request_fn != 
> blk_mq_make_request;
> +

This does _not_ allow dm's DM_TYPE_NVME_BIO_BASED to make use of your
direct_make_request() equivalent that you've encoded into
generic_make_request().

There is no easy way to _know_, at runtime, that an arbitrary queue
(e.g. stacked DM device) does _not_ split IO (therefore not needing to
recurse).

Same can be said of NVMe given NVMe multipath sets its make_request_fn
to nvme_ns_head_make_request().

So for both cases that don't need to recurse you're setting @may_recurse
to true.

As such, this patch is not equivalent to what we have now.

Mike

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to