Re: [dm-devel] [PATCH 5.4 26/34] dm verity: set DM_TARGET_IMMUTABLE feature flag

[Mike Snitzer <snitzer@kernel.org>]

On Wed, Jun 15 2022 at 10:36P -0400,
Guenter Roeck <linux@roeck-us.net> wrote:

> On Mon, Jun 13, 2022 at 11:13:21AM +0200, Greg KH wrote:
> > On Fri, Jun 10, 2022 at 11:11:00AM -0400, Mike Snitzer wrote:
> > > On Fri, Jun 10 2022 at  1:15P -0400,
> > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > > 
> > > > On Fri, Jun 10, 2022 at 04:22:00AM +0000, Oleksandr Tymoshenko wrote:
> > > > > I believe this commit introduced a regression in dm verity on systems
> > > > > where data device is an NVME one. Loading table fails with the
> > > > > following diagnostics:
> > > > > 
> > > > > device-mapper: table: table load rejected: including non-request-stackable devices
> > > > > 
> > > > > The same kernel works with the same data drive on the SCSI interface.
> > > > > NVME-backed dm verity works with just this commit reverted.
> > > > > 
> > > > > I believe the presence of the immutable partition is used as an indicator
> > > > > of special case NVME configuration and if the data device's name starts
> > > > > with "nvme" the code tries to switch the target type to
> > > > > DM_TYPE_NVME_BIO_BASED (drivers/md/dm-table.c lines 1003-1010).
> > > > > 
> > > > > The special NVME optimization case was removed in
> > > > > 5.10 by commit 9c37de297f6590937f95a28bec1b7ac68a38618f, so only 5.4 is
> > > > > affected.
> > > > > 
> > > > 
> > > > Why wouldn't 4.9, 4.14, and 4.19 also be affected here?  Should I also
> > > > just queue up 9c37de297f65 ("dm: remove special-casing of bio-based
> > > > immutable singleton target on NVMe") to those older kernels?  If so,
> > > > have you tested this and verified that it worked?
> > > 
> > > Sorry for the unforeseen stable@ troubles here!
> > > 
> > > In general we'd be fine to apply commit 9c37de297f65 but to do it
> > > properly would require also making sure commits that remove
> > > "DM_TYPE_NVME_BIO_BASED", like 8d47e65948dd ("dm mpath: remove
> > > unnecessary NVMe branching in favor of scsi_dh checks") are applied --
> > > basically any lingering references to DM_TYPE_NVME_BIO_BASED need to
> > > be removed.
> > > 
> > > The commit header for 8d47e65948dd documents what
> > > DM_TYPE_NVME_BIO_BASED was used for.. it was dm-mpath specific and
> > > "nvme" mode really never got used by any userspace that I'm aware of.
> > > 
> > > Sadly I currently don't have the time to do this backport for all N
> > > stable kernels... :(
> > > 
> > > But if that backport gets out of control: A simpler, albeit stable@
> > > unicorn, way to resolve this is to simply revert 9c37de297f65 and make
> 
> 9c37de297f65 can not be reverted in 5.4 and older because it isn't there,
> and trying to apply it results in conflicts which at least I can not
> resolve.
> 
> > > it so that DM-mpath and DM core just used bio-based if "nvme" is
> > > requested by dm-mpath, so also in drivers/md/dm-mpath.c e.g.:
> > > 
> > > @@ -1091,8 +1088,6 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
> > > 
> > >                         if (!strcasecmp(queue_mode_name, "bio"))
> > >                                 m->queue_mode = DM_TYPE_BIO_BASED;
> > > 			else if (!strcasecmp(queue_mode_name, "nvme"))
> > > -                               m->queue_mode = DM_TYPE_NVME_BIO_BASED;
> > > +                               m->queue_mode = DM_TYPE_BIO_BASED;
> > >                         else if (!strcasecmp(queue_mode_name, "rq"))
> > >                                 m->queue_mode = DM_TYPE_REQUEST_BASED;
> > >                         else if (!strcasecmp(queue_mode_name, "mq"))
> > > 
> > > Mike
> > > 
> > 
> > Ok, please submit a working patch for the kernels that need it so that
> > we can review and apply it to solve this regression.
> > 
> 
> So, effectively, v5.4.y and older are broken right now for use cases
> with dm on NVME drives.
> 
> Given that the regression does affect older branches, and given that we
> have to revert this patch to avoid regressions in ChromeOS, would it be
> possible to revert it from v5.4.y and older until a fix is found ?

I obviously would prefer to not have this false-start.

I'll look at latest 5.4.y _now_ and see what can be done.

Should hopefully be pretty straight-forward.

Mike

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