On Thu, Oct 20, 2022 at 10:26 PM Florian-Ewald Müller
<florian-ewald.mueller@xxxxxxxxx> wrote:
>
> Hello Song,
>
> Thank you for taking this patch into consideration.
> This patch was indeed meant only as a RFC.
>
> We have, sometimes, the situation that a md-raid1 device of e.g. size=128G, created
> with bitmap=internal and bitmap-chunk=256M, needs to be resized to (let's say) size=8T.
>
> We originally set the bitmap-chunk=256M because we found it a good compromise between performance and resync speed.
> With that bitmap-chunk=256G and the starting size=128G, the md-bitmap code will use only 512 bits fitting into a page (4K).
>
> When resizing to size=8T, the md-bitmap code will attempt to reuse the space available (4k) and calculate a bitmap-chunk=2G.
> We found here that such a huge bitmap-chunk (2G) is not so suitable for an eventual, later resync process.
> Also taking into consideration that IOPS issued on the now resized device won't increase/differ significantly.
>
> This is a particular usage scenario and the effect of this patch may, indeed, not be optimal in some other use case.
>
> Best regards,
> Florian
>
>
resent with plain text, so it's available on mainlist
>
> On Thu, Oct 20, 2022 at 9:22 PM Song Liu <song@xxxxxxxxxx> wrote:
>>
>> On Fri, Oct 14, 2022 at 5:22 AM Jack Wang <jinpu.wang@xxxxxxxxx> wrote:
>> >
>> > From: Florian-Ewald Mueller <florian-ewald.mueller@xxxxxxxxx>
>> >
>> > On bitmap resizing, attempt to reuse the former chunk size (if any)
>> > in order to preserve the, ev. on purpose set, chunk size resolution.
>>
>> Could you please explain more on why we would rather keep the chunk size
>> instead of recalculating it?
>>
>> Thanks,
>> Song
>>
>> >
>> > Signed-off-by: Florian-Ewald Mueller <florian-ewald.mueller@xxxxxxxxx>
>> > Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxx>
>> > ---
>> > drivers/md/md-cluster.c | 3 ++-
>> > drivers/md/raid1.c | 3 ++-
>> > drivers/md/raid10.c | 12 ++++++++----
>> > drivers/md/raid5.c | 3 ++-
>> > 4 files changed, 14 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
>> > index 10e0c5381d01..9929631bdc94 100644
>> > --- a/drivers/md/md-cluster.c
>> > +++ b/drivers/md/md-cluster.c
>> > @@ -604,7 +604,8 @@ static int process_recvd_msg(struct mddev *mddev, struct cluster_msg *msg)
>> > case BITMAP_RESIZE:
>> > if (le64_to_cpu(msg->high) != mddev->pers->size(mddev, 0, 0))
>> > ret = md_bitmap_resize(mddev->bitmap,
>> > - le64_to_cpu(msg->high), 0, 0);
>> > + le64_to_cpu(msg->high),
>> > + mddev->bitmap_info.chunksize, 0);
>> > break;
>> > default:
>> > ret = -1;
>> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> > index 05d8438cfec8..8f1d25064a53 100644
>> > --- a/drivers/md/raid1.c
>> > +++ b/drivers/md/raid1.c
>> > @@ -3225,7 +3225,8 @@ static int raid1_resize(struct mddev *mddev, sector_t sectors)
>> > mddev->array_sectors > newsize)
>> > return -EINVAL;
>> > if (mddev->bitmap) {
>> > - int ret = md_bitmap_resize(mddev->bitmap, newsize, 0, 0);
>> > + int ret = md_bitmap_resize(mddev->bitmap, newsize,
>> > + mddev->bitmap_info.chunksize, 0);
>> > if (ret)
>> > return ret;
>> > }
>> > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> > index 3aa8b6e11d58..8f0453b6e0c6 100644
>> > --- a/drivers/md/raid10.c
>> > +++ b/drivers/md/raid10.c
>> > @@ -4346,7 +4346,8 @@ static int raid10_resize(struct mddev *mddev, sector_t sectors)
>> > mddev->array_sectors > size)
>> > return -EINVAL;
>> > if (mddev->bitmap) {
>> > - int ret = md_bitmap_resize(mddev->bitmap, size, 0, 0);
>> > + int ret = md_bitmap_resize(mddev->bitmap, size,
>> > + mddev->bitmap_info.chunksize, 0);
>> > if (ret)
>> > return ret;
>> > }
>> > @@ -4618,7 +4619,8 @@ static int raid10_start_reshape(struct mddev *mddev)
>> > newsize = raid10_size(mddev, 0, conf->geo.raid_disks);
>> >
>> > if (!mddev_is_clustered(mddev)) {
>> > - ret = md_bitmap_resize(mddev->bitmap, newsize, 0, 0);
>> > + ret = md_bitmap_resize(mddev->bitmap, newsize,
>> > + mddev->bitmap_info.chunksize, 0);
>> > if (ret)
>> > goto abort;
>> > else
>> > @@ -4640,13 +4642,15 @@ static int raid10_start_reshape(struct mddev *mddev)
>> > MD_FEATURE_RESHAPE_ACTIVE)) || (oldsize == newsize))
>> > goto out;
>> >
>> > - ret = md_bitmap_resize(mddev->bitmap, newsize, 0, 0);
>> > + ret = md_bitmap_resize(mddev->bitmap, newsize,
>> > + mddev->bitmap_info.chunksize, 0);
>> > if (ret)
>> > goto abort;
>> >
>> > ret = md_cluster_ops->resize_bitmaps(mddev, newsize, oldsize);
>> > if (ret) {
>> > - md_bitmap_resize(mddev->bitmap, oldsize, 0, 0);
>> > + md_bitmap_resize(mddev->bitmap, oldsize,
>> > + mddev->bitmap_info.chunksize, 0);
>> > goto abort;
>> > }
>> > }
>> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> > index 7b820b81d8c2..bff7d3d779ae 100644
>> > --- a/drivers/md/raid5.c
>> > +++ b/drivers/md/raid5.c
>> > @@ -8372,7 +8372,8 @@ static int raid5_resize(struct mddev *mddev, sector_t sectors)
>> > mddev->array_sectors > newsize)
>> > return -EINVAL;
>> > if (mddev->bitmap) {
>> > - int ret = md_bitmap_resize(mddev->bitmap, sectors, 0, 0);
>> > + int ret = md_bitmap_resize(mddev->bitmap, sectors,
>> > + mddev->bitmap_info.chunksize, 0);
>> > if (ret)
>> > return ret;
>> > }
>> > --
>> > 2.34.1
>> >
