On Thu, Aug 29, 2024 at 9:34 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote: > > Hi, > > 在 2024/08/29 21:13, Xiao Ni 写道: > > On Thu, Aug 29, 2024 at 8:24 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote: > >> > >> Hi, > >> > >> 在 2024/08/29 18:01, Xiao Ni 写道: > >>> This interface is used to update 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. > >> > > Hi Kuai > > > >> I'm not sure why updaing new level during reshape. Will this corrupt > >> data in the array completely? > > > > There is no data corruption. > > > > > >>> > >>> mdadm -CR /dev/md0 -l6 -n4 /dev/loop[0-3] > >>> mdadm --wait /dev/md0 > >>> mdadm /dev/md0 --grow -l5 --backup=backup > >>> cat /proc/mdstat > >>> Personalities : [raid6] [raid5] [raid4] [raid0] [raid1] [raid10] > >>> md0 : active raid6 loop3[3] loop2[2] loop1[1] loop0[0] > >> > >> The problem is that level is still 6 after mddev --grow -l5? I don't > >> understand yet why this is related to update new level during reshape. > >> :) > > > > mdadm --grow command returns once reshape starts when specifying > > backup file. And it needs mdadm-grow-continue service to monitor the > > reshape progress. It doesn't record the new level when running `mdadm > > --grow`. So mdadm-grow-continue doesn't know the new level and it > > can't change to new level after reshape finishes. This needs userspace > > patch to work together. > > https://www.spinics.net/lists/raid/msg78081.html > > Hi, > > Got it. Then I just wonder, what kind of content are stored in the > backup file, why don't store the 'new level' in it as well, after > reshape finishes, can mdadm use the level api to do this? The backup file has a superblock too and the content is the data from the raid. The service monitors reshape progress and controls it. It copies the data from raid to backup file, and then it reshapes. Now we usually don't care about it because the data offset can change and it has free space. For situations where the data offset can't be changed, it needs to write the data to the region where it is read from. If there is a power down or something else, the backup file can avoid data corruption. It should be right to update the new level in md during reshape. If not, the new level is wrong. Best Regards Xiao > > Thanks, > Kuai > > > > > Best Regards > > > > Xiao > >> > >> Thanks, > >> Kuai > >> > >>> > >>> The case 07changelevels in mdadm can trigger this problem. Now it can't > >>> run successfully. This patch is used to fix this. There is also a > >>> userspace patch set that work together with this patch to fix this problem. > >>> > >>> Signed-off-by: Xiao Ni <xni@xxxxxxxxxx> > >>> --- > >>> V2: add detail about test information > >>> drivers/md/md.c | 29 +++++++++++++++++++++++++++++ > >>> 1 file changed, 29 insertions(+) > >>> > >>> diff --git a/drivers/md/md.c b/drivers/md/md.c > >>> index d3a837506a36..3c354e7a7825 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); > >>> + > >>> + mddev_unlock(mddev); > >>> + return len; > >>> +} > >>> +static struct md_sysfs_entry md_new_level = > >>> +__ATTR(new_level, 0664, new_level_show, new_level_store); > >>> + > >>> static ssize_t > >>> layout_show(struct mddev *mddev, char *page) > >>> { > >>> @@ -5666,6 +5694,7 @@ __ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show, > >>> > >>> static struct attribute *md_default_attrs[] = { > >>> &md_level.attr, > >>> + &md_new_level.attr, > >>> &md_layout.attr, > >>> &md_raid_disks.attr, > >>> &md_uuid.attr, > >>> > >> > > > > > > > > . > > >
