Re: [PATCH v6 00/15] md/raid5: set STRIPE_SIZE as a configurable value

[Song Liu <song@kernel.org>]

On Wed, Jul 15, 2020 at 5:42 AM Yufen Yu <yuyufen@huawei.com> wrote:
>
> Hi, all
>
>  For now, STRIPE_SIZE is equal to the value of PAGE_SIZE. That means, RAID5
>  will issue each bio to disk at least 64KB when PAGE_SIZE is 64KB in arm64.
>  However, filesystem usually issue bio in the unit of 4KB. Then, RAID5 may
>  waste resource of disk bandwidth.
>
>  To solve the problem, this patchset try to set stripe_size as a configuare
>  value. The default value is 4096. We will add a new sysfs entry and set it
>  by writing a new value, likely:
>
>         echo 16384 > /sys/block/md1/md/stripe_size

[...]

> V6:
>  * Convert stripe_size and stripe_sectors from 'unsigned int' to
>    'unsigned long' avoiding compiler warning.
>  * Add a new member of stripes_per_page into r5conf, avoiding to
>    compute each time.
>  * Cover mddev->private with mddev_lock(mddev) for raid5_store/show_stripe_size().
>  * Get rid of too many WARN_ON() and BUG_ON().
>  * Unfold raid5_get_page_index() into raid5_get_dev_page() directly.

We are running out of time before the upcoming merge window, so we may not
be able to ship the whole set. How about we ship 01, and 11 in this
merge window,
and the rest in the next merge window? Would the array get most of the
performance benefit with these 2 patches? Do you see other issues with
this path?

Also I would like to minimize changes for PAGE_SIZE==4096 systems. Ideally,
we can hide these changes from these system in ifdef's, like:

#if PAGE_SIZE == DEFAULT_STRIPE_SIZE
#define STRIPE_SIZE            PAGE_SIZE
#define STRIPE_SHIFT           (PAGE_SHIFT - 9)
#define STRIPE_SECTORS         (STRIPE_SIZE>>9)
#endif

#if PAGE_SIZE != DEFAULT_STRIPE_SIZE
unsigned long           stripe_size;
unsigned int            stripe_shift;
unsigned long           stripe_sectors;
#endif

#if PAGE_SIZE != DEFAULT_STRIPE_SIZE
#define RAID5_STRIPE_SIZE(conf) DEFAULT_STRIPE_SIZE
#else
#define RAID5_STRIPE_SIZE(conf) ((conf)->stripe_size)
#endif

And then use RAID5_STRIPE_SIZE(conf) in the rest of the code.

Thanks,
Song