Re: [PATCH v9 4/5] md: add check_new_feature module parameter

[Li Nan]

在 2025/11/4 9:47, Xiao Ni 写道:
> On Mon, Nov 3, 2025 at 9:06 PM <linan666@xxxxxxxxxxxxxxx> wrote:
> > From: Li Nan <linan122@xxxxxxxxxx>
> >
> > Raid checks if pad3 is zero when loading superblock from disk. Arrays
> > created with new features may fail to assemble on old kernels as pad3
> > is used.
> >
> > Add module parameter check_new_feature to bypass this check.
> >
> > Signed-off-by: Li Nan <linan122@xxxxxxxxxx>
> > ---
> >   drivers/md/md.c | 12 +++++++++---
> >   1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index dffc6a482181..5921fb245bfa 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -339,6 +339,7 @@ static int start_readonly;
> >    */
> >   static bool create_on_open = true;
> >   static bool legacy_async_del_gendisk = true;
> > +static bool check_new_feature = true;
> >
> >   /*
> >    * We have a system wide 'event count' that is incremented
> > @@ -1850,9 +1851,13 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
> >          }
> >          if (sb->pad0 ||
> >              sb->pad3[0] ||
> > -           memcmp(sb->pad3, sb->pad3+1, sizeof(sb->pad3) - sizeof(sb->pad3[1])))
> > -               /* Some padding is non-zero, might be a new feature */
> > -               return -EINVAL;
> > +           memcmp(sb->pad3, sb->pad3+1, sizeof(sb->pad3) - sizeof(sb->pad3[1]))) {
> > +               pr_warn("Some padding is non-zero on %pg, might be a new feature\n",
> > +                       rdev->bdev);
> > +               if (check_new_feature)
> > +                       return -EINVAL;
> > +               pr_warn("check_new_feature is disabled, data corruption possible\n");
> > +       }
> >
> >          rdev->preferred_minor = 0xffff;
> >          rdev->data_offset = le64_to_cpu(sb->data_offset);
> > @@ -10704,6 +10709,7 @@ module_param(start_dirty_degraded, int, S_IRUGO|S_IWUSR);
> >   module_param_call(new_array, add_named_array, NULL, NULL, S_IWUSR);
> >   module_param(create_on_open, bool, S_IRUSR|S_IWUSR);
> >   module_param(legacy_async_del_gendisk, bool, 0600);
> > +module_param(check_new_feature, bool, 0600);
> >
> >   MODULE_LICENSE("GPL");
> >   MODULE_DESCRIPTION("MD RAID framework");
> > --
> > 2.39.2
>
> Hi
>
> Thanks for finding this problem in time. The default of this kernel
> module is true. I don't think people can check new kernel modules
> after updating to a new kernel. They will find the array can't
> assemble and report bugs. You already use pad3, is it good to remove
> the check about pad3 directly here?
>
> By the way, have you run the regression tests?
>
> Regards
> Xiao
>
>
> .

Hi Xiao.

Thanks for your review.

Deleting this check directly is risky. For example, in configurable LBS:
if user sets LBS to 4K, the LBS of a RAID array assembled on old kernel
becomes 512. Forcing use of this array then risks data loss -- the
original issue this feature want to solve.

Future features may also have similar risks, so instead of deleting this
check directly, I chose to add a module parameter to give users a choice.
What do you think?

--
Thanks,
Nan