diff mbox

fix nvme test

Message ID alpine.LRH.2.02.1802261522141.5096@file01.intranet.prod.int.rdu2.redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Mike Snitzer
Headers show

Commit Message

Mikulas Patocka Feb. 26, 2018, 8:22 p.m. UTC
Hi

The strncmp function should compare 4 bytes.

But I'm wondering what's the purpose of this test at all? bio-based 
interface doesn't support partial completions for any device - so why do 
we need special code path just for nvme and why can't we use it for other 
block devices?

Mikulas


Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-table.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Comments

Mike Snitzer Feb. 26, 2018, 8:50 p.m. UTC | #1
On Mon, Feb 26 2018 at  3:22pm -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Hi
> 
> The strncmp function should compare 4 bytes.

Ah yeap, thanks.

> But I'm wondering what's the purpose of this test at all? bio-based 
> interface doesn't support partial completions for any device - so why do 
> we need special code path just for nvme?

request-based does support partial completions, it is the underlying
devices that matter here, not the upper layer DM device type.

The rationale for this atomic operation requirement is mainly historic:
previously DM_TYPE_NVME_BIO_BASED was (ab)using blk_steal_bios() to
retry a request that failed with a retryable error (as part of a hook
that never was allowed to be introduced to the NVMe request completion
code-path).  For that model it was important that the underlying device
not partially complete its request; because on retry all of the bios
were getting stolen from the request and resubmitted in their entirety
back to the upper layer bio-based device (multipath in my historic
case).

Now that DM_TYPE_NVME_BIO_BASED isn't making use of blk_steal_bios() we
_could_ relax this constraint but I'd prefer to have the option of
using blk_steal_bios() in the future.

> and why can't we use it for other block devices?

I'm not exactly sure about what you mean by "it" but I'll assume you
mean: why can't we use direct_make_request() for other devices?  I've
purposely constrained DM_TYPE_NVME_BIO_BASED to mean we have a target
that is an immutable singleton that is only layered on an NVMe device.
I know this case to currently be uniquely suited for use with
direct_make_request(): no splitting can occur and no partial completion
is used by the underlying driver (the latter less important than the
former).

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
diff mbox

Patch

Index: linux-2.6/drivers/md/dm-table.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-table.c	2018-02-24 03:40:18.000000000 +0100
+++ linux-2.6/drivers/md/dm-table.c	2018-02-26 19:36:29.728499000 +0100
@@ -1755,7 +1755,7 @@  static int device_no_partial_completion(
 	char b[BDEVNAME_SIZE];
 
 	/* For now, NVMe devices are the only devices of this class */
-	return (strncmp(bdevname(dev->bdev, b), "nvme", 3) == 0);
+	return (strncmp(bdevname(dev->bdev, b), "nvme", 4) == 0);
 }
 
 static bool dm_table_does_not_support_partial_completion(struct dm_table *t)