Re: [PATCH] md/raid1: only update stack limits with the device in use

[Song Liu]

On Sat, Sep 9, 2023 at 7:24 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> 在 2023/09/09 4:42, Song Liu 写道:
> > On Wed, Sep 6, 2023 at 11:30 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> 在 2023/09/06 17:37, Li Nan 写道:
> >>> Spare device affects array stack limits is unreasonable. For example,
> >>> create a raid1 with two 512 byte devices, the logical_block_size of array
> >>> will be 512. But after add a 4k devcie as spare, logical_block_size of
> >>> array will change as follows.
> >>>
> >>>     mdadm -C /dev/md0 -n 2 -l 10 /dev/sd[ab]   //sd[ab] is 512
> >>>     //logical_block_size of md0: 512
> >>>
> >>>     mdadm --add /dev/md0 /dev/sdc                      //sdc is 4k
> >>>     //logical_block_size of md0: 512
> >>>
> >>>     mdadm -S /dev/md0
> >>>     mdadm -A /dev/md0 /dev/sd[ab]
> >>>     //logical_block_size of md0: 4k
> >>>
> >>> This will confuse users, as nothing has been changed, why did the
> >>> logical_block_size of array change?
> >>>
> >>> Now, only update logical_block_size of array with the device in use.
> >>>
> >>> Signed-off-by: Li Nan <linan122@xxxxxxxxxx>
> >>> ---
> >>>    drivers/md/raid1.c | 19 ++++++++-----------
> >>>    1 file changed, 8 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> >>> index 95504612b7e2..d75c5dd89e86 100644
> >>> --- a/drivers/md/raid1.c
> >>> +++ b/drivers/md/raid1.c
> >>> @@ -3140,19 +3140,16 @@ static int raid1_run(struct mddev *mddev)
> >>
> >> I'm not sure about this behaviour, 'logical_block_size' can be
> >> increased while adding new underlying disk, the key point is not when
> >> to increase 'logical_block_size'. If there is a mounted fs, or
> >> partition in the array, I think the array will be corrupted.
> >
> > How common is such fs/partition corruption? I think some fs and partition
> > table can work properly with 512=>4096 change?
>
> For fs, that should depend on fs bs that is usually set in mkfs, if bs
> is less than 4096, then such fs can't be mounted.
>
> For partition, that is much worse, start sector and end sector will stay
> the same, while sector size is changed. And 4096 -> 512 change is the
> same.

Thanks for this information.

> >>
> >> Perhaps once that array is started, logical_block_size should not be
> >> changed anymore, this will require 'logical_block_size' to be metadate
> >> inside raid superblock. And the array should deny any new disk with
> >> bigger logical_block_size.

I really hope we can avoid adding this to the raid superblock. But I am not
sure what would be a better solution (that is also backward compatible).
Do we have real world reports of such issues?

Thanks,
Song