Re: [PATCH V3 3/4] md/raid10: improve raid10 discard request

On Thu, Aug 20, 2020 at 12:22 AM Xiao Ni [off-list ref] wrote:
[...]

+
+       if (conf->reshape_progress != MaxSector &&
+           ((bio->bi_iter.bi_sector >= conf->reshape_progress) !=
+            conf->mddev->reshape_backwards))
+               geo = conf->prev;
+
+       stripe_size = (1<<geo.chunk_shift) * geo.raid_disks;

This could be raid_disks << chunk_shift

+       stripe_mask = stripe_size - 1;

Does this work when raid_disks is not power of 2?

In test I used 5 disks to create the raid10 too, it worked well. Could you explain what
you worried in detail?

Say we have geo.raid_disks == 5, and geo.chunk_shift == 8. Then we get
stripe_size == 0x500 and stripe_mask == 0x4ff == b'100 1111 1111
Is this the proper stripe_mask?

Thanks,
Song

On 08/21/2020 06:13 AM, Song Liu wrote:

On Thu, Aug 20, 2020 at 12:22 AM Xiao Ni [off-list ref] wrote:
[...]

quoted

+
+       if (conf->reshape_progress != MaxSector &&
+           ((bio->bi_iter.bi_sector >= conf->reshape_progress) !=
+            conf->mddev->reshape_backwards))
+               geo = conf->prev;
+
+       stripe_size = (1<<geo.chunk_shift) * geo.raid_disks;

This could be raid_disks << chunk_shift

+       stripe_mask = stripe_size - 1;

Does this work when raid_disks is not power of 2?

In test I used 5 disks to create the raid10 too, it worked well. Could you explain what
you worried in detail?

Say we have geo.raid_disks == 5, and geo.chunk_shift == 8. Then we get
stripe_size == 0x500 and stripe_mask == 0x4ff == b'100 1111 1111
Is this the proper stripe_mask?

Thanks,
Song

I got it. Thanks very much. It can't use mask here to justify whether start/end address is aligned
with stripe size. It should check whether it's a multiple of stripe size. I'll send patch again after
fixing all problems.

Regards
Xiao