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]>
---
block/genhd.c | 13 +++++++++++++
drivers/block/null_blk/main.c | 12 ++++--------
include/linux/blkdev.h | 5 +++++
3 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 514395361d7c..701309a7388e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1470,3 +1470,16 @@ void inc_diskseq(struct gendisk *disk)
{
disk->diskseq = atomic64_inc_return(&diskseq);
}
+
+void init_disk(struct gendisk *disk, int major, int first_minor,
+ int minors, sector_t sectors, void *private_data,
+ const struct block_device_operations *fops)
+{
+ disk->major = major;
+ disk->first_minor = first_minor;
+ disk->minors = minors;
+ set_capacity(disk, sectors);
+ disk->private_data = private_data;
+ disk->fops = fops;
+}
+EXPORT_SYMBOL_GPL(init_disk);
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 1f154f92f4c2..d31085c94fd3 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1871,18 +1871,14 @@ static int init_driver_queues(struct nullb *nullb)
static int null_gendisk_register(struct nullb *nullb)
{
sector_t size = ((sector_t)nullb->dev->size * SZ_1M) >> SECTOR_SHIFT;
+ const struct block_device_operations *fops;
struct gendisk *disk = nullb->disk;
- set_capacity(disk, size);
-
- disk->major = null_major;
- disk->first_minor = nullb->index;
- disk->minors = 1;
if (queue_is_mq(nullb->q))
- disk->fops = &null_rq_ops;
+ fops = &null_rq_ops;
else
- disk->fops = &null_bio_ops;
- disk->private_data = nullb;
+ fops = &null_bio_ops;
+ init_disk(disk, null_major, nullb->index, 1, size, nullb, fops);
strncpy(disk->disk_name, nullb->disk_name, DISK_NAME_LEN);
if (nullb->dev->zoned) {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 49373d002631..cb9db857f890 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -757,6 +757,11 @@ static inline int __must_check add_disk(struct gendisk *disk)
{
return device_add_disk(NULL, disk, NULL);
}
+
+void init_disk(struct gendisk *disk, int major, int first_minor,
+ int minors, sector_t sectors, void *private_data,
+ const struct block_device_operations *fops);
+
void del_gendisk(struct gendisk *gp);
void invalidate_disk(struct gendisk *disk);
void set_disk_ro(struct gendisk *disk, bool read_only);
--
2.29.0
On Tue, Oct 04, 2022 at 10:00:10PM -0700, Chaitanya Kulkarni wrote:
> +void init_disk(struct gendisk *disk, int major, int first_minor,
> + int minors, sector_t sectors, void *private_data,
> + const struct block_device_operations *fops)
> +{
> + disk->major = major;
> + disk->first_minor = first_minor;
> + disk->minors = minors;
> + set_capacity(disk, sectors);
> + disk->private_data = private_data;
> + disk->fops = fops;
I don't like this at all. For one major/first_minor/minors are
optional and discouraged for new drivers. Setting the capacity is
a different thing and is done by helpers also used for revalidation
in many drivers.
It might make sense to pass the fops (and maybe private_data) to
blk_mq_alloc_disk / blk_alloc_disk, but even then I'm not quite
sure it is worth the churn.
On 10/10/22 00:59, Christoph Hellwig wrote:
> On Tue, Oct 04, 2022 at 10:00:10PM -0700, Chaitanya Kulkarni wrote:
>> +void init_disk(struct gendisk *disk, int major, int first_minor,
>> + int minors, sector_t sectors, void *private_data,
>> + const struct block_device_operations *fops)
>> +{
>> + disk->major = major;
>> + disk->first_minor = first_minor;
>> + disk->minors = minors;
>> + set_capacity(disk, sectors);
>> + disk->private_data = private_data;
>> + disk->fops = fops;
>
> I don't like this at all. For one major/first_minor/minors are
> optional and discouraged for new drivers. Setting the capacity is
> a different thing and is done by helpers also used for revalidation
> in many drivers.
>
> It might make sense to pass the fops (and maybe private_data) to
> blk_mq_alloc_disk / blk_alloc_disk, but even then I'm not quite
> sure it is worth the churn.
Okay I'll drop this, and see if I can get something meaningful
with fops and private data.
-ck