2022-10-05 05:05:36

by Chaitanya Kulkarni

Subject: [RFC PATCH 05/18] drbd: use init disk helper

Add and use the helper to initialize the common fields of struct gendisk
such as major, first_minor, minors, disk_name, private_data, and ops.
This initialization is spread all over the block drivers. This avoids
code repetation of inialization code of gendisk in current block drivers
and any future ones.

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
drivers/block/drbd/drbd_main.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index f3e4db16fd07..58fae122de16 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2706,13 +2706,9 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig

set_disk_ro(disk, true);

- disk->major = DRBD_MAJOR;
- disk->first_minor = minor;
- disk->minors = 1;
- disk->fops = &drbd_ops;
disk->flags |= GENHD_FL_NO_PART;
sprintf(disk->disk_name, "drbd%d", minor);
- disk->private_data = device;
+ init_disk(disk, DRBD_MAJOR, minor, 1, 0, device, &drbd_ops);

blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, disk->queue);
blk_queue_write_cache(disk->queue, true, true);
--
2.29.0

2022-10-05 10:14:13

by Christoph Böhmwalder

Subject: Re: [RFC PATCH 05/18] drbd: use init disk helper

Am 05.10.22 um 07:00 schrieb Chaitanya Kulkarni:
> Add and use the helper to initialize the common fields of struct gendisk
> such as major, first_minor, minors, disk_name, private_data, and ops.
> This initialization is spread all over the block drivers. This avoids
> code repetation of inialization code of gendisk in current block drivers
> and any future ones.
>
> Signed-off-by: Chaitanya Kulkarni <[email protected]>
> ---
> drivers/block/drbd/drbd_main.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
> index f3e4db16fd07..58fae122de16 100644
> --- a/drivers/block/drbd/drbd_main.c
> +++ b/drivers/block/drbd/drbd_main.c
> @@ -2706,13 +2706,9 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig
>
> set_disk_ro(disk, true);
>
> - disk->major = DRBD_MAJOR;
> - disk->first_minor = minor;
> - disk->minors = 1;
> - disk->fops = &drbd_ops;
> disk->flags |= GENHD_FL_NO_PART;
> sprintf(disk->disk_name, "drbd%d", minor);
> - disk->private_data = device;
> + init_disk(disk, DRBD_MAJOR, minor, 1, 0, device, &drbd_ops);
>
> blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, disk->queue);
> blk_queue_write_cache(disk->queue, true, true);

This now does a set_capacity(..., 0), which it did not do before.
I'm guessing this does not have any side effects as the capacity should
already be initialized to 0? Do you know this for sure?

--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA — Disaster Recovery — Software defined Storage

2022-10-05 17:44:56

by Chaitanya Kulkarni

Subject: Re: [RFC PATCH 05/18] drbd: use init disk helper

>> diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
>> index f3e4db16fd07..58fae122de16 100644
>> --- a/drivers/block/drbd/drbd_main.c
>> +++ b/drivers/block/drbd/drbd_main.c
>> @@ -2706,13 +2706,9 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig
>>
>> set_disk_ro(disk, true);
>>
>> - disk->major = DRBD_MAJOR;
>> - disk->first_minor = minor;
>> - disk->minors = 1;
>> - disk->fops = &drbd_ops;
>> disk->flags |= GENHD_FL_NO_PART;
>> sprintf(disk->disk_name, "drbd%d", minor);
>> - disk->private_data = device;
>> + init_disk(disk, DRBD_MAJOR, minor, 1, 0, device, &drbd_ops);
>>
>> blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, disk->queue);
>> blk_queue_write_cache(disk->queue, true, true);
>
> This now does a set_capacity(..., 0), which it did not do before.
> I'm guessing this does not have any side effects as the capacity should
> already be initialized to 0? Do you know this for sure?
>

I think I'll move the call to set_capcity out of the caller,
Will resend the series shortly.

-ck