On Tue, 3 Sep 2024 08:18:19 +0800
Xiao Ni <xni@xxxxxxxxxx> wrote:
> On Mon, Sep 2, 2024 at 6:14 PM Mariusz Tkaczyk
> <mariusz.tkaczyk@xxxxxxxxxxxxxxx> wrote:
> >
> > On Wed, 28 Aug 2024 10:18:28 +0800
> > Xiao Ni <xni@xxxxxxxxxx> wrote:
> >
> > > This interface is used to updating new level during reshape progress.
> > > Now it doesn't update new level during reshape. So it can't know the
> > > new level when systemd service mdadm-grow-continue runs. And it can't
> > > finally change to new level.
> > >
> > > Signed-off-by: Xiao Ni <xni@xxxxxxxxxx>
> > > ---
> > > drivers/md/md.c | 29 +++++++++++++++++++++++++++++
> > > 1 file changed, 29 insertions(+)
> > >
> > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > index d3a837506a36..c639eca03df9 100644
> > > --- a/drivers/md/md.c
> > > +++ b/drivers/md/md.c
> > > @@ -4141,6 +4141,34 @@ level_store(struct mddev *mddev, const char *buf,
> > > size_t len) static struct md_sysfs_entry md_level =
> > > __ATTR(level, S_IRUGO|S_IWUSR, level_show, level_store);
> > >
> > > +static ssize_t
> > > +new_level_show(struct mddev *mddev, char *page)
> > > +{
> > > + return sprintf(page, "%d\n", mddev->new_level);
> > > +}
> > > +
> > > +static ssize_t
> > > +new_level_store(struct mddev *mddev, const char *buf, size_t len)
> > > +{
> > > + unsigned int n;
> > > + int err;
> > > +
> > > + err = kstrtouint(buf, 10, &n);
> > > + if (err < 0)
> > > + return err;
> > > + err = mddev_lock(mddev);
> > > + if (err)
> > > + return err;
> > > +
> > > + mddev->new_level = n;
> > > + md_update_sb(mddev, 1);
> >
> > I don't see any code behind mddev->new_level handling so I suspect that
> > md_update_sb() does nothing in this case. Is there something I'm missing?
>
> You mean the calling path md_update_sb->sync_sbs->super_1_sync?
>
No, I missed that mddev->new_level is a exiting property and it is already
implemented in kernel. Now, I understand that it is super0 metadata property
and you just presented it. This is fine.
What I can see is that calling md_update_super() in case of
external might be unnecessary but it is fine anyway I think.
maybe: if (!mddev->external)
md_update_sb(mddev, 1);
but I don't see it used in the code anywhere. metadata update path is
still black box to me. Maybe Kuai can comment?
Thanks,
Mariusz
