在 2025/3/10 22:41, Blazej Kucman 写道:
> On Mon, 10 Mar 2025 19:09:36 +0800
> Wu Guanghao <wuguanghao3@xxxxxxxxxx> wrote:
>
> Hi,
> Thanks for your patch.
>
> you are only adding a change to native metadata so it would be good to
> emphasize this in the title, please change "mdadm:" to "super1:"
>
> There are also a few checkpatch issues,
>
>
>> When adding a disk to a RAID1 array, the metadata is read from the
>> existing member disks for sync. However, only the bad_blocks flag are
>> copied, the bad_blocks records are not copied, so the bad_blocks
>> records are all zeros. The kernel function super_1_load() detects
>> bad_blocks flag and reads the bad_blocks record, then sets the bad
>> block using badblocks_set().
>
> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit
> description?) #8:
> the bad_blocks records are not copied, so the bad_blocks records are
> all zeros.
>
>>
>> After the kernel commit 1726c7746("badblocks: improve badblocks_set()
>> for multiple ranges handling"), if the length of a bad_blocks record
>
> please use SHA-1 ID - first 12 characters and space between ID and
> (Tile)
>
>> is 0, it will return a failure. Therefore the device addition will
>> fail.
>>
>> So when adding a new disk, some flags cannot be sync and need to be
>> cleared.
>>
>> Signed-off-by: Wu Guanghao <wuguanghao3@xxxxxxxxxx>
>> ---
>> Changelog:
>> v2:
>> Add a testcase.
>> Clear extra replace flag.
>> ---
>> super1.c | 4 ++++
>> tests/05r1-add-badblocks | 25 +++++++++++++++++++++++++
>> 2 files changed, 29 insertions(+)
>> create mode 100644 tests/05r1-add-badblocks
>>
>> diff --git a/super1.c b/super1.c
>> index fe3c4c64..f4a29f4f 100644
>> --- a/super1.c
>> +++ b/super1.c
>> @@ -1971,6 +1971,10 @@ static int write_init_super1(struct supertype
>> *st) long bm_offset;
>> bool raid0_need_layout = false;
>>
>> + /* Clear extra flags */
>> + sb->feature_map &= ~__cpu_to_le32(MD_FEATURE_BAD_BLOCKS |
>> + MD_FEATURE_REPLACEMENT);
>
> ERROR: code indent should use tabs where possible
> #36: FILE: super1.c:1976:
> + MD_FEATURE_REPLACEMENT);$
>
> WARNING: please, no spaces at the start of a line
> #36: FILE: super1.c:1976:
> + MD_FEATURE_REPLACEMENT);$
>
> However, in this case the code will fit on one line, the limit is 100
> characters.
>
Thank you for your feedback. I will modify it in the next version.
>> +
>> /* Since linux kernel v5.4, raid0 always has a layout */
>> if (has_raid0_layout(sb) && get_linux_version() >= 5004000)
>> raid0_need_layout = true;
>> diff --git a/tests/05r1-add-badblocks b/tests/05r1-add-badblocks
>> new file mode 100644
>> index 00000000..88b064f2
>> --- /dev/null
>> +++ b/tests/05r1-add-badblocks
>> @@ -0,0 +1,25 @@
>> +#
>> +# create a raid1 with a drive and set badblocks for the drive.
>> +# add a new drive does not cause an error.
>> +#
>> +
>> +# create raid1
>> +mdadm -CR $md0 -l1 -n2 -e1.0 $dev1 missing
>> +testdev $md0 1 $mdsize1a 64
>> +sleep 3
>> +
>> +# set badblocks for the drive
>> +dev1_name=$(basename $dev1)
>> +echo "10000 100" > /sys/block/md0/md/dev-$dev1_name/bad_blocks
>> +echo "write_error" > /sys/block/md0/md/dev-$dev1_name/state
>> +
>> +# maybe fail but that's ok, as it's only intended to
>> +# record the bad block in the metadata.
>> +mkfs.ext3 $md0
>> +
>> +# re-add and recovery
>> +mdadm $md0 -a $dev2
>> +check recovery
>> +
>> +mdadm -S $md0
>> +
>
> Since you added the test, would you be able to issue a PR on github
> to get the tests running?
>
Sure, I will create a PR and run the testcase.
> Thansk,
> Blazej
>
> .
