Discussion:
[PATCH 01/26] block: refactor device number setup in __device_add_disk
Christoph Hellwig
2021-05-21 05:50:51 UTC
Permalink
Untangle the mess around blk_alloc_devt by moving the check for
the used allocation scheme into the callers.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
block/blk.h | 4 +-
block/genhd.c | 96 ++++++++++++++++-------------------------
block/partitions/core.c | 15 +++++--
3 files changed, 49 insertions(+), 66 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 8b3591aee0a5..cba3a94aabfa 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -343,8 +343,8 @@ static inline void blk_queue_free_zone_bitmaps(struct request_queue *q) {}
static inline void blk_queue_clear_zone_settings(struct request_queue *q) {}
#endif

-int blk_alloc_devt(struct block_device *part, dev_t *devt);
-void blk_free_devt(dev_t devt);
+int blk_alloc_ext_minor(void);
+void blk_free_ext_minor(unsigned int minor);
char *disk_name(struct gendisk *hd, int partno, char *buf);
#define ADDPART_FLAG_NONE 0
#define ADDPART_FLAG_RAID 1
diff --git a/block/genhd.c b/block/genhd.c
index 39ca97b0edc6..2c00bc3261d9 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -335,52 +335,22 @@ static int blk_mangle_minor(int minor)
return minor;
}

-/**
- * blk_alloc_devt - allocate a dev_t for a block device
- * @bdev: block device to allocate dev_t for
- * @devt: out parameter for resulting dev_t
- *
- * Allocate a dev_t for block device.
- *
- * RETURNS:
- * 0 on success, allocated dev_t is returned in *@devt. -errno on
- * failure.
- *
- * CONTEXT:
- * Might sleep.
- */
-int blk_alloc_devt(struct block_device *bdev, dev_t *devt)
+int blk_alloc_ext_minor(void)
{
- struct gendisk *disk = bdev->bd_disk;
int idx;

- /* in consecutive minor range? */
- if (bdev->bd_partno < disk->minors) {
- *devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
- return 0;
- }
-
idx = ida_alloc_range(&ext_devt_ida, 0, NR_EXT_DEVT, GFP_KERNEL);
- if (idx < 0)
- return idx == -ENOSPC ? -EBUSY : idx;
-
- *devt = MKDEV(BLOCK_EXT_MAJOR, blk_mangle_minor(idx));
- return 0;
+ if (idx < 0) {
+ if (idx == -ENOSPC)
+ return -EBUSY;
+ return idx;
+ }
+ return blk_mangle_minor(idx);
}

-/**
- * blk_free_devt - free a dev_t
- * @devt: dev_t to free
- *
- * Free @devt which was allocated using blk_alloc_devt().
- *
- * CONTEXT:
- * Might sleep.
- */
-void blk_free_devt(dev_t devt)
+void blk_free_ext_minor(unsigned int minor)
{
- if (MAJOR(devt) == BLOCK_EXT_MAJOR)
- ida_free(&ext_devt_ida, blk_mangle_minor(MINOR(devt)));
+ ida_free(&ext_devt_ida, blk_mangle_minor(minor));
}

static char *bdevt_str(dev_t devt, char *buf)
@@ -501,8 +471,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
const struct attribute_group **groups,
bool register_queue)
{
- dev_t devt;
- int retval;
+ int ret;

/*
* The disk queue should now be all set with enough information about
@@ -513,23 +482,29 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
if (register_queue)
elevator_init_mq(disk->queue);

- /* minors == 0 indicates to use ext devt from part0 and should
- * be accompanied with EXT_DEVT flag. Make sure all
- * parameters make sense.
+ /*
+ * If the driver provides an explicit major number it also must provide
+ * the number of minors numbers supported, and those will be used to
+ * setup the gendisk.
+ * Otherwise just allocate the device numbers for both the whole device
+ * and all partitions from the extended dev_t space.
*/
- WARN_ON(disk->minors && !(disk->major || disk->first_minor));
- WARN_ON(!disk->minors &&
- !(disk->flags & (GENHD_FL_EXT_DEVT | GENHD_FL_HIDDEN)));
-
- disk->flags |= GENHD_FL_UP;
+ if (disk->major) {
+ WARN_ON(!disk->minors);
+ } else {
+ WARN_ON(disk->minors);
+ WARN_ON(!(disk->flags & (GENHD_FL_EXT_DEVT | GENHD_FL_HIDDEN)));

- retval = blk_alloc_devt(disk->part0, &devt);
- if (retval) {
- WARN_ON(1);
- return;
+ ret = blk_alloc_ext_minor();
+ if (ret < 0) {
+ WARN_ON(1);
+ return;
+ }
+ disk->major = BLOCK_EXT_MAJOR;
+ disk->first_minor = MINOR(ret);
}
- disk->major = MAJOR(devt);
- disk->first_minor = MINOR(devt);
+
+ disk->flags |= GENHD_FL_UP;

disk_alloc_events(disk);

@@ -543,14 +518,14 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
} else {
struct backing_dev_info *bdi = disk->queue->backing_dev_info;
struct device *dev = disk_to_dev(disk);
- int ret;

/* Register BDI before referencing it from bdev */
- dev->devt = devt;
- ret = bdi_register(bdi, "%u:%u", MAJOR(devt), MINOR(devt));
+ dev->devt = MKDEV(disk->major, disk->first_minor);
+ ret = bdi_register(bdi, "%u:%u",
+ disk->major, disk->first_minor);
WARN_ON(ret);
bdi_set_owner(bdi, dev);
- bdev_add(disk->part0, devt);
+ bdev_add(disk->part0, dev->devt);
}
register_disk(parent, disk, groups);
if (register_queue)
@@ -1129,7 +1104,8 @@ static void disk_release(struct device *dev)

might_sleep();

- blk_free_devt(dev->devt);
+ if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR)
+ blk_free_ext_minor(MINOR(dev->devt));
disk_release_events(disk);
kfree(disk->random);
xa_destroy(&disk->part_tbl);
diff --git a/block/partitions/core.c b/block/partitions/core.c
index dc60ecf46fe6..504297bdc8bf 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -260,7 +260,8 @@ static const struct attribute_group *part_attr_groups[] = {

static void part_release(struct device *dev)
{
- blk_free_devt(dev->devt);
+ if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR)
+ blk_free_ext_minor(MINOR(dev->devt));
bdput(dev_to_bdev(dev));
}

@@ -379,9 +380,15 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
pdev->type = &part_type;
pdev->parent = ddev;

- err = blk_alloc_devt(bdev, &devt);
- if (err)
- goto out_put;
+ /* in consecutive minor range? */
+ if (bdev->bd_partno < disk->minors) {
+ devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
+ } else {
+ err = blk_alloc_ext_minor();
+ if (err < 0)
+ goto out_put;
+ devt = MKDEV(BLOCK_EXT_MAJOR, err);
+ }
pdev->devt = devt;

/* delay uevent until 'holders' subdir is created */
--
2.30.2
Christoph Hellwig
2021-05-21 05:50:52 UTC
Permalink
Keep this together with the first place that actually looks at
->minors and prepare for not passing a minors argument to
alloc_disk.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
block/genhd.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 2c00bc3261d9..7f9beaeede11 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -491,6 +491,12 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
*/
if (disk->major) {
WARN_ON(!disk->minors);
+
+ if (disk->minors > DISK_MAX_PARTS) {
+ pr_err("block: can't allocate more than %d partitions\n",
+ DISK_MAX_PARTS);
+ disk->minors = DISK_MAX_PARTS;
+ }
} else {
WARN_ON(disk->minors);
WARN_ON(!(disk->flags & (GENHD_FL_EXT_DEVT | GENHD_FL_HIDDEN)));
@@ -1264,13 +1270,6 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
{
struct gendisk *disk;

- if (minors > DISK_MAX_PARTS) {
- printk(KERN_ERR
- "block: can't allocate more than %d partitions\n",
- DISK_MAX_PARTS);
- minors = DISK_MAX_PARTS;
- }
-
disk = kzalloc_node(sizeof(struct gendisk), GFP_KERNEL, node_id);
if (!disk)
return NULL;
--
2.30.2
Luis Chamberlain
2021-05-21 17:18:07 UTC
Permalink
Post by Christoph Hellwig
Keep this together with the first place that actually looks at
->minors and prepare for not passing a minors argument to
alloc_disk.
Reviewed-by: Luis Chamberlain <***@kernel.org>

Luis
Hannes Reinecke
2021-05-23 07:48:20 UTC
Permalink
Post by Christoph Hellwig
Keep this together with the first place that actually looks at
->minors and prepare for not passing a minors argument to
alloc_disk.
---
block/genhd.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:50:54 UTC
Permalink
Add a flag to indicate that __device_add_disk did grab a queue reference
so that disk_release only drops it if we actually had it. This sort
out one of the major pitfals with partially initialized gendisk that
a lot of drivers did get wrong or still do.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
block/genhd.c | 7 +++++--
include/linux/genhd.h | 1 +
2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index eec266c9318d..e4974af3d729 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -541,7 +541,10 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
* Take an extra ref on queue which will be put on disk_release()
* so that it sticks around as long as @disk is there.
*/
- WARN_ON_ONCE(!blk_get_queue(disk->queue));
+ if (blk_get_queue(disk->queue))
+ set_bit(GD_QUEUE_REF, &disk->state);
+ else
+ WARN_ON_ONCE(1);

disk_add_events(disk);
blk_integrity_add(disk);
@@ -1116,7 +1119,7 @@ static void disk_release(struct device *dev)
kfree(disk->random);
xa_destroy(&disk->part_tbl);
bdput(disk->part0);
- if (disk->queue)
+ if (test_bit(GD_QUEUE_REF, &disk->state) && disk->queue)
blk_put_queue(disk->queue);
kfree(disk);
}
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 7e9660ea967d..a5443f0b139d 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -153,6 +153,7 @@ struct gendisk {
unsigned long state;
#define GD_NEED_PART_SCAN 0
#define GD_READ_ONLY 1
+#define GD_QUEUE_REF 2
struct kobject *slave_dir;

struct timer_rand_state *random;
--
2.30.2
Luis Chamberlain
2021-05-21 17:28:41 UTC
Permalink
Post by Christoph Hellwig
Add a flag to indicate that __device_add_disk did grab a queue reference
so that disk_release only drops it if we actually had it. This sort
out one of the major pitfals with partially initialized gendisk that
a lot of drivers did get wrong or still do.
Reviewed-by: Luis Chamberlain <***@kernel.org>

Luis
Hannes Reinecke
2021-05-23 07:54:05 UTC
Permalink
Post by Christoph Hellwig
Add a flag to indicate that __device_add_disk did grab a queue reference
so that disk_release only drops it if we actually had it. This sort
out one of the major pitfals with partially initialized gendisk that
pitfalls
Post by Christoph Hellwig
a lot of drivers did get wrong or still do.
---
block/genhd.c | 7 +++++--
include/linux/genhd.h | 1 +
2 files changed, 6 insertions(+), 2 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:50:55 UTC
Permalink
Add two new APIs to allocate and free a gendisk including the
request_queue for use with BIO based drivers. This is to avoid
boilerplate code in drivers.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
block/genhd.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/genhd.h | 22 ++++++++++++++++++++++
2 files changed, 57 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index e4974af3d729..6d4ce962866d 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1302,6 +1302,25 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
}
EXPORT_SYMBOL(__alloc_disk_node);

+struct gendisk *__blk_alloc_disk(int node)
+{
+ struct request_queue *q;
+ struct gendisk *disk;
+
+ q = blk_alloc_queue(node);
+ if (!q)
+ return NULL;
+
+ disk = __alloc_disk_node(0, node);
+ if (!disk) {
+ blk_cleanup_queue(q);
+ return NULL;
+ }
+ disk->queue = q;
+ return disk;
+}
+EXPORT_SYMBOL(__blk_alloc_disk);
+
/**
* put_disk - decrements the gendisk refcount
* @disk: the struct gendisk to decrement the refcount for
@@ -1319,6 +1338,22 @@ void put_disk(struct gendisk *disk)
}
EXPORT_SYMBOL(put_disk);

+/**
+ * blk_cleanup_disk - shutdown a gendisk allocated by blk_alloc_disk
+ * @disk: gendisk to shutdown
+ *
+ * Mark the queue hanging off @disk DYING, drain all pending requests, then mark
+ * the queue DEAD, destroy and put it and the gendisk structure.
+ *
+ * Context: can sleep
+ */
+void blk_cleanup_disk(struct gendisk *disk)
+{
+ blk_cleanup_queue(disk->queue);
+ put_disk(disk);
+}
+EXPORT_SYMBOL(blk_cleanup_disk);
+
static void set_disk_ro_uevent(struct gendisk *gd, int ro)
{
char event[] = "DISK_RO=1";
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index a5443f0b139d..03aa12730634 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -278,6 +278,28 @@ extern void put_disk(struct gendisk *disk);

#define alloc_disk(minors) alloc_disk_node(minors, NUMA_NO_NODE)

+/**
+ * blk_alloc_disk - allocate a gendisk structure
+ * @node_id: numa node to allocate on
+ *
+ * Allocate and pre-initialize a gendisk structure for use with BIO based
+ * drivers.
+ *
+ * Context: can sleep
+ */
+#define blk_alloc_disk(node_id) \
+({ \
+ struct gendisk *__disk = __blk_alloc_disk(node_id); \
+ static struct lock_class_key __key; \
+ \
+ if (__disk) \
+ lockdep_init_map(&__disk->lockdep_map, \
+ "(bio completion)", &__key, 0); \
+ __disk; \
+})
+struct gendisk *__blk_alloc_disk(int node);
+void blk_cleanup_disk(struct gendisk *disk);
+
int __register_blkdev(unsigned int major, const char *name,
void (*probe)(dev_t devt));
#define register_blkdev(major, name) \
--
2.30.2
Luis Chamberlain
2021-05-21 17:44:07 UTC
Permalink
Post by Christoph Hellwig
Add two new APIs to allocate and free a gendisk including the
request_queue for use with BIO based drivers. This is to avoid
boilerplate code in drivers.
---
block/genhd.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/genhd.h | 22 ++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index e4974af3d729..6d4ce962866d 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1302,6 +1302,25 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
}
EXPORT_SYMBOL(__alloc_disk_node);
+struct gendisk *__blk_alloc_disk(int node)
+{
+ struct request_queue *q;
+ struct gendisk *disk;
+
+ q = blk_alloc_queue(node);
+ if (!q)
+ return NULL;
+
+ disk = __alloc_disk_node(0, node);
+ if (!disk) {
+ blk_cleanup_queue(q);
+ return NULL;
+ }
+ disk->queue = q;
+ return disk;
+}
+EXPORT_SYMBOL(__blk_alloc_disk);
Its not obvious to me why using this new API requires you then to
set minors explicitly to 1, and yet here underneath we see the minors
argument passed is 0.

Nor is it clear from the documentation.

Luis
Christoph Hellwig
2021-05-24 07:24:13 UTC
Permalink
Post by Luis Chamberlain
Its not obvious to me why using this new API requires you then to
set minors explicitly to 1, and yet here underneath we see the minors
argument passed is 0.
Nor is it clear from the documentation.
Basically for all new drivers no one should set minors at all, and the
dynamic dev_t mechanism does all the work. For converted old drivers
minors is set manually instead of being passed an an argument that
should be 0 for all new drivers.
Hannes Reinecke
2021-05-23 07:55:12 UTC
Permalink
Post by Christoph Hellwig
Add two new APIs to allocate and free a gendisk including the
request_queue for use with BIO based drivers. This is to avoid
boilerplate code in drivers.
---
block/genhd.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/genhd.h | 22 ++++++++++++++++++++++
2 files changed, 57 insertions(+)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:50:53 UTC
Permalink
Automatically set the GENHD_FL_EXT_DEVT flag for all disks allocated
without an explicit number of minors. This is what all new block
drivers should do, so make sure it is the default without boilerplate
code.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
block/genhd.c | 2 +-
block/partitions/core.c | 4 ----
drivers/block/n64cart.c | 2 +-
drivers/lightnvm/core.c | 1 -
drivers/memstick/core/ms_block.c | 1 -
drivers/nvdimm/blk.c | 1 -
drivers/nvdimm/btt.c | 1 -
drivers/nvdimm/pmem.c | 1 -
drivers/nvme/host/core.c | 1 -
drivers/nvme/host/multipath.c | 1 -
10 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 7f9beaeede11..eec266c9318d 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -499,7 +499,6 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
}
} else {
WARN_ON(disk->minors);
- WARN_ON(!(disk->flags & (GENHD_FL_EXT_DEVT | GENHD_FL_HIDDEN)));

ret = blk_alloc_ext_minor();
if (ret < 0) {
@@ -508,6 +507,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
}
disk->major = BLOCK_EXT_MAJOR;
disk->first_minor = MINOR(ret);
+ disk->flags |= GENHD_FL_EXT_DEVT;
}

disk->flags |= GENHD_FL_UP;
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 504297bdc8bf..ada3e1e66989 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -326,10 +326,6 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
const char *dname;
int err;

- /*
- * disk_max_parts() won't be zero, either GENHD_FL_EXT_DEVT is set
- * or 'minors' is passed to alloc_disk().
- */
if (partno >= disk_max_parts(disk))
return ERR_PTR(-EINVAL);

diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 47bdf324e962..3dae4b631dea 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -141,7 +141,7 @@ static int __init n64cart_probe(struct platform_device *pdev)
return -ENOMEM;

disk->first_minor = 0;
- disk->flags = GENHD_FL_NO_PART_SCAN | GENHD_FL_EXT_DEVT;
+ disk->flags = GENHD_FL_NO_PART_SCAN;
disk->fops = &n64cart_fops;
disk->private_data = &pdev->dev;
strcpy(disk->disk_name, "n64cart");
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 40a948c08a0b..e7dc539fc0ac 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -383,7 +383,6 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
}

strlcpy(tdisk->disk_name, create->tgtname, sizeof(tdisk->disk_name));
- tdisk->flags = GENHD_FL_EXT_DEVT;
tdisk->major = 0;
tdisk->first_minor = 0;
tdisk->fops = tt->bops;
diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 8004dd64d09a..0bacf4268f83 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -2136,7 +2136,6 @@ static int msb_init_disk(struct memstick_dev *card)
msb->disk->fops = &msb_bdops;
msb->disk->private_data = msb;
msb->disk->queue = msb->queue;
- msb->disk->flags |= GENHD_FL_EXT_DEVT;

capacity = msb->pages_in_block * msb->logical_block_count;
capacity *= (msb->page_size / 512);
diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
index 7b9556291eb1..7ba446d224fb 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -267,7 +267,6 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk)
disk->first_minor = 0;
disk->fops = &nd_blk_fops;
disk->queue = q;
- disk->flags = GENHD_FL_EXT_DEVT;
disk->private_data = nsblk;
nvdimm_namespace_disk_name(&nsblk->common, disk->disk_name);

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 18a267d5073f..1741a7b0b30f 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1537,7 +1537,6 @@ static int btt_blk_init(struct btt *btt)
btt->btt_disk->fops = &btt_fops;
btt->btt_disk->private_data = btt;
btt->btt_disk->queue = btt->btt_queue;
- btt->btt_disk->flags = GENHD_FL_EXT_DEVT;

blk_queue_logical_block_size(btt->btt_queue, btt->sector_size);
blk_queue_max_hw_sectors(btt->btt_queue, UINT_MAX);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index ed10a8b66068..968b8483c763 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -477,7 +477,6 @@ static int pmem_attach_disk(struct device *dev,

disk->fops = &pmem_fops;
disk->queue = q;
- disk->flags = GENHD_FL_EXT_DEVT;
disk->private_data = pmem;
nvdimm_namespace_disk_name(ndns, disk->disk_name);
set_capacity(disk, (pmem->size - pmem->pfn_pad - pmem->data_offset)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 762125f2905f..24bcae88587a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3699,7 +3699,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
disk->fops = &nvme_bdev_ops;
disk->private_data = ns;
disk->queue = ns->queue;
- disk->flags = GENHD_FL_EXT_DEVT;
/*
* Without the multipath code enabled, multiple controller per
* subsystems are visible as devices and thus we cannot use the
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index f81871c7128a..a5d02f236cca 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -462,7 +462,6 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
head->disk->fops = &nvme_ns_head_ops;
head->disk->private_data = head;
head->disk->queue = q;
- head->disk->flags = GENHD_FL_EXT_DEVT;
sprintf(head->disk->disk_name, "nvme%dn%d",
ctrl->subsys->instance, head->instance);
return 0;
--
2.30.2
Luis Chamberlain
2021-05-21 17:22:37 UTC
Permalink
Post by Christoph Hellwig
Automatically set the GENHD_FL_EXT_DEVT flag for all disks allocated
without an explicit number of minors. This is what all new block
drivers should do, so make sure it is the default without boilerplate
code.
Reviewed-by: Luis Chamberlain <***@kernel.org>

Luis
Hannes Reinecke
2021-05-23 07:50:20 UTC
Permalink
Post by Christoph Hellwig
Automatically set the GENHD_FL_EXT_DEVT flag for all disks allocated
without an explicit number of minors. This is what all new block
drivers should do, so make sure it is the default without boilerplate
code.
---
block/genhd.c | 2 +-
block/partitions/core.c | 4 ----
drivers/block/n64cart.c | 2 +-
drivers/lightnvm/core.c | 1 -
drivers/memstick/core/ms_block.c | 1 -
drivers/nvdimm/blk.c | 1 -
drivers/nvdimm/btt.c | 1 -
drivers/nvdimm/pmem.c | 1 -
drivers/nvme/host/core.c | 1 -
drivers/nvme/host/multipath.c | 1 -
10 files changed, 2 insertions(+), 13 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:50:56 UTC
Permalink
Convert the brd driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation. This also
allows to remove the request_queue pointer in struct request_queue,
and to simplify the initialization as blk_cleanup_disk can be called
on any disk returned from blk_alloc_disk.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/brd.c | 94 ++++++++++++++++-----------------------------
1 file changed, 33 insertions(+), 61 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 7562cf30b14e..95694113e38e 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -38,9 +38,7 @@
* device).
*/
struct brd_device {
- int brd_number;
-
- struct request_queue *brd_queue;
+ int brd_number;
struct gendisk *brd_disk;
struct list_head brd_list;

@@ -372,7 +370,7 @@ static LIST_HEAD(brd_devices);
static DEFINE_MUTEX(brd_devices_mutex);
static struct dentry *brd_debugfs_dir;

-static struct brd_device *brd_alloc(int i)
+static int brd_alloc(int i)
{
struct brd_device *brd;
struct gendisk *disk;
@@ -380,64 +378,55 @@ static struct brd_device *brd_alloc(int i)

brd = kzalloc(sizeof(*brd), GFP_KERNEL);
if (!brd)
- goto out;
+ return -ENOMEM;
brd->brd_number = i;
spin_lock_init(&brd->brd_lock);
INIT_RADIX_TREE(&brd->brd_pages, GFP_ATOMIC);

- brd->brd_queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!brd->brd_queue)
- goto out_free_dev;
-
snprintf(buf, DISK_NAME_LEN, "ram%d", i);
if (!IS_ERR_OR_NULL(brd_debugfs_dir))
debugfs_create_u64(buf, 0444, brd_debugfs_dir,
&brd->brd_nr_pages);

- /* This is so fdisk will align partitions on 4k, because of
- * direct_access API needing 4k alignment, returning a PFN
- * (This is only a problem on very small devices <= 4M,
- * otherwise fdisk will align on 1M. Regardless this call
- * is harmless)
- */
- blk_queue_physical_block_size(brd->brd_queue, PAGE_SIZE);
- disk = brd->brd_disk = alloc_disk(max_part);
+ disk = brd->brd_disk = blk_alloc_disk(NUMA_NO_NODE);
if (!disk)
- goto out_free_queue;
+ goto out_free_dev;
+
disk->major = RAMDISK_MAJOR;
disk->first_minor = i * max_part;
+ disk->minors = max_part;
disk->fops = &brd_fops;
disk->private_data = brd;
disk->flags = GENHD_FL_EXT_DEVT;
strlcpy(disk->disk_name, buf, DISK_NAME_LEN);
set_capacity(disk, rd_size * 2);
+
+ /*
+ * This is so fdisk will align partitions on 4k, because of
+ * direct_access API needing 4k alignment, returning a PFN
+ * (This is only a problem on very small devices <= 4M,
+ * otherwise fdisk will align on 1M. Regardless this call
+ * is harmless)
+ */
+ blk_queue_physical_block_size(disk->queue, PAGE_SIZE);

/* Tell the block layer that this is not a rotational device */
- blk_queue_flag_set(QUEUE_FLAG_NONROT, brd->brd_queue);
- blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, brd->brd_queue);
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
+ blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);
+ add_disk(disk);
+ list_add_tail(&brd->brd_list, &brd_devices);

- return brd;
+ return 0;

-out_free_queue:
- blk_cleanup_queue(brd->brd_queue);
out_free_dev:
kfree(brd);
-out:
- return NULL;
-}
-
-static void brd_free(struct brd_device *brd)
-{
- put_disk(brd->brd_disk);
- blk_cleanup_queue(brd->brd_queue);
- brd_free_pages(brd);
- kfree(brd);
+ return -ENOMEM;
}

static void brd_probe(dev_t dev)
{
- struct brd_device *brd;
int i = MINOR(dev) / max_part;
+ struct brd_device *brd;

mutex_lock(&brd_devices_mutex);
list_for_each_entry(brd, &brd_devices, brd_list) {
@@ -445,13 +434,7 @@ static void brd_probe(dev_t dev)
goto out_unlock;
}

- brd = brd_alloc(i);
- if (brd) {
- brd->brd_disk->queue = brd->brd_queue;
- add_disk(brd->brd_disk);
- list_add_tail(&brd->brd_list, &brd_devices);
- }
-
+ brd_alloc(i);
out_unlock:
mutex_unlock(&brd_devices_mutex);
}
@@ -460,7 +443,9 @@ static void brd_del_one(struct brd_device *brd)
{
list_del(&brd->brd_list);
del_gendisk(brd->brd_disk);
- brd_free(brd);
+ blk_cleanup_disk(brd->brd_disk);
+ brd_free_pages(brd);
+ kfree(brd);
}

static inline void brd_check_and_reset_par(void)
@@ -485,7 +470,7 @@ static inline void brd_check_and_reset_par(void)
static int __init brd_init(void)
{
struct brd_device *brd, *next;
- int i;
+ int err, i;

/*
* brd module now has a feature to instantiate underlying device
@@ -511,22 +496,11 @@ static int __init brd_init(void)

mutex_lock(&brd_devices_mutex);
for (i = 0; i < rd_nr; i++) {
- brd = brd_alloc(i);
- if (!brd)
+ err = brd_alloc(i);
+ if (err)
goto out_free;
- list_add_tail(&brd->brd_list, &brd_devices);
}

- /* point of no return */
-
- list_for_each_entry(brd, &brd_devices, brd_list) {
- /*
- * associate with queue just before adding disk for
- * avoiding to mess up failure path
- */
- brd->brd_disk->queue = brd->brd_queue;
- add_disk(brd->brd_disk);
- }
mutex_unlock(&brd_devices_mutex);

pr_info("brd: module loaded\n");
@@ -535,15 +509,13 @@ static int __init brd_init(void)
out_free:
debugfs_remove_recursive(brd_debugfs_dir);

- list_for_each_entry_safe(brd, next, &brd_devices, brd_list) {
- list_del(&brd->brd_list);
- brd_free(brd);
- }
+ list_for_each_entry_safe(brd, next, &brd_devices, brd_list)
+ brd_del_one(brd);
mutex_unlock(&brd_devices_mutex);
unregister_blkdev(RAMDISK_MAJOR, "ramdisk");

pr_info("brd: module NOT loaded !!!\n");
- return -ENOMEM;
+ return err;
}

static void __exit brd_exit(void)
--
2.30.2
Hannes Reinecke
2021-05-23 07:58:48 UTC
Permalink
Post by Christoph Hellwig
Convert the brd driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation. This also
allows to remove the request_queue pointer in struct request_queue,
and to simplify the initialization as blk_cleanup_disk can be called
on any disk returned from blk_alloc_disk.
---
drivers/block/brd.c | 94 ++++++++++++++++-----------------------------
1 file changed, 33 insertions(+), 61 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 7562cf30b14e..95694113e38e 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -38,9 +38,7 @@
* device).
*/
struct brd_device {
- int brd_number;
-
- struct request_queue *brd_queue;
+ int brd_number;
struct gendisk *brd_disk;
struct list_head brd_list;
@@ -372,7 +370,7 @@ static LIST_HEAD(brd_devices);
static DEFINE_MUTEX(brd_devices_mutex);
static struct dentry *brd_debugfs_dir;
-static struct brd_device *brd_alloc(int i)
+static int brd_alloc(int i)
{
struct brd_device *brd;
struct gendisk *disk;
@@ -380,64 +378,55 @@ static struct brd_device *brd_alloc(int i)
brd = kzalloc(sizeof(*brd), GFP_KERNEL);
if (!brd)
- goto out;
+ return -ENOMEM;
brd->brd_number = i;
spin_lock_init(&brd->brd_lock);
INIT_RADIX_TREE(&brd->brd_pages, GFP_ATOMIC);
- brd->brd_queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!brd->brd_queue)
- goto out_free_dev;
-
snprintf(buf, DISK_NAME_LEN, "ram%d", i);
if (!IS_ERR_OR_NULL(brd_debugfs_dir))
debugfs_create_u64(buf, 0444, brd_debugfs_dir,
&brd->brd_nr_pages);
- /* This is so fdisk will align partitions on 4k, because of
- * direct_access API needing 4k alignment, returning a PFN
- * (This is only a problem on very small devices <= 4M,
- * otherwise fdisk will align on 1M. Regardless this call
- * is harmless)
- */
- blk_queue_physical_block_size(brd->brd_queue, PAGE_SIZE);
- disk = brd->brd_disk = alloc_disk(max_part);
+ disk = brd->brd_disk = blk_alloc_disk(NUMA_NO_NODE);
if (!disk)
- goto out_free_queue;
+ goto out_free_dev;
+
disk->major = RAMDISK_MAJOR;
disk->first_minor = i * max_part;
+ disk->minors = max_part;
disk->fops = &brd_fops;
disk->private_data = brd;
disk->flags = GENHD_FL_EXT_DEVT;
strlcpy(disk->disk_name, buf, DISK_NAME_LEN);
set_capacity(disk, rd_size * 2);
+
+ /*
+ * This is so fdisk will align partitions on 4k, because of
+ * direct_access API needing 4k alignment, returning a PFN
+ * (This is only a problem on very small devices <= 4M,
+ * otherwise fdisk will align on 1M. Regardless this call
+ * is harmless)
+ */
+ blk_queue_physical_block_size(disk->queue, PAGE_SIZE);
Maybe converting the comment to refer to 'PAGE_SIZE' instead of 4k while
you're at it ...
Post by Christoph Hellwig
/* Tell the block layer that this is not a rotational device */
- blk_queue_flag_set(QUEUE_FLAG_NONROT, brd->brd_queue);
- blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, brd->brd_queue);
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
+ blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);
+ add_disk(disk);
+ list_add_tail(&brd->brd_list, &brd_devices);
- return brd;
+ return 0;
- blk_cleanup_queue(brd->brd_queue);
kfree(brd);
- return NULL;
-}
-
-static void brd_free(struct brd_device *brd)
-{
- put_disk(brd->brd_disk);
- blk_cleanup_queue(brd->brd_queue);
- brd_free_pages(brd);
- kfree(brd);
+ return -ENOMEM;
}
static void brd_probe(dev_t dev)
{
- struct brd_device *brd;
int i = MINOR(dev) / max_part;
+ struct brd_device *brd;
mutex_lock(&brd_devices_mutex);
list_for_each_entry(brd, &brd_devices, brd_list) {
@@ -445,13 +434,7 @@ static void brd_probe(dev_t dev)
goto out_unlock;
}
- brd = brd_alloc(i);
- if (brd) {
- brd->brd_disk->queue = brd->brd_queue;
- add_disk(brd->brd_disk);
- list_add_tail(&brd->brd_list, &brd_devices);
- }
-
+ brd_alloc(i);
mutex_unlock(&brd_devices_mutex);
}
@@ -460,7 +443,9 @@ static void brd_del_one(struct brd_device *brd)
{
list_del(&brd->brd_list);
del_gendisk(brd->brd_disk);
- brd_free(brd);
+ blk_cleanup_disk(brd->brd_disk);
+ brd_free_pages(brd);
+ kfree(brd);
}
static inline void brd_check_and_reset_par(void)
@@ -485,7 +470,7 @@ static inline void brd_check_and_reset_par(void)
static int __init brd_init(void)
{
struct brd_device *brd, *next;
- int i;
+ int err, i;
/*
* brd module now has a feature to instantiate underlying device
@@ -511,22 +496,11 @@ static int __init brd_init(void)
mutex_lock(&brd_devices_mutex);
for (i = 0; i < rd_nr; i++) {
- brd = brd_alloc(i);
- if (!brd)
+ err = brd_alloc(i);
+ if (err)
goto out_free;
- list_add_tail(&brd->brd_list, &brd_devices);
}
- /* point of no return */
-
- list_for_each_entry(brd, &brd_devices, brd_list) {
- /*
- * associate with queue just before adding disk for
- * avoiding to mess up failure path
- */
- brd->brd_disk->queue = brd->brd_queue;
- add_disk(brd->brd_disk);
- }
mutex_unlock(&brd_devices_mutex);
pr_info("brd: module loaded\n");
@@ -535,15 +509,13 @@ static int __init brd_init(void)
debugfs_remove_recursive(brd_debugfs_dir);
- list_for_each_entry_safe(brd, next, &brd_devices, brd_list) {
- list_del(&brd->brd_list);
- brd_free(brd);
- }
+ list_for_each_entry_safe(brd, next, &brd_devices, brd_list)
+ brd_del_one(brd);
mutex_unlock(&brd_devices_mutex);
unregister_blkdev(RAMDISK_MAJOR, "ramdisk");
pr_info("brd: module NOT loaded !!!\n");
- return -ENOMEM;
+ return err;
}
static void __exit brd_exit(void)
Other than that:

Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-24 07:24:56 UTC
Permalink
Post by Hannes Reinecke
Post by Christoph Hellwig
+ /*
+ * This is so fdisk will align partitions on 4k, because of
+ * direct_access API needing 4k alignment, returning a PFN
+ * (This is only a problem on very small devices <= 4M,
+ * otherwise fdisk will align on 1M. Regardless this call
+ * is harmless)
+ */
+ blk_queue_physical_block_size(disk->queue, PAGE_SIZE);
Maybe converting the comment to refer to 'PAGE_SIZE' instead of 4k while
you're at it ...
I really do not want to touch these kinds of unrelated things here.
Christoph Hellwig
2021-05-21 05:50:57 UTC
Permalink
Convert the drbd driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/drbd/drbd_main.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index de463773b530..55234a558e98 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2231,8 +2231,7 @@ void drbd_destroy_device(struct kref *kref)
if (device->bitmap) /* should no longer be there. */
drbd_bm_cleanup(device);
__free_page(device->md_io.page);
- put_disk(device->vdisk);
- blk_cleanup_queue(device->rq_queue);
+ blk_cleanup_disk(device->vdisk);
kfree(device->rs_plan_s);

/* not for_each_connection(connection, resource):
@@ -2701,7 +2700,6 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig
struct drbd_device *device;
struct drbd_peer_device *peer_device, *tmp_peer_device;
struct gendisk *disk;
- struct request_queue *q;
int id;
int vnr = adm_ctx->volume;
enum drbd_ret_code err = ERR_NOMEM;
@@ -2723,29 +2721,26 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig

drbd_init_set_defaults(device);

- q = blk_alloc_queue(NUMA_NO_NODE);
- if (!q)
- goto out_no_q;
- device->rq_queue = q;
-
- disk = alloc_disk(1);
+ disk = blk_alloc_disk(NUMA_NO_NODE);
if (!disk)
goto out_no_disk;
+
device->vdisk = disk;
+ device->rq_queue = disk->queue;

set_disk_ro(disk, true);

- disk->queue = q;
disk->major = DRBD_MAJOR;
disk->first_minor = minor;
+ disk->minors = 1;
disk->fops = &drbd_ops;
sprintf(disk->disk_name, "drbd%d", minor);
disk->private_data = device;

- blk_queue_write_cache(q, true, true);
+ blk_queue_write_cache(disk->queue, true, true);
/* Setting the max_hw_sectors to an odd value of 8kibyte here
This triggers a max_bio_size message upon first attach or connect */
- blk_queue_max_hw_sectors(q, DRBD_MAX_BIO_SIZE_SAFE >> 8);
+ blk_queue_max_hw_sectors(disk->queue, DRBD_MAX_BIO_SIZE_SAFE >> 8);

device->md_io.page = alloc_page(GFP_KERNEL);
if (!device->md_io.page)
@@ -2834,10 +2829,8 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig
out_no_bitmap:
__free_page(device->md_io.page);
out_no_io_page:
- put_disk(disk);
+ blk_cleanup_disk(disk);
out_no_disk:
- blk_cleanup_queue(q);
-out_no_q:
kref_put(&resource->kref, drbd_destroy_resource);
kfree(device);
return err;
--
2.30.2
Hannes Reinecke
2021-05-23 07:59:55 UTC
Permalink
Post by Christoph Hellwig
Convert the drbd driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/block/drbd/drbd_main.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:50:58 UTC
Permalink
Convert the pktcdvd driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/pktcdvd.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index bd3556585122..f69b5c69c2a6 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2711,19 +2711,17 @@ static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev)
pd->write_congestion_off = write_congestion_off;

ret = -ENOMEM;
- disk = alloc_disk(1);
+ disk = blk_alloc_disk(NUMA_NO_NODE);
if (!disk)
goto out_mem;
pd->disk = disk;
disk->major = pktdev_major;
disk->first_minor = idx;
+ disk->minors = 1;
disk->fops = &pktcdvd_ops;
disk->flags = GENHD_FL_REMOVABLE;
strcpy(disk->disk_name, pd->name);
disk->private_data = pd;
- disk->queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!disk->queue)
- goto out_mem2;

pd->pkt_dev = MKDEV(pktdev_major, idx);
ret = pkt_new_dev(pd, dev);
@@ -2746,7 +2744,7 @@ static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev)
return 0;

out_mem2:
- put_disk(disk);
+ blk_cleanup_disk(disk);
out_mem:
mempool_exit(&pd->rb_pool);
kfree(pd);
@@ -2796,8 +2794,7 @@ static int pkt_remove_dev(dev_t pkt_dev)
pkt_dbg(1, pd, "writer unmapped\n");

del_gendisk(pd->disk);
- blk_cleanup_queue(pd->disk->queue);
- put_disk(pd->disk);
+ blk_cleanup_disk(pd->disk);

mempool_exit(&pd->rb_pool);
kfree(pd);
--
2.30.2
Hannes Reinecke
2021-05-23 08:00:31 UTC
Permalink
Post by Christoph Hellwig
Convert the pktcdvd driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/block/pktcdvd.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:50:59 UTC
Permalink
Convert the rsxx driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/rsxx/dev.c | 39 +++++++++++++---------------------
drivers/block/rsxx/rsxx_priv.h | 1 -
2 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/drivers/block/rsxx/dev.c b/drivers/block/rsxx/dev.c
index 9a28322a8cd8..1cc40b0ea761 100644
--- a/drivers/block/rsxx/dev.c
+++ b/drivers/block/rsxx/dev.c
@@ -236,47 +236,40 @@ int rsxx_setup_dev(struct rsxx_cardinfo *card)
return -ENOMEM;
}

- card->queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!card->queue) {
- dev_err(CARD_TO_DEV(card), "Failed queue alloc\n");
- unregister_blkdev(card->major, DRIVER_NAME);
- return -ENOMEM;
- }
-
- card->gendisk = alloc_disk(blkdev_minors);
+ card->gendisk = blk_alloc_disk(blkdev_minors);
if (!card->gendisk) {
dev_err(CARD_TO_DEV(card), "Failed disk alloc\n");
- blk_cleanup_queue(card->queue);
unregister_blkdev(card->major, DRIVER_NAME);
return -ENOMEM;
}

if (card->config_valid) {
blk_size = card->config.data.block_size;
- blk_queue_dma_alignment(card->queue, blk_size - 1);
- blk_queue_logical_block_size(card->queue, blk_size);
+ blk_queue_dma_alignment(card->gendisk->queue, blk_size - 1);
+ blk_queue_logical_block_size(card->gendisk->queue, blk_size);
}

- blk_queue_max_hw_sectors(card->queue, blkdev_max_hw_sectors);
- blk_queue_physical_block_size(card->queue, RSXX_HW_BLK_SIZE);
+ blk_queue_max_hw_sectors(card->gendisk->queue, blkdev_max_hw_sectors);
+ blk_queue_physical_block_size(card->gendisk->queue, RSXX_HW_BLK_SIZE);

- blk_queue_flag_set(QUEUE_FLAG_NONROT, card->queue);
- blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, card->queue);
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, card->gendisk->queue);
+ blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, card->gendisk->queue);
if (rsxx_discard_supported(card)) {
- blk_queue_flag_set(QUEUE_FLAG_DISCARD, card->queue);
- blk_queue_max_discard_sectors(card->queue,
+ blk_queue_flag_set(QUEUE_FLAG_DISCARD, card->gendisk->queue);
+ blk_queue_max_discard_sectors(card->gendisk->queue,
RSXX_HW_BLK_SIZE >> 9);
- card->queue->limits.discard_granularity = RSXX_HW_BLK_SIZE;
- card->queue->limits.discard_alignment = RSXX_HW_BLK_SIZE;
+ card->gendisk->queue->limits.discard_granularity =
+ RSXX_HW_BLK_SIZE;
+ card->gendisk->queue->limits.discard_alignment =
+ RSXX_HW_BLK_SIZE;
}

snprintf(card->gendisk->disk_name, sizeof(card->gendisk->disk_name),
"rsxx%d", card->disk_id);
card->gendisk->major = card->major;
- card->gendisk->first_minor = 0;
+ card->gendisk->minors = blkdev_minors;
card->gendisk->fops = &rsxx_fops;
card->gendisk->private_data = card;
- card->gendisk->queue = card->queue;

return 0;
}
@@ -286,10 +279,8 @@ void rsxx_destroy_dev(struct rsxx_cardinfo *card)
if (!enable_blkdev)
return;

- put_disk(card->gendisk);
+ blk_cleanup_disk(card->gendisk);
card->gendisk = NULL;
-
- blk_cleanup_queue(card->queue);
unregister_blkdev(card->major, DRIVER_NAME);
}

diff --git a/drivers/block/rsxx/rsxx_priv.h b/drivers/block/rsxx/rsxx_priv.h
index 6147977994ff..26c320c0d924 100644
--- a/drivers/block/rsxx/rsxx_priv.h
+++ b/drivers/block/rsxx/rsxx_priv.h
@@ -154,7 +154,6 @@ struct rsxx_cardinfo {
bool bdev_attached;
int disk_id;
int major;
- struct request_queue *queue;
struct gendisk *gendisk;
struct {
/* Used to convert a byte address to a device address. */
--
2.30.2
Hannes Reinecke
2021-05-23 08:01:09 UTC
Permalink
Post by Christoph Hellwig
Convert the rsxx driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/block/rsxx/dev.c | 39 +++++++++++++---------------------
drivers/block/rsxx/rsxx_priv.h | 1 -
2 files changed, 15 insertions(+), 25 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:00 UTC
Permalink
Convert the zram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/zram/zram_drv.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cf8deecc39ef..006416cc4969 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1890,7 +1890,6 @@ static const struct attribute_group *zram_disk_attr_groups[] = {
static int zram_add(void)
{
struct zram *zram;
- struct request_queue *queue;
int ret, device_id;

zram = kzalloc(sizeof(struct zram), GFP_KERNEL);
@@ -1906,27 +1905,20 @@ static int zram_add(void)
#ifdef CONFIG_ZRAM_WRITEBACK
spin_lock_init(&zram->wb_limit_lock);
#endif
- queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!queue) {
- pr_err("Error allocating disk queue for device %d\n",
- device_id);
- ret = -ENOMEM;
- goto out_free_idr;
- }

/* gendisk structure */
- zram->disk = alloc_disk(1);
+ zram->disk = blk_alloc_disk(NUMA_NO_NODE);
if (!zram->disk) {
pr_err("Error allocating disk structure for device %d\n",
device_id);
ret = -ENOMEM;
- goto out_free_queue;
+ goto out_free_idr;
}

zram->disk->major = zram_major;
zram->disk->first_minor = device_id;
+ zram->disk->minors = 1;
zram->disk->fops = &zram_devops;
- zram->disk->queue = queue;
zram->disk->private_data = zram;
snprintf(zram->disk->disk_name, 16, "zram%d", device_id);

@@ -1969,8 +1961,6 @@ static int zram_add(void)
pr_info("Added device: %s\n", zram->disk->disk_name);
return device_id;

-out_free_queue:
- blk_cleanup_queue(queue);
out_free_idr:
idr_remove(&zram_index_idr, device_id);
out_free_dev:
@@ -2000,8 +1990,7 @@ static int zram_remove(struct zram *zram)
pr_info("Removed device: %s\n", zram->disk->disk_name);

del_gendisk(zram->disk);
- blk_cleanup_queue(zram->disk->queue);
- put_disk(zram->disk);
+ blk_cleanup_disk(zram->disk);
kfree(zram);
return 0;
}
--
2.30.2
Hannes Reinecke
2021-05-23 08:01:38 UTC
Permalink
Post by Christoph Hellwig
Convert the zram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/block/zram/zram_drv.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:01 UTC
Permalink
Convert the lightnvm driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/lightnvm/core.c | 23 +++++------------------
1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index e7dc539fc0ac..cf8a75494833 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -305,7 +305,6 @@ static int __nvm_config_extended(struct nvm_dev *dev,
static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
{
struct nvm_ioctl_create_extended e;
- struct request_queue *tqueue;
struct gendisk *tdisk;
struct nvm_tgt_type *tt;
struct nvm_target *t;
@@ -370,23 +369,16 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
goto err_t;
}

- tdisk = alloc_disk(0);
+ tdisk = blk_alloc_disk(dev->q->node);
if (!tdisk) {
ret = -ENOMEM;
goto err_dev;
}

- tqueue = blk_alloc_queue(dev->q->node);
- if (!tqueue) {
- ret = -ENOMEM;
- goto err_disk;
- }
-
strlcpy(tdisk->disk_name, create->tgtname, sizeof(tdisk->disk_name));
tdisk->major = 0;
tdisk->first_minor = 0;
tdisk->fops = tt->bops;
- tdisk->queue = tqueue;

targetdata = tt->init(tgt_dev, tdisk, create->flags);
if (IS_ERR(targetdata)) {
@@ -395,14 +387,14 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
}

tdisk->private_data = targetdata;
- tqueue->queuedata = targetdata;
+ tdisk->queue->queuedata = targetdata;

mdts = (dev->geo.csecs >> 9) * NVM_MAX_VLBA;
if (dev->geo.mdts) {
mdts = min_t(u32, dev->geo.mdts,
(dev->geo.csecs >> 9) * NVM_MAX_VLBA);
}
- blk_queue_max_hw_sectors(tqueue, mdts);
+ blk_queue_max_hw_sectors(tdisk->queue, mdts);

set_capacity(tdisk, tt->capacity(targetdata));
add_disk(tdisk);
@@ -427,10 +419,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
if (tt->exit)
tt->exit(targetdata, true);
err_init:
- blk_cleanup_queue(tqueue);
- tdisk->queue = NULL;
-err_disk:
- put_disk(tdisk);
+ blk_cleanup_disk(tdisk);
err_dev:
nvm_remove_tgt_dev(tgt_dev, 0);
err_t:
@@ -444,10 +433,8 @@ static void __nvm_remove_target(struct nvm_target *t, bool graceful)
{
struct nvm_tgt_type *tt = t->type;
struct gendisk *tdisk = t->disk;
- struct request_queue *q = tdisk->queue;

del_gendisk(tdisk);
- blk_cleanup_queue(q);

if (tt->sysfs_exit)
tt->sysfs_exit(tdisk);
@@ -456,7 +443,7 @@ static void __nvm_remove_target(struct nvm_target *t, bool graceful)
tt->exit(tdisk->private_data, graceful);

nvm_remove_tgt_dev(t->dev, 1);
- put_disk(tdisk);
+ blk_cleanup_disk(tdisk);
module_put(t->type->owner);

list_del(&t->list);
--
2.30.2
Hannes Reinecke
2021-05-23 08:02:21 UTC
Permalink
Post by Christoph Hellwig
Convert the lightnvm driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/lightnvm/core.c | 23 +++++------------------
1 file changed, 5 insertions(+), 18 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:02 UTC
Permalink
Convert the bcache driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/md/bcache/super.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index bea8c4429ae8..185246a0d855 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -890,13 +890,9 @@ static void bcache_device_free(struct bcache_device *d)
if (disk_added)
del_gendisk(disk);

- if (disk->queue)
- blk_cleanup_queue(disk->queue);
-
+ blk_cleanup_disk(disk);
ida_simple_remove(&bcache_device_idx,
first_minor_to_idx(disk->first_minor));
- if (disk_added)
- put_disk(disk);
}

bioset_exit(&d->bio_split);
@@ -946,7 +942,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
goto err;

- d->disk = alloc_disk(BCACHE_MINORS);
+ d->disk = blk_alloc_disk(NUMA_NO_NODE);
if (!d->disk)
goto err;

@@ -955,14 +951,11 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,

d->disk->major = bcache_major;
d->disk->first_minor = idx_to_first_minor(idx);
+ d->disk->minors = BCACHE_MINORS;
d->disk->fops = ops;
d->disk->private_data = d;

- q = blk_alloc_queue(NUMA_NO_NODE);
- if (!q)
- return -ENOMEM;
-
- d->disk->queue = q;
+ q = d->disk->queue;
q->limits.max_hw_sectors = UINT_MAX;
q->limits.max_sectors = UINT_MAX;
q->limits.max_segment_size = UINT_MAX;
--
2.30.2
Coly Li
2021-05-21 06:15:32 UTC
Permalink
Post by Christoph Hellwig
Convert the bcache driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/md/bcache/super.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index bea8c4429ae8..185246a0d855 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -890,13 +890,9 @@ static void bcache_device_free(struct bcache_device *d)
if (disk_added)
del_gendisk(disk);
- if (disk->queue)
- blk_cleanup_queue(disk->queue);
-
+ blk_cleanup_disk(disk);
ida_simple_remove(&bcache_device_idx,
first_minor_to_idx(disk->first_minor));
- if (disk_added)
- put_disk(disk);
The above 2 lines are added on purpose to prevent an refcount
underflow. It is from commit 86da9f736740 ("bcache: fix refcount
underflow in bcache_device_free()").

Maybe add a parameter to blk_cleanup_disk() or checking (disk->flags &
GENHD_FL_UP) inside blk_cleanup_disk() ?

Coly Li
Post by Christoph Hellwig
}
bioset_exit(&d->bio_split);
@@ -946,7 +942,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
goto err;
- d->disk = alloc_disk(BCACHE_MINORS);
+ d->disk = blk_alloc_disk(NUMA_NO_NODE);
if (!d->disk)
goto err;
@@ -955,14 +951,11 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
d->disk->major = bcache_major;
d->disk->first_minor = idx_to_first_minor(idx);
+ d->disk->minors = BCACHE_MINORS;
d->disk->fops = ops;
d->disk->private_data = d;
- q = blk_alloc_queue(NUMA_NO_NODE);
- if (!q)
- return -ENOMEM;
-
- d->disk->queue = q;
+ q = d->disk->queue;
q->limits.max_hw_sectors = UINT_MAX;
q->limits.max_sectors = UINT_MAX;
q->limits.max_segment_size = UINT_MAX;
The rested looks fine to me.

Thanks.

Coly Li
Christoph Hellwig
2021-05-21 06:23:01 UTC
Permalink
Post by Coly Li
The above 2 lines are added on purpose to prevent an refcount
underflow. It is from commit 86da9f736740 ("bcache: fix refcount
underflow in bcache_device_free()").
Maybe add a parameter to blk_cleanup_disk() or checking (disk->flags &
GENHD_FL_UP) inside blk_cleanup_disk() ?
Please take a look at patch 4 in the series.
Hannes Reinecke
2021-05-23 08:04:58 UTC
Permalink
Post by Christoph Hellwig
Convert the bcache driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/md/bcache/super.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Coly Li
2021-05-23 16:20:34 UTC
Permalink
Post by Christoph Hellwig
Convert the bcache driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
Acked-by: Coly Li <***@suse.de>

Thanks.


Coly Li
Post by Christoph Hellwig
---
drivers/md/bcache/super.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index bea8c4429ae8..185246a0d855 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -890,13 +890,9 @@ static void bcache_device_free(struct bcache_device *d)
if (disk_added)
del_gendisk(disk);
- if (disk->queue)
- blk_cleanup_queue(disk->queue);
-
+ blk_cleanup_disk(disk);
ida_simple_remove(&bcache_device_idx,
first_minor_to_idx(disk->first_minor));
- if (disk_added)
- put_disk(disk);
}
bioset_exit(&d->bio_split);
@@ -946,7 +942,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
goto err;
- d->disk = alloc_disk(BCACHE_MINORS);
+ d->disk = blk_alloc_disk(NUMA_NO_NODE);
if (!d->disk)
goto err;
@@ -955,14 +951,11 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
d->disk->major = bcache_major;
d->disk->first_minor = idx_to_first_minor(idx);
+ d->disk->minors = BCACHE_MINORS;
d->disk->fops = ops;
d->disk->private_data = d;
- q = blk_alloc_queue(NUMA_NO_NODE);
- if (!q)
- return -ENOMEM;
-
- d->disk->queue = q;
+ q = d->disk->queue;
q->limits.max_hw_sectors = UINT_MAX;
q->limits.max_sectors = UINT_MAX;
q->limits.max_segment_size = UINT_MAX;
Christoph Hellwig
2021-05-21 05:51:03 UTC
Permalink
Convert the dm driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/md/dm.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index ca2aedd8ee7d..3c7c2d257018 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1801,13 +1801,13 @@ static void cleanup_mapped_device(struct mapped_device *md)
md->disk->private_data = NULL;
spin_unlock(&_minor_lock);
del_gendisk(md->disk);
- put_disk(md->disk);
}

- if (md->queue) {
+ if (md->queue)
dm_queue_destroy_keyslot_manager(md->queue);
- blk_cleanup_queue(md->queue);
- }
+
+ if (md->disk)
+ blk_cleanup_disk(md->disk);

cleanup_srcu_struct(&md->io_barrier);

@@ -1869,13 +1869,10 @@ static struct mapped_device *alloc_dev(int minor)
* established. If request-based table is loaded: blk-mq will
* override accordingly.
*/
- md->queue = blk_alloc_queue(numa_node_id);
- if (!md->queue)
- goto bad;
-
- md->disk = alloc_disk_node(1, md->numa_node_id);
+ md->disk = blk_alloc_disk(md->numa_node_id);
if (!md->disk)
goto bad;
+ md->queue = md->disk->queue;

init_waitqueue_head(&md->wait);
INIT_WORK(&md->work, dm_wq_work);
@@ -1888,6 +1885,7 @@ static struct mapped_device *alloc_dev(int minor)

md->disk->major = _major;
md->disk->first_minor = minor;
+ md->disk->minors = 1;
md->disk->fops = &dm_blk_dops;
md->disk->queue = md->queue;
md->disk->private_data = md;
--
2.30.2
Hannes Reinecke
2021-05-23 08:10:34 UTC
Permalink
Post by Christoph Hellwig
Convert the dm driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/md/dm.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index ca2aedd8ee7d..3c7c2d257018 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1801,13 +1801,13 @@ static void cleanup_mapped_device(struct mapped_device *md)
md->disk->private_data = NULL;
spin_unlock(&_minor_lock);
del_gendisk(md->disk);
- put_disk(md->disk);
}
- if (md->queue) {
+ if (md->queue)
dm_queue_destroy_keyslot_manager(md->queue);
- blk_cleanup_queue(md->queue);
- }
+
+ if (md->disk)
+ blk_cleanup_disk(md->disk);
cleanup_srcu_struct(&md->io_barrier);
Can't these conditionals be merged into a single 'if (md->disk)'?
Eg like:

if (md->disk) {
spin_lock(&_minor_lock);
md->disk->private_data = NULL;
spin_unlock(&_minor_lock);
del_gendisk(md->disk);
dm_queue_destroy_keyslot_manager(md->queue);
blk_cleanup_disk(md->queue);
}

We're now always allocating 'md->disk' and 'md->queue' together,
so how can we end up in a situation where one is set without the other?
Post by Christoph Hellwig
@@ -1869,13 +1869,10 @@ static struct mapped_device *alloc_dev(int minor)
* established. If request-based table is loaded: blk-mq will
* override accordingly.
*/
- md->queue = blk_alloc_queue(numa_node_id);
- if (!md->queue)
- goto bad;
-
- md->disk = alloc_disk_node(1, md->numa_node_id);
+ md->disk = blk_alloc_disk(md->numa_node_id);
if (!md->disk)
goto bad;
+ md->queue = md->disk->queue;
init_waitqueue_head(&md->wait);
INIT_WORK(&md->work, dm_wq_work);
@@ -1888,6 +1885,7 @@ static struct mapped_device *alloc_dev(int minor)
md->disk->major = _major;
md->disk->first_minor = minor;
+ md->disk->minors = 1;
md->disk->fops = &dm_blk_dops;
md->disk->queue = md->queue;
md->disk->private_data = md;
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-24 07:25:57 UTC
Permalink
Post by Hannes Reinecke
Can't these conditionals be merged into a single 'if (md->disk)'?
if (md->disk) {
spin_lock(&_minor_lock);
md->disk->private_data = NULL;
spin_unlock(&_minor_lock);
del_gendisk(md->disk);
dm_queue_destroy_keyslot_manager(md->queue);
blk_cleanup_disk(md->queue);
}
We're now always allocating 'md->disk' and 'md->queue' together,
so how can we end up in a situation where one is set without the other?
I guess we could do that, not sure it is worth the churn, though.
Christoph Hellwig
2021-05-21 05:51:04 UTC
Permalink
Convert the md driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/md/md.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 49f897fbb89b..d806be8cc210 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5598,12 +5598,10 @@ static void md_free(struct kobject *ko)
if (mddev->sysfs_level)
sysfs_put(mddev->sysfs_level);

- if (mddev->gendisk)
+ if (mddev->gendisk) {
del_gendisk(mddev->gendisk);
- if (mddev->queue)
- blk_cleanup_queue(mddev->queue);
- if (mddev->gendisk)
- put_disk(mddev->gendisk);
+ blk_cleanup_disk(mddev->gendisk);
+ }
percpu_ref_exit(&mddev->writes_pending);

bioset_exit(&mddev->bio_set);
@@ -5711,20 +5709,13 @@ static int md_alloc(dev_t dev, char *name)
goto abort;

error = -ENOMEM;
- mddev->queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!mddev->queue)
+ disk = blk_alloc_disk(NUMA_NO_NODE);
+ if (!disk)
goto abort;

- blk_set_stacking_limits(&mddev->queue->limits);
-
- disk = alloc_disk(1 << shift);
- if (!disk) {
- blk_cleanup_queue(mddev->queue);
- mddev->queue = NULL;
- goto abort;
- }
disk->major = MAJOR(mddev->unit);
disk->first_minor = unit << shift;
+ disk->minors = 1 << shift;
if (name)
strcpy(disk->disk_name, name);
else if (partitioned)
@@ -5733,7 +5724,9 @@ static int md_alloc(dev_t dev, char *name)
sprintf(disk->disk_name, "md%d", unit);
disk->fops = &md_fops;
disk->private_data = mddev;
- disk->queue = mddev->queue;
+
+ mddev->queue = disk->queue;
+ blk_set_stacking_limits(&mddev->queue->limits);
blk_queue_write_cache(mddev->queue, true, true);
/* Allow extended partitions. This makes the
* 'mdp' device redundant, but we can't really
--
2.30.2
Hannes Reinecke
2021-05-23 08:12:49 UTC
Permalink
Post by Christoph Hellwig
Convert the md driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/md/md.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 49f897fbb89b..d806be8cc210 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5598,12 +5598,10 @@ static void md_free(struct kobject *ko)
if (mddev->sysfs_level)
sysfs_put(mddev->sysfs_level);
- if (mddev->gendisk)
+ if (mddev->gendisk) {
del_gendisk(mddev->gendisk);
- if (mddev->queue)
- blk_cleanup_queue(mddev->queue);
- if (mddev->gendisk)
- put_disk(mddev->gendisk);
+ blk_cleanup_disk(mddev->gendisk);
+ }
percpu_ref_exit(&mddev->writes_pending);
bioset_exit(&mddev->bio_set);
@@ -5711,20 +5709,13 @@ static int md_alloc(dev_t dev, char *name)
goto abort;
error = -ENOMEM;
- mddev->queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!mddev->queue)
+ disk = blk_alloc_disk(NUMA_NO_NODE);
+ if (!disk)
goto abort;
- blk_set_stacking_limits(&mddev->queue->limits);
-
- disk = alloc_disk(1 << shift);
- if (!disk) {
- blk_cleanup_queue(mddev->queue);
- mddev->queue = NULL;
- goto abort;
- }
disk->major = MAJOR(mddev->unit);
disk->first_minor = unit << shift;
+ disk->minors = 1 << shift;
if (name)
strcpy(disk->disk_name, name);
else if (partitioned)
@@ -5733,7 +5724,9 @@ static int md_alloc(dev_t dev, char *name)
sprintf(disk->disk_name, "md%d", unit);
disk->fops = &md_fops;
disk->private_data = mddev;
- disk->queue = mddev->queue;
+
+ mddev->queue = disk->queue;
+ blk_set_stacking_limits(&mddev->queue->limits);
blk_queue_write_cache(mddev->queue, true, true);
/* Allow extended partitions. This makes the
* 'mdp' device redundant, but we can't really
Wouldn't it make sense to introduce a helper 'blk_queue_from_disk()' or
somesuch to avoid having to keep an explicit 'queue' pointer?


Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-24 07:26:42 UTC
Permalink
Post by Hannes Reinecke
Post by Christoph Hellwig
+ blk_set_stacking_limits(&mddev->queue->limits);
blk_queue_write_cache(mddev->queue, true, true);
/* Allow extended partitions. This makes the
* 'mdp' device redundant, but we can't really
Wouldn't it make sense to introduce a helper 'blk_queue_from_disk()' or
somesuch to avoid having to keep an explicit 'queue' pointer?
My rought plan is that a few series from now bio based drivers will
never directly deal with the request_queue at all.
Hannes Reinecke
2021-05-24 08:27:16 UTC
Permalink
Post by Christoph Hellwig
Post by Hannes Reinecke
Post by Christoph Hellwig
+ blk_set_stacking_limits(&mddev->queue->limits);
blk_queue_write_cache(mddev->queue, true, true);
/* Allow extended partitions. This makes the
* 'mdp' device redundant, but we can't really
Wouldn't it make sense to introduce a helper 'blk_queue_from_disk()' or
somesuch to avoid having to keep an explicit 'queue' pointer?
My rought plan is that a few series from now bio based drivers will
never directly deal with the request_queue at all.
Go for it.

Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:05 UTC
Permalink
Convert the nvdimm-blk driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/nvdimm/blk.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
index 7ba446d224fb..088d3dd6f6fa 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -228,48 +228,34 @@ static const struct block_device_operations nd_blk_fops = {
.submit_bio = nd_blk_submit_bio,
};

-static void nd_blk_release_queue(void *q)
-{
- blk_cleanup_queue(q);
-}
-
static void nd_blk_release_disk(void *disk)
{
del_gendisk(disk);
- put_disk(disk);
+ blk_cleanup_disk(disk);
}

static int nsblk_attach_disk(struct nd_namespace_blk *nsblk)
{
struct device *dev = &nsblk->common.dev;
resource_size_t available_disk_size;
- struct request_queue *q;
struct gendisk *disk;
u64 internal_nlba;

internal_nlba = div_u64(nsblk->size, nsblk_internal_lbasize(nsblk));
available_disk_size = internal_nlba * nsblk_sector_size(nsblk);

- q = blk_alloc_queue(NUMA_NO_NODE);
- if (!q)
- return -ENOMEM;
- if (devm_add_action_or_reset(dev, nd_blk_release_queue, q))
- return -ENOMEM;
-
- blk_queue_max_hw_sectors(q, UINT_MAX);
- blk_queue_logical_block_size(q, nsblk_sector_size(nsblk));
- blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
-
- disk = alloc_disk(0);
+ disk = blk_alloc_disk(NUMA_NO_NODE);
if (!disk)
return -ENOMEM;

- disk->first_minor = 0;
disk->fops = &nd_blk_fops;
- disk->queue = q;
disk->private_data = nsblk;
nvdimm_namespace_disk_name(&nsblk->common, disk->disk_name);

+ blk_queue_max_hw_sectors(disk->queue, UINT_MAX);
+ blk_queue_logical_block_size(disk->queue, nsblk_sector_size(nsblk));
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
+
if (devm_add_action_or_reset(dev, nd_blk_release_disk, disk))
return -ENOMEM;
--
2.30.2
Hannes Reinecke
2021-05-23 08:13:31 UTC
Permalink
Post by Christoph Hellwig
Convert the nvdimm-blk driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.
---
drivers/nvdimm/blk.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:06 UTC
Permalink
Convert the nvdimm-btt driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/nvdimm/btt.c | 24 +++++++-----------------
drivers/nvdimm/btt.h | 2 --
2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 1741a7b0b30f..92dec4952297 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1521,34 +1521,25 @@ static int btt_blk_init(struct btt *btt)
struct nd_btt *nd_btt = btt->nd_btt;
struct nd_namespace_common *ndns = nd_btt->ndns;

- /* create a new disk and request queue for btt */
- btt->btt_queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!btt->btt_queue)
+ btt->btt_disk = blk_alloc_disk(NUMA_NO_NODE);
+ if (!btt->btt_disk)
return -ENOMEM;

- btt->btt_disk = alloc_disk(0);
- if (!btt->btt_disk) {
- blk_cleanup_queue(btt->btt_queue);
- return -ENOMEM;
- }
-
nvdimm_namespace_disk_name(ndns, btt->btt_disk->disk_name);
btt->btt_disk->first_minor = 0;
btt->btt_disk->fops = &btt_fops;
btt->btt_disk->private_data = btt;
- btt->btt_disk->queue = btt->btt_queue;

- blk_queue_logical_block_size(btt->btt_queue, btt->sector_size);
- blk_queue_max_hw_sectors(btt->btt_queue, UINT_MAX);
- blk_queue_flag_set(QUEUE_FLAG_NONROT, btt->btt_queue);
+ blk_queue_logical_block_size(btt->btt_disk->queue, btt->sector_size);
+ blk_queue_max_hw_sectors(btt->btt_disk->queue, UINT_MAX);
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, btt->btt_disk->queue);

if (btt_meta_size(btt)) {
int rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt));

if (rc) {
del_gendisk(btt->btt_disk);
- put_disk(btt->btt_disk);
- blk_cleanup_queue(btt->btt_queue);
+ blk_cleanup_disk(btt->btt_disk);
return rc;
}
}
@@ -1563,8 +1554,7 @@ static int btt_blk_init(struct btt *btt)
static void btt_blk_cleanup(struct btt *btt)
{
del_gendisk(btt->btt_disk);
- put_disk(btt->btt_disk);
- blk_cleanup_queue(btt->btt_queue);
+ blk_cleanup_disk(btt->btt_disk);
}

/**
diff --git a/drivers/nvdimm/btt.h b/drivers/nvdimm/btt.h
index aa53e0b769bd..0c76c0333f6e 100644
--- a/drivers/nvdimm/btt.h
+++ b/drivers/nvdimm/btt.h
@@ -201,7 +201,6 @@ struct badblocks;
/**
* struct btt - handle for a BTT instance
* @btt_disk: Pointer to the gendisk for BTT device
- * @btt_queue: Pointer to the request queue for the BTT device
* @arena_list: Head of the list of arenas
* @debugfs_dir: Debugfs dentry
* @nd_btt: Parent nd_btt struct
@@ -219,7 +218,6 @@ struct badblocks;
*/
struct btt {
struct gendisk *btt_disk;
- struct request_queue *btt_queue;
struct list_head arena_list;
struct dentry *debugfs_dir;
struct nd_btt *nd_btt;
--
2.30.2
Hannes Reinecke
2021-05-23 08:14:14 UTC
Permalink
Post by Christoph Hellwig
Convert the nvdimm-btt driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.
---
drivers/nvdimm/btt.c | 24 +++++++-----------------
drivers/nvdimm/btt.h | 2 --
2 files changed, 7 insertions(+), 19 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:07 UTC
Permalink
Convert the nvdimm-pmem driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/nvdimm/pmem.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 968b8483c763..9fcd05084564 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -338,7 +338,7 @@ static void pmem_pagemap_cleanup(struct dev_pagemap *pgmap)
struct request_queue *q =
container_of(pgmap->ref, struct request_queue, q_usage_counter);

- blk_cleanup_queue(q);
+ blk_cleanup_disk(queue_to_disk(q));
}

static void pmem_release_queue(void *pgmap)
@@ -361,7 +361,6 @@ static void pmem_release_disk(void *__pmem)
kill_dax(pmem->dax_dev);
put_dax(pmem->dax_dev);
del_gendisk(pmem->disk);
- put_disk(pmem->disk);
}

static const struct dev_pagemap_ops fsdax_pagemap_ops = {
@@ -422,10 +421,12 @@ static int pmem_attach_disk(struct device *dev,
return -EBUSY;
}

- q = blk_alloc_queue(dev_to_node(dev));
- if (!q)
+ disk = blk_alloc_disk(nid);
+ if (!disk)
return -ENOMEM;
+ q = disk->queue;

+ pmem->disk = disk;
pmem->pfn_flags = PFN_DEV;
pmem->pgmap.ref = &q->q_usage_counter;
if (is_nd_pfn(dev)) {
@@ -470,11 +471,6 @@ static int pmem_attach_disk(struct device *dev,
if (pmem->pfn_flags & PFN_MAP)
blk_queue_flag_set(QUEUE_FLAG_DAX, q);

- disk = alloc_disk_node(0, nid);
- if (!disk)
- return -ENOMEM;
- pmem->disk = disk;
-
disk->fops = &pmem_fops;
disk->queue = q;
disk->private_data = pmem;
@@ -490,7 +486,6 @@ static int pmem_attach_disk(struct device *dev,
flags = DAXDEV_F_SYNC;
dax_dev = alloc_dax(pmem, disk->disk_name, &pmem_dax_ops, flags);
if (IS_ERR(dax_dev)) {
- put_disk(disk);
return PTR_ERR(dax_dev);
}
dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
--
2.30.2
Hannes Reinecke
2021-05-23 08:14:52 UTC
Permalink
Post by Christoph Hellwig
Convert the nvdimm-pmem driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.
---
drivers/nvdimm/pmem.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Dan Williams
2021-06-07 04:43:03 UTC
Permalink
[ add Sachin who reported this commit in -next ]
Post by Christoph Hellwig
Convert the nvdimm-pmem driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.
---
drivers/nvdimm/pmem.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 968b8483c763..9fcd05084564 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -338,7 +338,7 @@ static void pmem_pagemap_cleanup(struct dev_pagemap *pgmap)
struct request_queue *q =
container_of(pgmap->ref, struct request_queue, q_usage_counter);
- blk_cleanup_queue(q);
+ blk_cleanup_disk(queue_to_disk(q));
This is broken. This comes after del_gendisk() which means the queue
device is no longer associated with its disk parent. Perhaps @pmem
could be stashed in pgmap->owner and then this can use pmem->disk? Not
see any other readily available ways to get back to the disk from here
after del_gendisk().

Christoph Hellwig
2021-05-21 05:51:08 UTC
Permalink
Convert the nvme-multipath driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/nvdimm/pmem.c | 1 -
drivers/nvme/host/multipath.c | 45 ++++++++++-------------------------
2 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 9fcd05084564..31f3c4bd6f72 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -472,7 +472,6 @@ static int pmem_attach_disk(struct device *dev,
blk_queue_flag_set(QUEUE_FLAG_DAX, q);

disk->fops = &pmem_fops;
- disk->queue = q;
disk->private_data = pmem;
nvdimm_namespace_disk_name(ndns, disk->disk_name);
set_capacity(disk, (pmem->size - pmem->pfn_pad - pmem->data_offset)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a5d02f236cca..b5fbdb416022 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -427,7 +427,6 @@ static void nvme_requeue_work(struct work_struct *work)

int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
{
- struct request_queue *q;
bool vwc = false;

mutex_init(&head->lock);
@@ -443,33 +442,24 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
if (!(ctrl->subsys->cmic & NVME_CTRL_CMIC_MULTI_CTRL) || !multipath)
return 0;

- q = blk_alloc_queue(ctrl->numa_node);
- if (!q)
- goto out;
- blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
- /* set to a default value for 512 until disk is validated */
- blk_queue_logical_block_size(q, 512);
- blk_set_stacking_limits(&q->limits);
-
- /* we need to propagate up the VMC settings */
- if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
- vwc = true;
- blk_queue_write_cache(q, vwc, vwc);
-
- head->disk = alloc_disk(0);
+ head->disk = blk_alloc_disk(ctrl->numa_node);
if (!head->disk)
- goto out_cleanup_queue;
+ return -ENOMEM;
head->disk->fops = &nvme_ns_head_ops;
head->disk->private_data = head;
- head->disk->queue = q;
sprintf(head->disk->disk_name, "nvme%dn%d",
ctrl->subsys->instance, head->instance);
- return 0;

-out_cleanup_queue:
- blk_cleanup_queue(q);
-out:
- return -ENOMEM;
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, head->disk->queue);
+ /* set to a default value of 512 until the disk is validated */
+ blk_queue_logical_block_size(head->disk->queue, 512);
+ blk_set_stacking_limits(&head->disk->queue->limits);
+
+ /* we need to propagate up the VMC settings */
+ if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
+ vwc = true;
+ blk_queue_write_cache(head->disk->queue, vwc, vwc);
+ return 0;
}

static void nvme_mpath_set_live(struct nvme_ns *ns)
@@ -768,16 +758,7 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
/* make sure all pending bios are cleaned up */
kblockd_schedule_work(&head->requeue_work);
flush_work(&head->requeue_work);
- blk_cleanup_queue(head->disk->queue);
- if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
- /*
- * if device_add_disk wasn't called, prevent
- * disk release to put a bogus reference on the
- * request queue
- */
- head->disk->queue = NULL;
- }
- put_disk(head->disk);
+ blk_cleanup_disk(head->disk);
}

void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)
--
2.30.2
Hannes Reinecke
2021-05-23 08:20:27 UTC
Permalink
Post by Christoph Hellwig
Convert the nvme-multipath driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.
---
drivers/nvdimm/pmem.c | 1 -
drivers/nvme/host/multipath.c | 45 ++++++++++-------------------------
2 files changed, 13 insertions(+), 33 deletions(-)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 9fcd05084564..31f3c4bd6f72 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -472,7 +472,6 @@ static int pmem_attach_disk(struct device *dev,
blk_queue_flag_set(QUEUE_FLAG_DAX, q);
disk->fops = &pmem_fops;
- disk->queue = q;
disk->private_data = pmem;
nvdimm_namespace_disk_name(ndns, disk->disk_name);
set_capacity(disk, (pmem->size - pmem->pfn_pad - pmem->data_offset)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a5d02f236cca..b5fbdb416022 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -427,7 +427,6 @@ static void nvme_requeue_work(struct work_struct *work)
int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
{
- struct request_queue *q;
bool vwc = false;
mutex_init(&head->lock);
@@ -443,33 +442,24 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
if (!(ctrl->subsys->cmic & NVME_CTRL_CMIC_MULTI_CTRL) || !multipath)
return 0;
- q = blk_alloc_queue(ctrl->numa_node);
- if (!q)
- goto out;
- blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
- /* set to a default value for 512 until disk is validated */
- blk_queue_logical_block_size(q, 512);
- blk_set_stacking_limits(&q->limits);
-
- /* we need to propagate up the VMC settings */
- if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
- vwc = true;
- blk_queue_write_cache(q, vwc, vwc);
-
- head->disk = alloc_disk(0);
+ head->disk = blk_alloc_disk(ctrl->numa_node);
if (!head->disk)
- goto out_cleanup_queue;
+ return -ENOMEM;
head->disk->fops = &nvme_ns_head_ops;
head->disk->private_data = head;
- head->disk->queue = q;
sprintf(head->disk->disk_name, "nvme%dn%d",
ctrl->subsys->instance, head->instance);
- return 0;
- blk_cleanup_queue(q);
- return -ENOMEM;
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, head->disk->queue);
+ /* set to a default value of 512 until the disk is validated */
+ blk_queue_logical_block_size(head->disk->queue, 512);
+ blk_set_stacking_limits(&head->disk->queue->limits);
+
+ /* we need to propagate up the VMC settings */
+ if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
+ vwc = true;
+ blk_queue_write_cache(head->disk->queue, vwc, vwc);
+ return 0;
}
static void nvme_mpath_set_live(struct nvme_ns *ns)
@@ -768,16 +758,7 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
/* make sure all pending bios are cleaned up */
kblockd_schedule_work(&head->requeue_work);
flush_work(&head->requeue_work);
- blk_cleanup_queue(head->disk->queue);
- if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
- /*
- * if device_add_disk wasn't called, prevent
- * disk release to put a bogus reference on the
- * request queue
- */
- head->disk->queue = NULL;
- }
- put_disk(head->disk);
+ blk_cleanup_disk(head->disk);
}
void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)
What about the check for GENHD_FL_UP a bit further up in line 766?
Can this still happen with the new allocation scheme, ie is there still
a difference in lifetime between ->disk and ->disk->queue?

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-24 07:29:50 UTC
Permalink
Post by Hannes Reinecke
What about the check for GENHD_FL_UP a bit further up in line 766?
Can this still happen with the new allocation scheme, ie is there still a
difference in lifetime between ->disk and ->disk->queue?
Yes, nvme_free_ns_head can still be called before device_add_disk was
called for an allocated nshead gendisk during error handling of the
setup path. There is still a difference in the lifetime in that they
are separately refcounted, but it does not matter to the driver.
Christoph Hellwig
2021-05-21 05:51:09 UTC
Permalink
Convert the nfblock driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
arch/m68k/emu/nfblock.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/m68k/emu/nfblock.c b/arch/m68k/emu/nfblock.c
index ba808543161a..9a8394e96388 100644
--- a/arch/m68k/emu/nfblock.c
+++ b/arch/m68k/emu/nfblock.c
@@ -55,7 +55,6 @@ struct nfhd_device {
int id;
u32 blocks, bsize;
int bshift;
- struct request_queue *queue;
struct gendisk *disk;
};

@@ -119,32 +118,24 @@ static int __init nfhd_init_one(int id, u32 blocks, u32 bsize)
dev->bsize = bsize;
dev->bshift = ffs(bsize) - 10;

- dev->queue = blk_alloc_queue(NUMA_NO_NODE);
- if (dev->queue == NULL)
- goto free_dev;
-
- blk_queue_logical_block_size(dev->queue, bsize);
-
- dev->disk = alloc_disk(16);
+ dev->disk = blk_alloc_disk(NUMA_NO_NODE);
if (!dev->disk)
- goto free_queue;
+ goto free_dev;

dev->disk->major = major_num;
dev->disk->first_minor = dev_id * 16;
+ dev->disk->minors = 16;
dev->disk->fops = &nfhd_ops;
dev->disk->private_data = dev;
sprintf(dev->disk->disk_name, "nfhd%u", dev_id);
set_capacity(dev->disk, (sector_t)blocks * (bsize / 512));
- dev->disk->queue = dev->queue;
-
+ blk_queue_logical_block_size(dev->disk->queue, bsize);
add_disk(dev->disk);

list_add_tail(&dev->list, &nfhd_list);

return 0;

-free_queue:
- blk_cleanup_queue(dev->queue);
free_dev:
kfree(dev);
out:
@@ -186,8 +177,7 @@ static void __exit nfhd_exit(void)
list_for_each_entry_safe(dev, next, &nfhd_list, list) {
list_del(&dev->list);
del_gendisk(dev->disk);
- put_disk(dev->disk);
- blk_cleanup_queue(dev->queue);
+ blk_cleanup_disk(dev->disk);
kfree(dev);
}
unregister_blkdev(major_num, "nfhd");
--
2.30.2
Geert Uytterhoeven
2021-05-21 08:37:56 UTC
Permalink
Post by Christoph Hellwig
Convert the nfblock driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
Acked-by: Geert Uytterhoeven <***@linux-m68k.org>

Gr{oetje,eeting}s,

Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hannes Reinecke
2021-05-23 08:21:08 UTC
Permalink
Post by Christoph Hellwig
Convert the nfblock driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
arch/m68k/emu/nfblock.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:10 UTC
Permalink
Convert the simdisk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
arch/xtensa/platforms/iss/simdisk.c | 29 +++++++----------------------
1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c
index fc09be7b1347..3cdfa00738e0 100644
--- a/arch/xtensa/platforms/iss/simdisk.c
+++ b/arch/xtensa/platforms/iss/simdisk.c
@@ -27,7 +27,6 @@
struct simdisk {
const char *filename;
spinlock_t lock;
- struct request_queue *queue;
struct gendisk *gd;
struct proc_dir_entry *procfile;
int users;
@@ -266,21 +265,13 @@ static int __init simdisk_setup(struct simdisk *dev, int which,
spin_lock_init(&dev->lock);
dev->users = 0;

- dev->queue = blk_alloc_queue(NUMA_NO_NODE);
- if (dev->queue == NULL) {
- pr_err("blk_alloc_queue failed\n");
- goto out_alloc_queue;
- }
-
- dev->gd = alloc_disk(SIMDISK_MINORS);
- if (dev->gd == NULL) {
- pr_err("alloc_disk failed\n");
- goto out_alloc_disk;
- }
+ dev->gd = blk_alloc_disk(NUMA_NO_NODE);
+ if (!dev->gd)
+ return -ENOMEM;
dev->gd->major = simdisk_major;
dev->gd->first_minor = which;
+ dev->gd->minors = SIMDISK_MINORS;
dev->gd->fops = &simdisk_ops;
- dev->gd->queue = dev->queue;
dev->gd->private_data = dev;
snprintf(dev->gd->disk_name, 32, "simdisk%d", which);
set_capacity(dev->gd, 0);
@@ -288,12 +279,6 @@ static int __init simdisk_setup(struct simdisk *dev, int which,

dev->procfile = proc_create_data(tmp, 0644, procdir, &simdisk_proc_ops, dev);
return 0;
-
-out_alloc_disk:
- blk_cleanup_queue(dev->queue);
- dev->queue = NULL;
-out_alloc_queue:
- return -ENOMEM;
}

static int __init simdisk_init(void)
@@ -343,10 +328,10 @@ static void simdisk_teardown(struct simdisk *dev, int which,
char tmp[2] = { '0' + which, 0 };

simdisk_detach(dev);
- if (dev->gd)
+ if (dev->gd) {
del_gendisk(dev->gd);
- if (dev->queue)
- blk_cleanup_queue(dev->queue);
+ blk_cleanup_disk(dev->gd);
+ }
remove_proc_entry(tmp, procdir);
}
--
2.30.2
Hannes Reinecke
2021-05-23 08:22:01 UTC
Permalink
Post by Christoph Hellwig
Convert the simdisk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
arch/xtensa/platforms/iss/simdisk.c | 29 +++++++----------------------
1 file changed, 7 insertions(+), 22 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:11 UTC
Permalink
Convert the n64cart driver to use the blk_alloc_disk helper to simplify
gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/n64cart.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 3dae4b631dea..7b4dd10af9ec 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -132,14 +132,10 @@ static int __init n64cart_probe(struct platform_device *pdev)
if (!reg_base)
return -EINVAL;

- disk = alloc_disk(0);
+ disk = blk_alloc_disk(NUMA_NO_NODE);
if (!disk)
return -ENOMEM;

- disk->queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!disk->queue)
- return -ENOMEM;
-
disk->first_minor = 0;
disk->flags = GENHD_FL_NO_PART_SCAN;
disk->fops = &n64cart_fops;
--
2.30.2
Hannes Reinecke
2021-05-23 08:22:37 UTC
Permalink
Post by Christoph Hellwig
Convert the n64cart driver to use the blk_alloc_disk helper to simplify
gendisk and request_queue allocation.
---
drivers/block/n64cart.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:12 UTC
Permalink
Convert the ps3vram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/ps3vram.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index 1d738999fb69..7fbf469651c4 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -67,7 +67,6 @@ struct ps3vram_cache {
};

struct ps3vram_priv {
- struct request_queue *queue;
struct gendisk *gendisk;

u64 size;
@@ -613,7 +612,6 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
{
struct ps3vram_priv *priv;
int error, status;
- struct request_queue *queue;
struct gendisk *gendisk;
u64 ddr_size, ddr_lpar, ctrl_lpar, info_lpar, reports_lpar,
reports_size, xdr_lpar;
@@ -736,33 +734,23 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)

ps3vram_proc_init(dev);

- queue = blk_alloc_queue(NUMA_NO_NODE);
- if (!queue) {
- dev_err(&dev->core, "blk_alloc_queue failed\n");
- error = -ENOMEM;
- goto out_cache_cleanup;
- }
-
- priv->queue = queue;
- blk_queue_max_segments(queue, BLK_MAX_SEGMENTS);
- blk_queue_max_segment_size(queue, BLK_MAX_SEGMENT_SIZE);
- blk_queue_max_hw_sectors(queue, BLK_SAFE_MAX_SECTORS);
-
- gendisk = alloc_disk(1);
+ gendisk = blk_alloc_disk(NUMA_NO_NODE);
if (!gendisk) {
- dev_err(&dev->core, "alloc_disk failed\n");
+ dev_err(&dev->core, "blk_alloc_disk failed\n");
error = -ENOMEM;
- goto fail_cleanup_queue;
+ goto out_cache_cleanup;
}

priv->gendisk = gendisk;
gendisk->major = ps3vram_major;
- gendisk->first_minor = 0;
+ gendisk->minors = 1;
gendisk->fops = &ps3vram_fops;
- gendisk->queue = queue;
gendisk->private_data = dev;
strlcpy(gendisk->disk_name, DEVICE_NAME, sizeof(gendisk->disk_name));
set_capacity(gendisk, priv->size >> 9);
+ blk_queue_max_segments(gendisk->queue, BLK_MAX_SEGMENTS);
+ blk_queue_max_segment_size(gendisk->queue, BLK_MAX_SEGMENT_SIZE);
+ blk_queue_max_hw_sectors(gendisk->queue, BLK_SAFE_MAX_SECTORS);

dev_info(&dev->core, "%s: Using %llu MiB of GPU memory\n",
gendisk->disk_name, get_capacity(gendisk) >> 11);
@@ -770,8 +758,6 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
device_add_disk(&dev->core, gendisk, NULL);
return 0;

-fail_cleanup_queue:
- blk_cleanup_queue(queue);
out_cache_cleanup:
remove_proc_entry(DEVICE_NAME, NULL);
ps3vram_cache_cleanup(dev);
@@ -802,8 +788,7 @@ static void ps3vram_remove(struct ps3_system_bus_device *dev)
struct ps3vram_priv *priv = ps3_system_bus_get_drvdata(dev);

del_gendisk(priv->gendisk);
- put_disk(priv->gendisk);
- blk_cleanup_queue(priv->queue);
+ blk_cleanup_disk(priv->gendisk);
remove_proc_entry(DEVICE_NAME, NULL);
ps3vram_cache_cleanup(dev);
iounmap(priv->reports);
--
2.30.2
Hannes Reinecke
2021-05-23 08:23:17 UTC
Permalink
Post by Christoph Hellwig
Convert the ps3vram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/block/ps3vram.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:13 UTC
Permalink
Convert the dcssblk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/s390/block/dcssblk.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index da33cb4cba28..7faa56399999 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -90,7 +90,6 @@ struct dcssblk_dev_info {
int segment_type;
unsigned char save_pending;
unsigned char is_shared;
- struct request_queue *dcssblk_queue;
int num_of_segments;
struct list_head seg_list;
struct dax_device *dax_dev;
@@ -429,9 +428,7 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
kill_dax(dev_info->dax_dev);
put_dax(dev_info->dax_dev);
del_gendisk(dev_info->gd);
- blk_cleanup_queue(dev_info->dcssblk_queue);
- dev_info->gd->queue = NULL;
- put_disk(dev_info->gd);
+ blk_cleanup_disk(dev_info->gd);
up_write(&dcssblk_devices_sem);

if (device_remove_file_self(dev, attr)) {
@@ -644,18 +641,17 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
dev_info->dev.release = dcssblk_release_segment;
dev_info->dev.groups = dcssblk_dev_attr_groups;
INIT_LIST_HEAD(&dev_info->lh);
- dev_info->gd = alloc_disk(DCSSBLK_MINORS_PER_DISK);
+ dev_info->gd = blk_alloc_disk(NUMA_NO_NODE);
if (dev_info->gd == NULL) {
rc = -ENOMEM;
goto seg_list_del;
}
dev_info->gd->major = dcssblk_major;
+ dev_info->gd->minors = DCSSBLK_MINORS_PER_DISK;
dev_info->gd->fops = &dcssblk_devops;
- dev_info->dcssblk_queue = blk_alloc_queue(NUMA_NO_NODE);
- dev_info->gd->queue = dev_info->dcssblk_queue;
dev_info->gd->private_data = dev_info;
- blk_queue_logical_block_size(dev_info->dcssblk_queue, 4096);
- blk_queue_flag_set(QUEUE_FLAG_DAX, dev_info->dcssblk_queue);
+ blk_queue_logical_block_size(dev_info->gd->queue, 4096);
+ blk_queue_flag_set(QUEUE_FLAG_DAX, dev_info->gd->queue);

seg_byte_size = (dev_info->end - dev_info->start + 1);
set_capacity(dev_info->gd, seg_byte_size >> 9); // size in sectors
@@ -719,9 +715,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char

put_dev:
list_del(&dev_info->lh);
- blk_cleanup_queue(dev_info->dcssblk_queue);
- dev_info->gd->queue = NULL;
- put_disk(dev_info->gd);
+ blk_cleanup_disk(dev_info->gd);
list_for_each_entry(seg_info, &dev_info->seg_list, lh) {
segment_unload(seg_info->segment_name);
}
@@ -731,9 +725,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
dev_list_del:
list_del(&dev_info->lh);
release_gd:
- blk_cleanup_queue(dev_info->dcssblk_queue);
- dev_info->gd->queue = NULL;
- put_disk(dev_info->gd);
+ blk_cleanup_disk(dev_info->gd);
up_write(&dcssblk_devices_sem);
seg_list_del:
if (dev_info == NULL)
@@ -801,9 +793,7 @@ dcssblk_remove_store(struct device *dev, struct device_attribute *attr, const ch
kill_dax(dev_info->dax_dev);
put_dax(dev_info->dax_dev);
del_gendisk(dev_info->gd);
- blk_cleanup_queue(dev_info->dcssblk_queue);
- dev_info->gd->queue = NULL;
- put_disk(dev_info->gd);
+ blk_cleanup_disk(dev_info->gd);

/* unload all related segments */
list_for_each_entry(entry, &dev_info->seg_list, lh)
--
2.30.2
Hannes Reinecke
2021-05-23 08:23:52 UTC
Permalink
Post by Christoph Hellwig
Convert the dcssblk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/s390/block/dcssblk.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:14 UTC
Permalink
Convert the xpram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/s390/block/xpram.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/s390/block/xpram.c b/drivers/s390/block/xpram.c
index d1ed39162943..91ef710edfd2 100644
--- a/drivers/s390/block/xpram.c
+++ b/drivers/s390/block/xpram.c
@@ -56,7 +56,6 @@ typedef struct {
static xpram_device_t xpram_devices[XPRAM_MAX_DEVS];
static unsigned int xpram_sizes[XPRAM_MAX_DEVS];
static struct gendisk *xpram_disks[XPRAM_MAX_DEVS];
-static struct request_queue *xpram_queues[XPRAM_MAX_DEVS];
static unsigned int xpram_pages;
static int xpram_devs;

@@ -341,17 +340,13 @@ static int __init xpram_setup_blkdev(void)
int i, rc = -ENOMEM;

for (i = 0; i < xpram_devs; i++) {
- xpram_disks[i] = alloc_disk(1);
+ xpram_disks[i] = blk_alloc_disk(NUMA_NO_NODE);
if (!xpram_disks[i])
goto out;
- xpram_queues[i] = blk_alloc_queue(NUMA_NO_NODE);
- if (!xpram_queues[i]) {
- put_disk(xpram_disks[i]);
- goto out;
- }
- blk_queue_flag_set(QUEUE_FLAG_NONROT, xpram_queues[i]);
- blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, xpram_queues[i]);
- blk_queue_logical_block_size(xpram_queues[i], 4096);
+ blk_queue_flag_set(QUEUE_FLAG_NONROT, xpram_disks[i]->queue);
+ blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM,
+ xpram_disks[i]->queue);
+ blk_queue_logical_block_size(xpram_disks[i]->queue, 4096);
}

/*
@@ -373,9 +368,9 @@ static int __init xpram_setup_blkdev(void)
offset += xpram_devices[i].size;
disk->major = XPRAM_MAJOR;
disk->first_minor = i;
+ disk->minors = 1;
disk->fops = &xpram_devops;
disk->private_data = &xpram_devices[i];
- disk->queue = xpram_queues[i];
sprintf(disk->disk_name, "slram%d", i);
set_capacity(disk, xpram_sizes[i] << 1);
add_disk(disk);
@@ -383,10 +378,8 @@ static int __init xpram_setup_blkdev(void)

return 0;
out:
- while (i--) {
- blk_cleanup_queue(xpram_queues[i]);
- put_disk(xpram_disks[i]);
- }
+ while (i--)
+ blk_cleanup_disk(xpram_disks[i]);
return rc;
}

@@ -434,8 +427,7 @@ static void __exit xpram_exit(void)
int i;
for (i = 0; i < xpram_devs; i++) {
del_gendisk(xpram_disks[i]);
- blk_cleanup_queue(xpram_queues[i]);
- put_disk(xpram_disks[i]);
+ blk_cleanup_disk(xpram_disks[i]);
}
unregister_blkdev(XPRAM_MAJOR, XPRAM_NAME);
platform_device_unregister(xpram_pdev);
--
2.30.2
Hannes Reinecke
2021-05-23 08:24:35 UTC
Permalink
Post by Christoph Hellwig
Convert the xpram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.
---
drivers/s390/block/xpram.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:15 UTC
Permalink
Convert the null_blk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation. Note that the
blk-mq mode is left with its own allocations scheme, to be handled later.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/block/null_blk/main.c | 38 +++++++++++++++++------------------
1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 5f006d9e1472..d8e098f1e5b5 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1597,11 +1597,10 @@ static void null_del_dev(struct nullb *nullb)
null_restart_queue_async(nullb);
}

- blk_cleanup_queue(nullb->q);
+ blk_cleanup_disk(nullb->disk);
if (dev->queue_mode == NULL_Q_MQ &&
nullb->tag_set == &nullb->__tag_set)
blk_mq_free_tag_set(nullb->tag_set);
- put_disk(nullb->disk);
cleanup_queues(nullb);
if (null_cache_active(nullb))
null_free_device_storage(nullb->dev, true);
@@ -1700,22 +1699,19 @@ 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;
- struct gendisk *disk;
+ struct gendisk *disk = nullb->disk;

- disk = nullb->disk = alloc_disk_node(1, nullb->dev->home_node);
- if (!disk)
- return -ENOMEM;
set_capacity(disk, size);

disk->flags |= GENHD_FL_EXT_DEVT | GENHD_FL_SUPPRESS_PARTITION_INFO;
disk->major = null_major;
disk->first_minor = nullb->index;
+ disk->minors = 1;
if (queue_is_mq(nullb->q))
disk->fops = &null_rq_ops;
else
disk->fops = &null_bio_ops;
disk->private_data = nullb;
- disk->queue = nullb->q;
strncpy(disk->disk_name, nullb->disk_name, DISK_NAME_LEN);

if (nullb->dev->zoned) {
@@ -1851,23 +1847,27 @@ static int null_add_dev(struct nullb_device *dev)
goto out_cleanup_queues;

if (!null_setup_fault())
- goto out_cleanup_queues;
+ goto out_cleanup_tags;

+ rv = -ENOMEM;
nullb->tag_set->timeout = 5 * HZ;
nullb->q = blk_mq_init_queue_data(nullb->tag_set, nullb);
- if (IS_ERR(nullb->q)) {
- rv = -ENOMEM;
+ if (IS_ERR(nullb->q))
goto out_cleanup_tags;
- }
+ nullb->disk = alloc_disk_node(1, nullb->dev->home_node);
+ if (!nullb->disk)
+ goto out_cleanup_disk;
+ nullb->disk->queue = nullb->q;
} else if (dev->queue_mode == NULL_Q_BIO) {
- nullb->q = blk_alloc_queue(dev->home_node);
- if (!nullb->q) {
- rv = -ENOMEM;
+ rv = -ENOMEM;
+ nullb->disk = blk_alloc_disk(nullb->dev->home_node);
+ if (!nullb->disk)
goto out_cleanup_queues;
- }
+
+ nullb->q = nullb->disk->queue;
rv = init_driver_queues(nullb);
if (rv)
- goto out_cleanup_blk_queue;
+ goto out_cleanup_disk;
}

if (dev->mbps) {
@@ -1883,7 +1883,7 @@ static int null_add_dev(struct nullb_device *dev)
if (dev->zoned) {
rv = null_init_zoned_dev(dev, nullb->q);
if (rv)
- goto out_cleanup_blk_queue;
+ goto out_cleanup_disk;
}

nullb->q->queuedata = nullb;
@@ -1921,8 +1921,8 @@ static int null_add_dev(struct nullb_device *dev)
return 0;
out_cleanup_zone:
null_free_zoned_dev(dev);
-out_cleanup_blk_queue:
- blk_cleanup_queue(nullb->q);
+out_cleanup_disk:
+ blk_cleanup_disk(nullb->disk);
out_cleanup_tags:
if (dev->queue_mode == NULL_Q_MQ && nullb->tag_set == &nullb->__tag_set)
blk_mq_free_tag_set(nullb->tag_set);
--
2.30.2
Hannes Reinecke
2021-05-23 08:25:24 UTC
Permalink
Post by Christoph Hellwig
Convert the null_blk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation. Note that the
blk-mq mode is left with its own allocations scheme, to be handled later.
---
drivers/block/null_blk/main.c | 38 +++++++++++++++++------------------
1 file changed, 19 insertions(+), 19 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-21 05:51:16 UTC
Permalink
blk_alloc_queue is just an internal helper now, unexport it and remove
it from the public header.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
block/blk-core.c | 1 -
block/blk.h | 2 ++
include/linux/blkdev.h | 1 -
3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 689aac2625d2..3515a66022d7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -599,7 +599,6 @@ struct request_queue *blk_alloc_queue(int node_id)
kmem_cache_free(blk_requestq_cachep, q);
return NULL;
}
-EXPORT_SYMBOL(blk_alloc_queue);

/**
* blk_get_queue - increment the request_queue refcount
diff --git a/block/blk.h b/block/blk.h
index cba3a94aabfa..3440142f029b 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -359,4 +359,6 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
struct page *page, unsigned int len, unsigned int offset,
unsigned int max_sectors, bool *same_page);

+struct request_queue *blk_alloc_queue(int node_id);
+
#endif /* BLK_INTERNAL_H */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 2c28577b50f4..d66d0da72529 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1213,7 +1213,6 @@ static inline int blk_rq_map_sg(struct request_queue *q, struct request *rq,
extern void blk_dump_rq_flags(struct request *, char *);

bool __must_check blk_get_queue(struct request_queue *);
-struct request_queue *blk_alloc_queue(int node_id);
extern void blk_put_queue(struct request_queue *);
extern void blk_set_queue_dying(struct request_queue *);
--
2.30.2
Hannes Reinecke
2021-05-23 08:26:01 UTC
Permalink
Post by Christoph Hellwig
blk_alloc_queue is just an internal helper now, unexport it and remove
it from the public header.
---
block/blk-core.c | 1 -
block/blk.h | 2 ++
include/linux/blkdev.h | 1 -
3 files changed, 2 insertions(+), 2 deletions(-)
Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Luis Chamberlain
2021-05-21 17:16:46 UTC
Permalink
Post by Christoph Hellwig
diff --git a/block/genhd.c b/block/genhd.c
index 39ca97b0edc6..2c00bc3261d9 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -335,52 +335,22 @@ static int blk_mangle_minor(int minor)
<-- snip -->
Post by Christoph Hellwig
-int blk_alloc_devt(struct block_device *bdev, dev_t *devt)
+int blk_alloc_ext_minor(void)
{
- struct gendisk *disk = bdev->bd_disk;
int idx;
- /* in consecutive minor range? */
- if (bdev->bd_partno < disk->minors) {
- *devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
- return 0;
- }
-
It is not obviously clear to me, why this was part of add_disk()
path, and ...
Post by Christoph Hellwig
diff --git a/block/partitions/core.c b/block/partitions/core.c
index dc60ecf46fe6..504297bdc8bf 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -379,9 +380,15 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
pdev->type = &part_type;
pdev->parent = ddev;
- err = blk_alloc_devt(bdev, &devt);
- if (err)
- goto out_put;
+ /* in consecutive minor range? */
+ if (bdev->bd_partno < disk->minors) {
+ devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
+ } else {
+ err = blk_alloc_ext_minor();
+ if (err < 0)
+ goto out_put;
+ devt = MKDEV(BLOCK_EXT_MAJOR, err);
+ }
pdev->devt = devt;
/* delay uevent until 'holders' subdir is created */
... and why we only add this here now.

Other than that, this looks like a super nice cleanup!

Reviewed-by: Luis Chamberlain <***@kernel.org>

Luis
Christoph Hellwig
2021-05-24 07:20:13 UTC
Permalink
Post by Luis Chamberlain
Post by Christoph Hellwig
- /* in consecutive minor range? */
- if (bdev->bd_partno < disk->minors) {
- *devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
- return 0;
- }
-
It is not obviously clear to me, why this was part of add_disk()
path, and ...
Post by Christoph Hellwig
diff --git a/block/partitions/core.c b/block/partitions/core.c
index dc60ecf46fe6..504297bdc8bf 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -379,9 +380,15 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
pdev->type = &part_type;
pdev->parent = ddev;
- err = blk_alloc_devt(bdev, &devt);
- if (err)
- goto out_put;
+ /* in consecutive minor range? */
+ if (bdev->bd_partno < disk->minors) {
+ devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
+ } else {
+ err = blk_alloc_ext_minor();
+ if (err < 0)
+ goto out_put;
+ devt = MKDEV(BLOCK_EXT_MAJOR, err);
+ }
pdev->devt = devt;
/* delay uevent until 'holders' subdir is created */
... and why we only add this here now.
For the genhd minors == 0 (aka GENHD_FL_EXT_DEVT) implies having to
allocate a dynamic dev_t, so it can be folded into another conditional.
Hannes Reinecke
2021-05-23 07:46:01 UTC
Permalink
Post by Christoph Hellwig
Untangle the mess around blk_alloc_devt by moving the check for
the used allocation scheme into the callers.
---
block/blk.h | 4 +-
block/genhd.c | 96 ++++++++++++++++-------------------------
block/partitions/core.c | 15 +++++--
3 files changed, 49 insertions(+), 66 deletions(-)
... and also fixes an issue with GENHD_FL_UP remained set in an error
path in __device_add_disk().

Reviewed-by: Hannes Reinecke <***@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
***@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Christoph Hellwig
2021-05-24 07:22:23 UTC
Permalink
... and also fixes an issue with GENHD_FL_UP remained set in an error path
in __device_add_disk().
Well, the error path in __device_add_disk is a complete disaster right
now, but Luis is looking into it fortunately.
Ulf Hansson
2021-05-25 22:41:37 UTC
Permalink
Hi all,
this series is the first part of cleaning up lifetimes and allocation of
the gendisk and request_queue structure. It adds a new interface to
allocate the disk and queue together for bio based drivers, and a helper
for cleanup/free them when a driver is unloaded or a device is removed.
May I ask what else you have in the pipe for the next steps?

The reason why I ask is that I am looking into some issues related to
lifecycle problems of gendisk/mmc, typically triggered at SD/MMC card
removal.
Together this removes the need to treat the gendisk and request_queue
as separate entities for bio based drivers.
arch/m68k/emu/nfblock.c | 20 +---
arch/xtensa/platforms/iss/simdisk.c | 29 +------
block/blk-core.c | 1
block/blk.h | 6 -
block/genhd.c | 149 +++++++++++++++++++-----------------
block/partitions/core.c | 19 ++--
drivers/block/brd.c | 94 +++++++---------------
drivers/block/drbd/drbd_main.c | 23 +----
drivers/block/n64cart.c | 8 -
drivers/block/null_blk/main.c | 38 ++++-----
drivers/block/pktcdvd.c | 11 --
drivers/block/ps3vram.c | 31 +------
drivers/block/rsxx/dev.c | 39 +++------
drivers/block/rsxx/rsxx_priv.h | 1
drivers/block/zram/zram_drv.c | 19 ----
drivers/lightnvm/core.c | 24 +----
drivers/md/bcache/super.c | 15 ---
drivers/md/dm.c | 16 +--
drivers/md/md.c | 25 ++----
drivers/memstick/core/ms_block.c | 1
drivers/nvdimm/blk.c | 27 +-----
drivers/nvdimm/btt.c | 25 +-----
drivers/nvdimm/btt.h | 2
drivers/nvdimm/pmem.c | 17 +---
drivers/nvme/host/core.c | 1
drivers/nvme/host/multipath.c | 46 +++--------
drivers/s390/block/dcssblk.c | 26 +-----
drivers/s390/block/xpram.c | 26 ++----
include/linux/blkdev.h | 1
include/linux/genhd.h | 23 +++++
30 files changed, 297 insertions(+), 466 deletions(-)
This looks like a nice cleanup to me. Feel free to add, for the series:

Reviewed-by: Ulf Hansson <***@linaro.org>

Kind regards
Uffe
Christoph Hellwig
2021-05-26 04:49:43 UTC
Permalink
Post by Ulf Hansson
Hi all,
this series is the first part of cleaning up lifetimes and allocation of
the gendisk and request_queue structure. It adds a new interface to
allocate the disk and queue together for bio based drivers, and a helper
for cleanup/free them when a driver is unloaded or a device is removed.
May I ask what else you have in the pipe for the next steps?
The reason why I ask is that I am looking into some issues related to
lifecycle problems of gendisk/mmc, typically triggered at SD/MMC card
removal.
In the short run not much more than superficial cleanups. Eventually
I want bio based drivers to not require a separate request_queue, leaving
that purely as a data structure for blk-mq based drivers. But it will
take a while until we get there, so it should not block any fixes.

For hot unplug handling it might be worth to take a look at nvme, as it
is tested a lot for that case.
Ulf Hansson
2021-05-26 08:07:31 UTC
Permalink
Post by Christoph Hellwig
Post by Ulf Hansson
Hi all,
this series is the first part of cleaning up lifetimes and allocation of
the gendisk and request_queue structure. It adds a new interface to
allocate the disk and queue together for bio based drivers, and a helper
for cleanup/free them when a driver is unloaded or a device is removed.
May I ask what else you have in the pipe for the next steps?
The reason why I ask is that I am looking into some issues related to
lifecycle problems of gendisk/mmc, typically triggered at SD/MMC card
removal.
In the short run not much more than superficial cleanups. Eventually
I want bio based drivers to not require a separate request_queue, leaving
that purely as a data structure for blk-mq based drivers. But it will
take a while until we get there, so it should not block any fixes.
Alright, thanks for clarifying.
Post by Christoph Hellwig
For hot unplug handling it might be worth to take a look at nvme, as it
is tested a lot for that case.
Okay, thanks for the hint.

Kind regards
Uffe
Jens Axboe
2021-06-01 13:48:09 UTC
Permalink
Hi all,
this series is the first part of cleaning up lifetimes and allocation of
the gendisk and request_queue structure. It adds a new interface to
allocate the disk and queue together for bio based drivers, and a helper
for cleanup/free them when a driver is unloaded or a device is removed.
Together this removes the need to treat the gendisk and request_queue
as separate entities for bio based drivers.
Applied, thanks.
--
Jens Axboe
Loading...