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

[Guenter Roeck <linux@roeck-us.net>]

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 ?

Thanks,
Guenter