linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* simplify gendisk lookup and remove struct block_device aliases
@ 2020-08-26  6:24 Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 01/19] char_dev: replace cdev_map with an xarray Christoph Hellwig
                   ` (18 more replies)
  0 siblings, 19 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Hi all,

this series removes the annoying struct block_device aliases, which can
happen for a bunch of old floppy drivers (and z2ram).  In that case
multiple struct block device instances for different dev_t's can point
to the same gendisk, without being partitions.  The cause for that
is the probe/get callback registered through blk_register_regions.

This series removes blk_register_region entirely, splitting it it into
a simple xarray lookup of registered gendisks, and a probe callback
stored in the major_names array that can be used for modprobe overrides
or creating devices on demands when no gendisk is found.  The old
remapping is gone entirely, and instead the 4 remaining drivers just
register a gendisk for each operating mode.  In case of the two drivers
that have lots of aliases that is done on-demand using the new probe
callback, while for the other two I simply register all at probe time
to keep things simple.

Note that the m68k drivers are compile tested only.

Diffstat:
 b/block/genhd.c           |  183 +++++++--------
 b/drivers/base/Makefile   |    2 
 b/drivers/block/amiflop.c |   98 ++++----
 b/drivers/block/ataflop.c |  135 +++++++----
 b/drivers/block/brd.c     |   39 ---
 b/drivers/block/floppy.c  |  154 ++++++++----
 b/drivers/block/loop.c    |   30 --
 b/drivers/block/swim.c    |   17 -
 b/drivers/block/z2ram.c   |  551 ++++++++++++++++++++++------------------------
 b/drivers/ide/ide-probe.c |   66 -----
 b/drivers/ide/ide-tape.c  |    2 
 b/drivers/md/md.c         |   21 -
 b/drivers/scsi/sd.c       |   19 -
 b/fs/char_dev.c           |   94 +++----
 b/fs/dcache.c             |    1 
 b/fs/internal.h           |    5 
 b/include/linux/genhd.h   |   12 -
 b/include/linux/ide.h     |    3 
 drivers/base/map.c        |  154 ------------
 include/linux/kobj_map.h  |   20 -
 20 files changed, 686 insertions(+), 920 deletions(-)

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/19] char_dev: replace cdev_map with an xarray
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  8:19   ` Greg Kroah-Hartman
  2020-08-27  7:25   ` Hannes Reinecke
  2020-08-26  6:24 ` [PATCH 02/19] block: merge drivers/base/map.c into block/genhd.c Christoph Hellwig
                   ` (17 subsequent siblings)
  18 siblings, 2 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

None of the complicated overlapping regions bits of the kobj_map are
required for the character device lookup, so just a trivial xarray
instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/char_dev.c | 94 +++++++++++++++++++++++++--------------------------
 fs/dcache.c   |  1 -
 fs/internal.h |  5 ---
 3 files changed, 46 insertions(+), 54 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index ba0ded7842a779..6c4d6c4938f14b 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -17,7 +17,6 @@
 #include <linux/seq_file.h>
 
 #include <linux/kobject.h>
-#include <linux/kobj_map.h>
 #include <linux/cdev.h>
 #include <linux/mutex.h>
 #include <linux/backing-dev.h>
@@ -25,8 +24,7 @@
 
 #include "internal.h"
 
-static struct kobj_map *cdev_map;
-
+static DEFINE_XARRAY(cdev_map);
 static DEFINE_MUTEX(chrdevs_lock);
 
 #define CHRDEV_MAJOR_HASH_SIZE 255
@@ -367,6 +365,29 @@ void cdev_put(struct cdev *p)
 	}
 }
 
+static struct cdev *cdev_lookup(dev_t dev)
+{
+	struct cdev *cdev;
+
+retry:
+	mutex_lock(&chrdevs_lock);
+	cdev = xa_load(&cdev_map, dev);
+	if (!cdev) {
+		mutex_unlock(&chrdevs_lock);
+
+		if (request_module("char-major-%d-%d",
+				   MAJOR(dev), MINOR(dev)) > 0)
+			/* Make old-style 2.4 aliases work */
+			request_module("char-major-%d", MAJOR(dev));
+		goto retry;
+	}
+
+	if (!cdev_get(cdev))
+		cdev = NULL;
+	mutex_unlock(&chrdevs_lock);
+	return cdev;
+}
+
 /*
  * Called every time a character special file is opened
  */
@@ -380,13 +401,10 @@ static int chrdev_open(struct inode *inode, struct file *filp)
 	spin_lock(&cdev_lock);
 	p = inode->i_cdev;
 	if (!p) {
-		struct kobject *kobj;
-		int idx;
 		spin_unlock(&cdev_lock);
-		kobj = kobj_lookup(cdev_map, inode->i_rdev, &idx);
-		if (!kobj)
+		new = cdev_lookup(inode->i_rdev);
+		if (!new)
 			return -ENXIO;
-		new = container_of(kobj, struct cdev, kobj);
 		spin_lock(&cdev_lock);
 		/* Check i_cdev again in case somebody beat us to it while
 		   we dropped the lock. */
@@ -454,18 +472,6 @@ const struct file_operations def_chr_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct kobject *exact_match(dev_t dev, int *part, void *data)
-{
-	struct cdev *p = data;
-	return &p->kobj;
-}
-
-static int exact_lock(dev_t dev, void *data)
-{
-	struct cdev *p = data;
-	return cdev_get(p) ? 0 : -1;
-}
-
 /**
  * cdev_add() - add a char device to the system
  * @p: the cdev structure for the device
@@ -478,7 +484,7 @@ static int exact_lock(dev_t dev, void *data)
  */
 int cdev_add(struct cdev *p, dev_t dev, unsigned count)
 {
-	int error;
+	int error, i;
 
 	p->dev = dev;
 	p->count = count;
@@ -486,14 +492,22 @@ int cdev_add(struct cdev *p, dev_t dev, unsigned count)
 	if (WARN_ON(dev == WHITEOUT_DEV))
 		return -EBUSY;
 
-	error = kobj_map(cdev_map, dev, count, NULL,
-			 exact_match, exact_lock, p);
-	if (error)
-		return error;
+	mutex_lock(&chrdevs_lock);
+	for (i = 0; i < count; i++) {
+		error = xa_insert(&cdev_map, dev + i, p, GFP_KERNEL);
+		if (error)
+			goto out_unwind;
+	}
+	mutex_unlock(&chrdevs_lock);
 
 	kobject_get(p->kobj.parent);
-
 	return 0;
+
+out_unwind:
+	while (--i >= 0)
+		xa_erase(&cdev_map, dev + i);
+	mutex_unlock(&chrdevs_lock);
+	return error;
 }
 
 /**
@@ -575,11 +589,6 @@ void cdev_device_del(struct cdev *cdev, struct device *dev)
 		cdev_del(cdev);
 }
 
-static void cdev_unmap(dev_t dev, unsigned count)
-{
-	kobj_unmap(cdev_map, dev, count);
-}
-
 /**
  * cdev_del() - remove a cdev from the system
  * @p: the cdev structure to be removed
@@ -593,10 +602,13 @@ static void cdev_unmap(dev_t dev, unsigned count)
  */
 void cdev_del(struct cdev *p)
 {
-	cdev_unmap(p->dev, p->count);
-	kobject_put(&p->kobj);
-}
+	int i;
 
+	mutex_lock(&chrdevs_lock);
+	for (i = 0; i < p->count; i++)
+		xa_erase(&cdev_map, p->dev + i);
+	mutex_unlock(&chrdevs_lock);
+}
 
 static void cdev_default_release(struct kobject *kobj)
 {
@@ -656,20 +668,6 @@ void cdev_init(struct cdev *cdev, const struct file_operations *fops)
 	cdev->ops = fops;
 }
 
-static struct kobject *base_probe(dev_t dev, int *part, void *data)
-{
-	if (request_module("char-major-%d-%d", MAJOR(dev), MINOR(dev)) > 0)
-		/* Make old-style 2.4 aliases work */
-		request_module("char-major-%d", MAJOR(dev));
-	return NULL;
-}
-
-void __init chrdev_init(void)
-{
-	cdev_map = kobj_map_init(base_probe, &chrdevs_lock);
-}
-
-
 /* Let modules do char dev stuff */
 EXPORT_SYMBOL(register_chrdev_region);
 EXPORT_SYMBOL(unregister_chrdev_region);
diff --git a/fs/dcache.c b/fs/dcache.c
index ea0485861d9377..55e534ad6f8f7f 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3233,5 +3233,4 @@ void __init vfs_caches_init(void)
 	files_maxfiles_init();
 	mnt_init();
 	bdev_cache_init();
-	chrdev_init();
 }
diff --git a/fs/internal.h b/fs/internal.h
index 10517ece45167f..110e952e75a8aa 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -54,11 +54,6 @@ static inline void bd_forget(struct inode *inode)
 extern int __block_write_begin_int(struct page *page, loff_t pos, unsigned len,
 		get_block_t *get_block, struct iomap *iomap);
 
-/*
- * char_dev.c
- */
-extern void __init chrdev_init(void);
-
 /*
  * fs_context.c
  */
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/19] block: merge drivers/base/map.c into block/genhd.c
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 01/19] char_dev: replace cdev_map with an xarray Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  8:19   ` Greg Kroah-Hartman
  2020-08-26  6:24 ` [PATCH 03/19] block: cleanup del_gendisk a bit Christoph Hellwig
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Now that there is just a single user of the kobj_map functionality left,
merge it into the user to prepare for additional simplications.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c            | 130 +++++++++++++++++++++++++++++----
 drivers/base/Makefile    |   2 +-
 drivers/base/map.c       | 154 ---------------------------------------
 include/linux/kobj_map.h |  20 -----
 4 files changed, 118 insertions(+), 188 deletions(-)
 delete mode 100644 drivers/base/map.c
 delete mode 100644 include/linux/kobj_map.h

diff --git a/block/genhd.c b/block/genhd.c
index 99c64641c3148c..cb9a51be35b053 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -17,7 +17,6 @@
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/kmod.h>
-#include <linux/kobj_map.h>
 #include <linux/mutex.h>
 #include <linux/idr.h>
 #include <linux/log2.h>
@@ -29,6 +28,16 @@
 static DEFINE_MUTEX(block_class_lock);
 static struct kobject *block_depr;
 
+struct bdev_map {
+	struct bdev_map *next;
+	dev_t dev;
+	unsigned long range;
+	struct module *owner;
+	struct kobject *(*probe)(dev_t, int *, void *);
+	int (*lock)(dev_t, void *);
+	void *data;
+} *bdev_map[255];
+
 /* for extended dynamic devt allocation, currently only one major is used */
 #define NR_EXT_DEVT		(1 << MINORBITS)
 
@@ -520,8 +529,6 @@ void unregister_blkdev(unsigned int major, const char *name)
 
 EXPORT_SYMBOL(unregister_blkdev);
 
-static struct kobj_map *bdev_map;
-
 /**
  * blk_mangle_minor - scatter minor numbers apart
  * @minor: minor number to mangle
@@ -648,16 +655,60 @@ void blk_register_region(dev_t devt, unsigned long range, struct module *module,
 			 struct kobject *(*probe)(dev_t, int *, void *),
 			 int (*lock)(dev_t, void *), void *data)
 {
-	kobj_map(bdev_map, devt, range, module, probe, lock, data);
-}
+	unsigned n = MAJOR(devt + range - 1) - MAJOR(devt) + 1;
+	unsigned index = MAJOR(devt);
+	unsigned i;
+	struct bdev_map *p;
+
+	n = min(n, 255u);
+	p = kmalloc_array(n, sizeof(struct bdev_map), GFP_KERNEL);
+	if (p == NULL)
+		return;
 
+	for (i = 0; i < n; i++, p++) {
+		p->owner = module;
+		p->probe = probe;
+		p->lock = lock;
+		p->dev = devt;
+		p->range = range;
+		p->data = data;
+	}
+
+	mutex_lock(&block_class_lock);
+	for (i = 0, p -= n; i < n; i++, p++, index++) {
+		struct bdev_map **s = &bdev_map[index % 255];
+		while (*s && (*s)->range < range)
+			s = &(*s)->next;
+		p->next = *s;
+		*s = p;
+	}
+	mutex_unlock(&block_class_lock);
+}
 EXPORT_SYMBOL(blk_register_region);
 
 void blk_unregister_region(dev_t devt, unsigned long range)
 {
-	kobj_unmap(bdev_map, devt, range);
-}
+	unsigned n = MAJOR(devt + range - 1) - MAJOR(devt) + 1;
+	unsigned index = MAJOR(devt);
+	unsigned i;
+	struct bdev_map *found = NULL;
 
+	mutex_lock(&block_class_lock);
+	for (i = 0; i < min(n, 255u); i++, index++) {
+		struct bdev_map **s;
+		for (s = &bdev_map[index % 255]; *s; s = &(*s)->next) {
+			struct bdev_map *p = *s;
+			if (p->dev == devt && p->range == range) {
+				*s = p->next;
+				if (!found)
+					found = p;
+				break;
+			}
+		}
+	}
+	mutex_unlock(&block_class_lock);
+	kfree(found);
+}
 EXPORT_SYMBOL(blk_unregister_region);
 
 static struct kobject *exact_match(dev_t devt, int *partno, void *data)
@@ -983,6 +1034,47 @@ static ssize_t disk_badblocks_store(struct device *dev,
 	return badblocks_store(disk->bb, page, len, 0);
 }
 
+static struct gendisk *lookup_gendisk(dev_t dev, int *partno)
+{
+	struct kobject *kobj;
+	struct bdev_map *p;
+	unsigned long best = ~0UL;
+
+retry:
+	mutex_lock(&block_class_lock);
+	for (p = bdev_map[MAJOR(dev) % 255]; p; p = p->next) {
+		struct kobject *(*probe)(dev_t, int *, void *);
+		struct module *owner;
+		void *data;
+
+		if (p->dev > dev || p->dev + p->range - 1 < dev)
+			continue;
+		if (p->range - 1 >= best)
+			break;
+		if (!try_module_get(p->owner))
+			continue;
+		owner = p->owner;
+		data = p->data;
+		probe = p->probe;
+		best = p->range - 1;
+		*partno = dev - p->dev;
+		if (p->lock && p->lock(dev, data) < 0) {
+			module_put(owner);
+			continue;
+		}
+		mutex_unlock(&block_class_lock);
+		kobj = probe(dev, partno, data);
+		/* Currently ->owner protects _only_ ->probe() itself. */
+		module_put(owner);
+		if (kobj)
+			return dev_to_disk(kobj_to_dev(kobj));
+		goto retry;
+	}
+	mutex_unlock(&block_class_lock);
+	return NULL;
+}
+
+
 /**
  * get_gendisk - get partitioning information for a given device
  * @devt: device to get partitioning information for
@@ -1000,11 +1092,7 @@ struct gendisk *get_gendisk(dev_t devt, int *partno)
 	might_sleep();
 
 	if (MAJOR(devt) != BLOCK_EXT_MAJOR) {
-		struct kobject *kobj;
-
-		kobj = kobj_lookup(bdev_map, devt, partno);
-		if (kobj)
-			disk = dev_to_disk(kobj_to_dev(kobj));
+		disk = lookup_gendisk(devt, partno);
 	} else {
 		struct hd_struct *part;
 
@@ -1217,6 +1305,22 @@ static struct kobject *base_probe(dev_t devt, int *partno, void *data)
 	return NULL;
 }
 
+static void bdev_map_init(void)
+{
+	struct bdev_map *base;
+	int i;
+
+	base = kzalloc(sizeof(*base), GFP_KERNEL);
+	if (!base)
+		panic("cannot allocate bdev_map");
+
+	base->dev = 1;
+	base->range = ~0 ;
+	base->probe = base_probe;
+	for (i = 0; i < 255; i++)
+		bdev_map[i] = base;
+}
+
 static int __init genhd_device_init(void)
 {
 	int error;
@@ -1225,7 +1329,7 @@ static int __init genhd_device_init(void)
 	error = class_register(&block_class);
 	if (unlikely(error))
 		return error;
-	bdev_map = kobj_map_init(base_probe, &block_class_lock);
+	bdev_map_init();
 	blk_dev_init();
 
 	register_blkdev(BLOCK_EXT_MAJOR, "blkext");
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 157452080f3d7f..4ffd2a785f5ed3 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -3,7 +3,7 @@
 
 obj-y			:= component.o core.o bus.o dd.o syscore.o \
 			   driver.o class.o platform.o \
-			   cpu.o firmware.o init.o map.o devres.o \
+			   cpu.o firmware.o init.o devres.o \
 			   attribute_container.o transport_class.o \
 			   topology.o container.o property.o cacheinfo.o \
 			   devcon.o swnode.o
diff --git a/drivers/base/map.c b/drivers/base/map.c
deleted file mode 100644
index 5650ab2b247ada..00000000000000
--- a/drivers/base/map.c
+++ /dev/null
@@ -1,154 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- *  linux/drivers/base/map.c
- *
- * (C) Copyright Al Viro 2002,2003
- *
- * NOTE: data structure needs to be changed.  It works, but for large dev_t
- * it will be too slow.  It is isolated, though, so these changes will be
- * local to that file.
- */
-
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/mutex.h>
-#include <linux/kdev_t.h>
-#include <linux/kobject.h>
-#include <linux/kobj_map.h>
-
-struct kobj_map {
-	struct probe {
-		struct probe *next;
-		dev_t dev;
-		unsigned long range;
-		struct module *owner;
-		kobj_probe_t *get;
-		int (*lock)(dev_t, void *);
-		void *data;
-	} *probes[255];
-	struct mutex *lock;
-};
-
-int kobj_map(struct kobj_map *domain, dev_t dev, unsigned long range,
-	     struct module *module, kobj_probe_t *probe,
-	     int (*lock)(dev_t, void *), void *data)
-{
-	unsigned n = MAJOR(dev + range - 1) - MAJOR(dev) + 1;
-	unsigned index = MAJOR(dev);
-	unsigned i;
-	struct probe *p;
-
-	if (n > 255)
-		n = 255;
-
-	p = kmalloc_array(n, sizeof(struct probe), GFP_KERNEL);
-	if (p == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < n; i++, p++) {
-		p->owner = module;
-		p->get = probe;
-		p->lock = lock;
-		p->dev = dev;
-		p->range = range;
-		p->data = data;
-	}
-	mutex_lock(domain->lock);
-	for (i = 0, p -= n; i < n; i++, p++, index++) {
-		struct probe **s = &domain->probes[index % 255];
-		while (*s && (*s)->range < range)
-			s = &(*s)->next;
-		p->next = *s;
-		*s = p;
-	}
-	mutex_unlock(domain->lock);
-	return 0;
-}
-
-void kobj_unmap(struct kobj_map *domain, dev_t dev, unsigned long range)
-{
-	unsigned n = MAJOR(dev + range - 1) - MAJOR(dev) + 1;
-	unsigned index = MAJOR(dev);
-	unsigned i;
-	struct probe *found = NULL;
-
-	if (n > 255)
-		n = 255;
-
-	mutex_lock(domain->lock);
-	for (i = 0; i < n; i++, index++) {
-		struct probe **s;
-		for (s = &domain->probes[index % 255]; *s; s = &(*s)->next) {
-			struct probe *p = *s;
-			if (p->dev == dev && p->range == range) {
-				*s = p->next;
-				if (!found)
-					found = p;
-				break;
-			}
-		}
-	}
-	mutex_unlock(domain->lock);
-	kfree(found);
-}
-
-struct kobject *kobj_lookup(struct kobj_map *domain, dev_t dev, int *index)
-{
-	struct kobject *kobj;
-	struct probe *p;
-	unsigned long best = ~0UL;
-
-retry:
-	mutex_lock(domain->lock);
-	for (p = domain->probes[MAJOR(dev) % 255]; p; p = p->next) {
-		struct kobject *(*probe)(dev_t, int *, void *);
-		struct module *owner;
-		void *data;
-
-		if (p->dev > dev || p->dev + p->range - 1 < dev)
-			continue;
-		if (p->range - 1 >= best)
-			break;
-		if (!try_module_get(p->owner))
-			continue;
-		owner = p->owner;
-		data = p->data;
-		probe = p->get;
-		best = p->range - 1;
-		*index = dev - p->dev;
-		if (p->lock && p->lock(dev, data) < 0) {
-			module_put(owner);
-			continue;
-		}
-		mutex_unlock(domain->lock);
-		kobj = probe(dev, index, data);
-		/* Currently ->owner protects _only_ ->probe() itself. */
-		module_put(owner);
-		if (kobj)
-			return kobj;
-		goto retry;
-	}
-	mutex_unlock(domain->lock);
-	return NULL;
-}
-
-struct kobj_map *kobj_map_init(kobj_probe_t *base_probe, struct mutex *lock)
-{
-	struct kobj_map *p = kmalloc(sizeof(struct kobj_map), GFP_KERNEL);
-	struct probe *base = kzalloc(sizeof(*base), GFP_KERNEL);
-	int i;
-
-	if ((p == NULL) || (base == NULL)) {
-		kfree(p);
-		kfree(base);
-		return NULL;
-	}
-
-	base->dev = 1;
-	base->range = ~0;
-	base->get = base_probe;
-	for (i = 0; i < 255; i++)
-		p->probes[i] = base;
-	p->lock = lock;
-	return p;
-}
diff --git a/include/linux/kobj_map.h b/include/linux/kobj_map.h
deleted file mode 100644
index c9919f8b22932c..00000000000000
--- a/include/linux/kobj_map.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * kobj_map.h
- */
-
-#ifndef _KOBJ_MAP_H_
-#define _KOBJ_MAP_H_
-
-#include <linux/mutex.h>
-
-typedef struct kobject *kobj_probe_t(dev_t, int *, void *);
-struct kobj_map;
-
-int kobj_map(struct kobj_map *, dev_t, unsigned long, struct module *,
-	     kobj_probe_t *, int (*)(dev_t, void *), void *);
-void kobj_unmap(struct kobj_map *, dev_t, unsigned long);
-struct kobject *kobj_lookup(struct kobj_map *, dev_t, int *);
-struct kobj_map *kobj_map_init(kobj_probe_t *, struct mutex *);
-
-#endif /* _KOBJ_MAP_H_ */
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/19] block: cleanup del_gendisk a bit
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 01/19] char_dev: replace cdev_map with an xarray Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 02/19] block: merge drivers/base/map.c into block/genhd.c Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 04/19] block: split block_class_lock Christoph Hellwig
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Merge three hidden gendisk checks into one.

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

diff --git a/block/genhd.c b/block/genhd.c
index cb9a51be35b053..df6485223a2c3d 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -973,22 +973,22 @@ void del_gendisk(struct gendisk *disk)
 	disk->flags &= ~GENHD_FL_UP;
 	up_write(&disk->lookup_sem);
 
-	if (!(disk->flags & GENHD_FL_HIDDEN))
+	WARN_ON(!disk->queue);
+
+	if (!(disk->flags & GENHD_FL_HIDDEN)) {
 		sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
-	if (disk->queue) {
+
 		/*
 		 * Unregister bdi before releasing device numbers (as they can
 		 * get reused and we'd get clashes in sysfs).
 		 */
-		if (!(disk->flags & GENHD_FL_HIDDEN))
-			bdi_unregister(disk->queue->backing_dev_info);
+		bdi_unregister(disk->queue->backing_dev_info);
 		blk_unregister_queue(disk);
+		blk_unregister_region(disk_devt(disk), disk->minors);
 	} else {
-		WARN_ON(1);
+		blk_unregister_queue(disk);
 	}
 
-	if (!(disk->flags & GENHD_FL_HIDDEN))
-		blk_unregister_region(disk_devt(disk), disk->minors);
 	/*
 	 * Remove gendisk pointer from idr so that it cannot be looked up
 	 * while RCU period before freeing gendisk is running to prevent
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/19] block: split block_class_lock
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 03/19] block: cleanup del_gendisk a bit Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 05/19] block: rework requesting modules for unclaimed devices Christoph Hellwig
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Split the block_class_lock mutex into one each to protect bdev_map
and major_names.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index df6485223a2c3d..0ae6210e141ee5 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -25,7 +25,6 @@
 
 #include "blk.h"
 
-static DEFINE_MUTEX(block_class_lock);
 static struct kobject *block_depr;
 
 struct bdev_map {
@@ -37,6 +36,7 @@ struct bdev_map {
 	int (*lock)(dev_t, void *);
 	void *data;
 } *bdev_map[255];
+static DEFINE_MUTEX(bdev_map_lock);
 
 /* for extended dynamic devt allocation, currently only one major is used */
 #define NR_EXT_DEVT		(1 << MINORBITS)
@@ -403,6 +403,7 @@ static struct blk_major_name {
 	int major;
 	char name[16];
 } *major_names[BLKDEV_MAJOR_HASH_SIZE];
+static DEFINE_MUTEX(major_names_lock);
 
 /* index in the above - for now: assume no multimajor ranges */
 static inline int major_to_index(unsigned major)
@@ -415,11 +416,11 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
 {
 	struct blk_major_name *dp;
 
-	mutex_lock(&block_class_lock);
+	mutex_lock(&major_names_lock);
 	for (dp = major_names[major_to_index(offset)]; dp; dp = dp->next)
 		if (dp->major == offset)
 			seq_printf(seqf, "%3d %s\n", dp->major, dp->name);
-	mutex_unlock(&block_class_lock);
+	mutex_unlock(&major_names_lock);
 }
 #endif /* CONFIG_PROC_FS */
 
@@ -448,7 +449,7 @@ int register_blkdev(unsigned int major, const char *name)
 	struct blk_major_name **n, *p;
 	int index, ret = 0;
 
-	mutex_lock(&block_class_lock);
+	mutex_lock(&major_names_lock);
 
 	/* temporary */
 	if (major == 0) {
@@ -501,7 +502,7 @@ int register_blkdev(unsigned int major, const char *name)
 		kfree(p);
 	}
 out:
-	mutex_unlock(&block_class_lock);
+	mutex_unlock(&major_names_lock);
 	return ret;
 }
 
@@ -513,7 +514,7 @@ void unregister_blkdev(unsigned int major, const char *name)
 	struct blk_major_name *p = NULL;
 	int index = major_to_index(major);
 
-	mutex_lock(&block_class_lock);
+	mutex_lock(&major_names_lock);
 	for (n = &major_names[index]; *n; n = &(*n)->next)
 		if ((*n)->major == major)
 			break;
@@ -523,7 +524,7 @@ void unregister_blkdev(unsigned int major, const char *name)
 		p = *n;
 		*n = p->next;
 	}
-	mutex_unlock(&block_class_lock);
+	mutex_unlock(&major_names_lock);
 	kfree(p);
 }
 
@@ -674,7 +675,7 @@ void blk_register_region(dev_t devt, unsigned long range, struct module *module,
 		p->data = data;
 	}
 
-	mutex_lock(&block_class_lock);
+	mutex_lock(&bdev_map_lock);
 	for (i = 0, p -= n; i < n; i++, p++, index++) {
 		struct bdev_map **s = &bdev_map[index % 255];
 		while (*s && (*s)->range < range)
@@ -682,7 +683,7 @@ void blk_register_region(dev_t devt, unsigned long range, struct module *module,
 		p->next = *s;
 		*s = p;
 	}
-	mutex_unlock(&block_class_lock);
+	mutex_unlock(&bdev_map_lock);
 }
 EXPORT_SYMBOL(blk_register_region);
 
@@ -693,7 +694,7 @@ void blk_unregister_region(dev_t devt, unsigned long range)
 	unsigned i;
 	struct bdev_map *found = NULL;
 
-	mutex_lock(&block_class_lock);
+	mutex_lock(&bdev_map_lock);
 	for (i = 0; i < min(n, 255u); i++, index++) {
 		struct bdev_map **s;
 		for (s = &bdev_map[index % 255]; *s; s = &(*s)->next) {
@@ -706,7 +707,7 @@ void blk_unregister_region(dev_t devt, unsigned long range)
 			}
 		}
 	}
-	mutex_unlock(&block_class_lock);
+	mutex_unlock(&bdev_map_lock);
 	kfree(found);
 }
 EXPORT_SYMBOL(blk_unregister_region);
@@ -1041,7 +1042,7 @@ static struct gendisk *lookup_gendisk(dev_t dev, int *partno)
 	unsigned long best = ~0UL;
 
 retry:
-	mutex_lock(&block_class_lock);
+	mutex_lock(&bdev_map_lock);
 	for (p = bdev_map[MAJOR(dev) % 255]; p; p = p->next) {
 		struct kobject *(*probe)(dev_t, int *, void *);
 		struct module *owner;
@@ -1062,7 +1063,7 @@ static struct gendisk *lookup_gendisk(dev_t dev, int *partno)
 			module_put(owner);
 			continue;
 		}
-		mutex_unlock(&block_class_lock);
+		mutex_unlock(&bdev_map_lock);
 		kobj = probe(dev, partno, data);
 		/* Currently ->owner protects _only_ ->probe() itself. */
 		module_put(owner);
@@ -1070,7 +1071,7 @@ static struct gendisk *lookup_gendisk(dev_t dev, int *partno)
 			return dev_to_disk(kobj_to_dev(kobj));
 		goto retry;
 	}
-	mutex_unlock(&block_class_lock);
+	mutex_unlock(&bdev_map_lock);
 	return NULL;
 }
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/19] block: rework requesting modules for unclaimed devices
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 04/19] block: split block_class_lock Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 06/19] block: add an optional probe callback to major_names Christoph Hellwig
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Instead of reusing the ranges in bdev_map, add a new helper that is
called if no ranges was found.  This is a first step to unpeel and
eventually remove the complex ranges structure.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 0ae6210e141ee5..00164304317cfa 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1035,6 +1035,13 @@ static ssize_t disk_badblocks_store(struct device *dev,
 	return badblocks_store(disk->bb, page, len, 0);
 }
 
+static void request_gendisk_module(dev_t devt)
+{
+	if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
+		/* Make old-style 2.4 aliases work */
+		request_module("block-major-%d", MAJOR(devt));
+}
+
 static struct gendisk *lookup_gendisk(dev_t dev, int *partno)
 {
 	struct kobject *kobj;
@@ -1059,6 +1066,14 @@ static struct gendisk *lookup_gendisk(dev_t dev, int *partno)
 		probe = p->probe;
 		best = p->range - 1;
 		*partno = dev - p->dev;
+
+		if (!probe) {
+			mutex_unlock(&bdev_map_lock);
+			module_put(owner);
+			request_gendisk_module(dev);
+			goto retry;
+		}
+
 		if (p->lock && p->lock(dev, data) < 0) {
 			module_put(owner);
 			continue;
@@ -1297,15 +1312,6 @@ static const struct seq_operations partitions_op = {
 };
 #endif
 
-
-static struct kobject *base_probe(dev_t devt, int *partno, void *data)
-{
-	if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
-		/* Make old-style 2.4 aliases work */
-		request_module("block-major-%d", MAJOR(devt));
-	return NULL;
-}
-
 static void bdev_map_init(void)
 {
 	struct bdev_map *base;
@@ -1317,7 +1323,6 @@ static void bdev_map_init(void)
 
 	base->dev = 1;
 	base->range = ~0 ;
-	base->probe = base_probe;
 	for (i = 0; i < 255; i++)
 		bdev_map[i] = base;
 }
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/19] block: add an optional probe callback to major_names
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (4 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 05/19] block: rework requesting modules for unclaimed devices Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 07/19] ide: remove ide_{,un}register_region Christoph Hellwig
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Add a callback to the major_names array that allows a driver to override
how to probe for dev_t that doesn't currently have a gendisk registered.
This will help separating the lookup of the gendisk by dev_t vs probe
action for a not currently registered dev_t.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c         | 21 ++++++++++++++++++---
 include/linux/genhd.h |  5 ++++-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 00164304317cfa..4cbeff3ec1ef5a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -402,6 +402,7 @@ static struct blk_major_name {
 	struct blk_major_name *next;
 	int major;
 	char name[16];
+	void (*probe)(dev_t devt);
 } *major_names[BLKDEV_MAJOR_HASH_SIZE];
 static DEFINE_MUTEX(major_names_lock);
 
@@ -444,7 +445,8 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
  * See Documentation/admin-guide/devices.txt for the list of allocated
  * major numbers.
  */
-int register_blkdev(unsigned int major, const char *name)
+int __register_blkdev(unsigned int major, const char *name,
+		void (*probe)(dev_t devt))
 {
 	struct blk_major_name **n, *p;
 	int index, ret = 0;
@@ -483,6 +485,7 @@ int register_blkdev(unsigned int major, const char *name)
 	}
 
 	p->major = major;
+	p->probe = probe;
 	strlcpy(p->name, name, sizeof(p->name));
 	p->next = NULL;
 	index = major_to_index(major);
@@ -505,8 +508,7 @@ int register_blkdev(unsigned int major, const char *name)
 	mutex_unlock(&major_names_lock);
 	return ret;
 }
-
-EXPORT_SYMBOL(register_blkdev);
+EXPORT_SYMBOL(__register_blkdev);
 
 void unregister_blkdev(unsigned int major, const char *name)
 {
@@ -1037,6 +1039,19 @@ static ssize_t disk_badblocks_store(struct device *dev,
 
 static void request_gendisk_module(dev_t devt)
 {
+	unsigned int major = MAJOR(devt);
+	struct blk_major_name **n;
+
+	mutex_lock(&major_names_lock);
+	for (n = &major_names[major_to_index(major)]; *n; n = &(*n)->next) {
+		if ((*n)->major == major && (*n)->probe) {
+			(*n)->probe(devt);
+			mutex_unlock(&major_names_lock);
+			return;
+		}
+	}
+	mutex_unlock(&major_names_lock);
+
 	if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
 		/* Make old-style 2.4 aliases work */
 		request_module("block-major-%d", MAJOR(devt));
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 4ab853461dff25..c0bde190a3dd0c 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -369,7 +369,10 @@ extern void blk_unregister_region(dev_t devt, unsigned long range);
 
 #define alloc_disk(minors) alloc_disk_node(minors, NUMA_NO_NODE)
 
-int register_blkdev(unsigned int major, const char *name);
+int __register_blkdev(unsigned int major, const char *name,
+		void (*probe)(dev_t devt));
+#define register_blkdev(major, name) \
+	__register_blkdev(major, name, NULL)
 void unregister_blkdev(unsigned int major, const char *name);
 
 int revalidate_disk(struct gendisk *disk);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/19] ide: remove ide_{,un}register_region
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (5 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 06/19] block: add an optional probe callback to major_names Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-27  7:25   ` Hannes Reinecke
  2020-08-26  6:24 ` [PATCH 08/19] swim: don't call blk_register_region Christoph Hellwig
                   ` (11 subsequent siblings)
  18 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

There is no need to ever register the fake gendisk used for ide-tape.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ide/ide-probe.c | 32 --------------------------------
 drivers/ide/ide-tape.c  |  2 --
 include/linux/ide.h     |  3 ---
 3 files changed, 37 deletions(-)

diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index 1ddc45a04418cd..076d34b381720f 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -929,38 +929,6 @@ static struct kobject *ata_probe(dev_t dev, int *part, void *data)
 	return NULL;
 }
 
-static struct kobject *exact_match(dev_t dev, int *part, void *data)
-{
-	struct gendisk *p = data;
-	*part &= (1 << PARTN_BITS) - 1;
-	return &disk_to_dev(p)->kobj;
-}
-
-static int exact_lock(dev_t dev, void *data)
-{
-	struct gendisk *p = data;
-
-	if (!get_disk_and_module(p))
-		return -1;
-	return 0;
-}
-
-void ide_register_region(struct gendisk *disk)
-{
-	blk_register_region(MKDEV(disk->major, disk->first_minor),
-			    disk->minors, NULL, exact_match, exact_lock, disk);
-}
-
-EXPORT_SYMBOL_GPL(ide_register_region);
-
-void ide_unregister_region(struct gendisk *disk)
-{
-	blk_unregister_region(MKDEV(disk->major, disk->first_minor),
-			      disk->minors);
-}
-
-EXPORT_SYMBOL_GPL(ide_unregister_region);
-
 void ide_init_disk(struct gendisk *disk, ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 6f26634b22bbec..88b96437b22e62 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1822,7 +1822,6 @@ static void ide_tape_remove(ide_drive_t *drive)
 
 	ide_proc_unregister_driver(drive, tape->driver);
 	device_del(&tape->dev);
-	ide_unregister_region(tape->disk);
 
 	mutex_lock(&idetape_ref_mutex);
 	put_device(&tape->dev);
@@ -2026,7 +2025,6 @@ static int ide_tape_probe(ide_drive_t *drive)
 		      "n%s", tape->name);
 
 	g->fops = &idetape_block_ops;
-	ide_register_region(g);
 
 	return 0;
 
diff --git a/include/linux/ide.h b/include/linux/ide.h
index a254841bd3156d..cfa9e4b0c325a4 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1495,9 +1495,6 @@ static inline void ide_acpi_port_init_devices(ide_hwif_t *hwif) { ; }
 static inline void ide_acpi_set_state(ide_hwif_t *hwif, int on) {}
 #endif
 
-void ide_register_region(struct gendisk *);
-void ide_unregister_region(struct gendisk *);
-
 void ide_check_nien_quirk_list(ide_drive_t *);
 void ide_undecoded_slave(ide_drive_t *);
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/19] swim: don't call blk_register_region
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (6 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 07/19] ide: remove ide_{,un}register_region Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 09/19] sd: use __register_blkdev to avoid a modprobe for an unregistered dev_t Christoph Hellwig
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

The swim driver (unlike various other floppy drivers) doesn't have
magic device nodes for certain modes, and already registers a gendisk
for each of the floppies supported by a device.  Thus the region
registered is a no-op and can be removed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/swim.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index dd34504382e533..5a8f5932f9bde4 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -763,18 +763,6 @@ static const struct block_device_operations floppy_fops = {
 	.revalidate_disk = floppy_revalidate,
 };
 
-static struct kobject *floppy_find(dev_t dev, int *part, void *data)
-{
-	struct swim_priv *swd = data;
-	int drive = (*part & 3);
-
-	if (drive >= swd->floppy_count)
-		return NULL;
-
-	*part = 0;
-	return get_disk_and_module(swd->unit[drive].disk);
-}
-
 static int swim_add_floppy(struct swim_priv *swd, enum drive_location location)
 {
 	struct floppy_state *fs = &swd->unit[swd->floppy_count];
@@ -864,9 +852,6 @@ static int swim_floppy_init(struct swim_priv *swd)
 		add_disk(swd->unit[drive].disk);
 	}
 
-	blk_register_region(MKDEV(FLOPPY_MAJOR, 0), 256, THIS_MODULE,
-			    floppy_find, NULL, swd);
-
 	return 0;
 
 exit_put_disks:
@@ -950,8 +935,6 @@ static int swim_remove(struct platform_device *dev)
 	int drive;
 	struct resource *res;
 
-	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
-
 	for (drive = 0; drive < swd->floppy_count; drive++) {
 		del_gendisk(swd->unit[drive].disk);
 		blk_cleanup_queue(swd->unit[drive].disk->queue);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/19] sd: use __register_blkdev to avoid a modprobe for an unregistered dev_t
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (7 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 08/19] swim: don't call blk_register_region Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 10/19] brd: use __register_blkdev to allocate devices on demand Christoph Hellwig
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Switch from using blk_register_region to the probe callback passed to
__register_blkdev to disable the request_module call for an unclaimed
dev_t in the SD majors.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sd.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 95018e650f2d0c..ece87b1aab1b78 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -596,13 +596,11 @@ static struct scsi_driver sd_template = {
 };
 
 /*
- * Dummy kobj_map->probe function.
- * The default ->probe function will call modprobe, which is
- * pointless as this module is already loaded.
+ * Don't request a new module, as that could deadlock in multipath
+ * environment.
  */
-static struct kobject *sd_default_probe(dev_t devt, int *partno, void *data)
+static void sd_default_probe(dev_t devt)
 {
-	return NULL;
 }
 
 /*
@@ -3479,9 +3477,6 @@ static int sd_remove(struct device *dev)
 
 	free_opal_dev(sdkp->opal_dev);
 
-	blk_register_region(devt, SD_MINORS, NULL,
-			    sd_default_probe, NULL, NULL);
-
 	mutex_lock(&sd_ref_mutex);
 	dev_set_drvdata(dev, NULL);
 	put_device(&sdkp->dev);
@@ -3671,11 +3666,9 @@ static int __init init_sd(void)
 	SCSI_LOG_HLQUEUE(3, printk("init_sd: sd driver entry point\n"));
 
 	for (i = 0; i < SD_MAJORS; i++) {
-		if (register_blkdev(sd_major(i), "sd") != 0)
+		if (__register_blkdev(sd_major(i), "sd", sd_default_probe))
 			continue;
 		majors++;
-		blk_register_region(sd_major(i), SD_MINORS, NULL,
-				    sd_default_probe, NULL, NULL);
 	}
 
 	if (!majors)
@@ -3748,10 +3741,8 @@ static void __exit exit_sd(void)
 
 	class_unregister(&sd_disk_class);
 
-	for (i = 0; i < SD_MAJORS; i++) {
-		blk_unregister_region(sd_major(i), SD_MINORS);
+	for (i = 0; i < SD_MAJORS; i++)
 		unregister_blkdev(sd_major(i), "sd");
-	}
 }
 
 module_init(init_sd);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/19] brd: use __register_blkdev to allocate devices on demand
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (8 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 09/19] sd: use __register_blkdev to avoid a modprobe for an unregistered dev_t Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 11/19] loop: " Christoph Hellwig
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Use the simpler mechanism attached to major_name to allocate a brd device
when a currently unregistered minor is accessed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/brd.c | 39 +++++++++++----------------------------
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 2723a70eb85593..c8ac36351115ef 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -427,14 +427,15 @@ static void brd_free(struct brd_device *brd)
 	kfree(brd);
 }
 
-static struct brd_device *brd_init_one(int i, bool *new)
+static void brd_probe(dev_t dev)
 {
 	struct brd_device *brd;
+	int i = MINOR(dev) / max_part;
 
-	*new = false;
+	mutex_lock(&brd_devices_mutex);
 	list_for_each_entry(brd, &brd_devices, brd_list) {
 		if (brd->brd_number == i)
-			goto out;
+			goto out_unlock;
 	}
 
 	brd = brd_alloc(i);
@@ -443,9 +444,9 @@ static struct brd_device *brd_init_one(int i, bool *new)
 		add_disk(brd->brd_disk);
 		list_add_tail(&brd->brd_list, &brd_devices);
 	}
-	*new = true;
-out:
-	return brd;
+
+out_unlock:
+	mutex_unlock(&brd_devices_mutex);
 }
 
 static void brd_del_one(struct brd_device *brd)
@@ -455,23 +456,6 @@ static void brd_del_one(struct brd_device *brd)
 	brd_free(brd);
 }
 
-static struct kobject *brd_probe(dev_t dev, int *part, void *data)
-{
-	struct brd_device *brd;
-	struct kobject *kobj;
-	bool new;
-
-	mutex_lock(&brd_devices_mutex);
-	brd = brd_init_one(MINOR(dev) / max_part, &new);
-	kobj = brd ? get_disk_and_module(brd->brd_disk) : NULL;
-	mutex_unlock(&brd_devices_mutex);
-
-	if (new)
-		*part = 0;
-
-	return kobj;
-}
-
 static inline void brd_check_and_reset_par(void)
 {
 	if (unlikely(!max_part))
@@ -511,11 +495,12 @@ static int __init brd_init(void)
 	 *	dynamically.
 	 */
 
-	if (register_blkdev(RAMDISK_MAJOR, "ramdisk"))
+	if (__register_blkdev(RAMDISK_MAJOR, "ramdisk", brd_probe))
 		return -EIO;
 
 	brd_check_and_reset_par();
 
+	mutex_lock(&brd_devices_mutex);
 	for (i = 0; i < rd_nr; i++) {
 		brd = brd_alloc(i);
 		if (!brd)
@@ -533,9 +518,7 @@ static int __init brd_init(void)
 		brd->brd_disk->queue = brd->brd_queue;
 		add_disk(brd->brd_disk);
 	}
-
-	blk_register_region(MKDEV(RAMDISK_MAJOR, 0), 1UL << MINORBITS,
-				  THIS_MODULE, brd_probe, NULL, NULL);
+	mutex_unlock(&brd_devices_mutex);
 
 	pr_info("brd: module loaded\n");
 	return 0;
@@ -545,6 +528,7 @@ static int __init brd_init(void)
 		list_del(&brd->brd_list);
 		brd_free(brd);
 	}
+	mutex_unlock(&brd_devices_mutex);
 	unregister_blkdev(RAMDISK_MAJOR, "ramdisk");
 
 	pr_info("brd: module NOT loaded !!!\n");
@@ -558,7 +542,6 @@ static void __exit brd_exit(void)
 	list_for_each_entry_safe(brd, next, &brd_devices, brd_list)
 		brd_del_one(brd);
 
-	blk_unregister_region(MKDEV(RAMDISK_MAJOR, 0), 1UL << MINORBITS);
 	unregister_blkdev(RAMDISK_MAJOR, "ramdisk");
 
 	pr_info("brd: module unloaded\n");
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/19] loop: use __register_blkdev to allocate devices on demand
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (9 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 10/19] brd: use __register_blkdev to allocate devices on demand Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 12/19] md: " Christoph Hellwig
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Use the simpler mechanism attached to major_name to allocate a brd device
when a currently unregistered minor is accessed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/loop.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 5a991510136225..7c944a425f718f 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2234,24 +2234,18 @@ static int loop_lookup(struct loop_device **l, int i)
 	return ret;
 }
 
-static struct kobject *loop_probe(dev_t dev, int *part, void *data)
+static void loop_probe(dev_t dev)
 {
+	int idx = MINOR(dev) >> part_shift;
 	struct loop_device *lo;
-	struct kobject *kobj;
-	int err;
+
+	if (max_loop && idx >= max_loop)
+		return;
 
 	mutex_lock(&loop_ctl_mutex);
-	err = loop_lookup(&lo, MINOR(dev) >> part_shift);
-	if (err < 0)
-		err = loop_add(&lo, MINOR(dev) >> part_shift);
-	if (err < 0)
-		kobj = NULL;
-	else
-		kobj = get_disk_and_module(lo->lo_disk);
+	if (loop_lookup(&lo, idx) < 0)
+		loop_add(&lo, idx);
 	mutex_unlock(&loop_ctl_mutex);
-
-	*part = 0;
-	return kobj;
 }
 
 static long loop_control_ioctl(struct file *file, unsigned int cmd,
@@ -2371,14 +2365,11 @@ static int __init loop_init(void)
 		goto err_out;
 
 
-	if (register_blkdev(LOOP_MAJOR, "loop")) {
+	if (__register_blkdev(LOOP_MAJOR, "loop", loop_probe)) {
 		err = -EIO;
 		goto misc_out;
 	}
 
-	blk_register_region(MKDEV(LOOP_MAJOR, 0), range,
-				  THIS_MODULE, loop_probe, NULL, NULL);
-
 	/* pre-create number of devices given by config or max_loop */
 	mutex_lock(&loop_ctl_mutex);
 	for (i = 0; i < nr; i++)
@@ -2404,16 +2395,11 @@ static int loop_exit_cb(int id, void *ptr, void *data)
 
 static void __exit loop_exit(void)
 {
-	unsigned long range;
-
-	range = max_loop ? max_loop << part_shift : 1UL << MINORBITS;
-
 	mutex_lock(&loop_ctl_mutex);
 
 	idr_for_each(&loop_index_idr, &loop_exit_cb, NULL);
 	idr_destroy(&loop_index_idr);
 
-	blk_unregister_region(MKDEV(LOOP_MAJOR, 0), range);
 	unregister_blkdev(LOOP_MAJOR, "loop");
 
 	misc_deregister(&loop_misc);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/19] md: use __register_blkdev to allocate devices on demand
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (10 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 11/19] loop: " Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-28  7:08   ` Song Liu
  2020-08-26  6:24 ` [PATCH 13/19] ide: switch to __register_blkdev for command set probing Christoph Hellwig
                   ` (6 subsequent siblings)
  18 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Use the simpler mechanism attached to major_name to allocate a brd device
when a currently unregistered minor is accessed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 60727820702300..3c4ef862ac5fb0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5766,11 +5766,12 @@ static int md_alloc(dev_t dev, char *name)
 	return error;
 }
 
-static struct kobject *md_probe(dev_t dev, int *part, void *data)
+static void md_probe(dev_t dev)
 {
+	if (MAJOR(dev) == MD_MAJOR && MINOR(dev) >= 512)
+		return;
 	if (create_on_open)
 		md_alloc(dev, NULL);
-	return NULL;
 }
 
 static int add_named_array(const char *val, const struct kernel_param *kp)
@@ -6536,7 +6537,7 @@ static void autorun_devices(int part)
 			break;
 		}
 
-		md_probe(dev, NULL, NULL);
+		md_probe(dev);
 		mddev = mddev_find(dev);
 		if (!mddev || !mddev->gendisk) {
 			if (mddev)
@@ -9548,18 +9549,15 @@ static int __init md_init(void)
 	if (!md_misc_wq)
 		goto err_rdev_misc_wq;
 
-	if ((ret = register_blkdev(MD_MAJOR, "md")) < 0)
+	ret = __register_blkdev(MD_MAJOR, "md", md_probe);
+	if (ret < 0)
 		goto err_md;
 
-	if ((ret = register_blkdev(0, "mdp")) < 0)
+	ret = __register_blkdev(0, "mdp", md_probe);
+	if (ret < 0)
 		goto err_mdp;
 	mdp_major = ret;
 
-	blk_register_region(MKDEV(MD_MAJOR, 0), 512, THIS_MODULE,
-			    md_probe, NULL, NULL);
-	blk_register_region(MKDEV(mdp_major, 0), 1UL<<MINORBITS, THIS_MODULE,
-			    md_probe, NULL, NULL);
-
 	register_reboot_notifier(&md_notifier);
 	raid_table_header = register_sysctl_table(raid_root_table);
 
@@ -9826,9 +9824,6 @@ static __exit void md_exit(void)
 	struct list_head *tmp;
 	int delay = 1;
 
-	blk_unregister_region(MKDEV(MD_MAJOR,0), 512);
-	blk_unregister_region(MKDEV(mdp_major,0), 1U << MINORBITS);
-
 	unregister_blkdev(MD_MAJOR,"md");
 	unregister_blkdev(mdp_major, "mdp");
 	unregister_reboot_notifier(&md_notifier);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/19] ide: switch to __register_blkdev for command set probing
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (11 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 12/19] md: " Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 14/19] floppy: use a separate gendisk for each media format Christoph Hellwig
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

ide is the last user of the blk_register_region framework except for the
tracking of allocated gendisk.  Switch to __register_blkdev, even if that
doesn't allow us to trivially find out which command set to probe for.
That means we now always request all modules when a user tries to access
an unclaimed ide device node, but except for a few potentially loaded
modules for a fringe use case of a deprecated and soon to be removed
driver that doesn't make a difference.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ide/ide-probe.c | 34 ++++++----------------------------
 1 file changed, 6 insertions(+), 28 deletions(-)

diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index 076d34b381720f..1c1567bb519429 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -902,31 +902,12 @@ static int init_irq (ide_hwif_t *hwif)
 	return 1;
 }
 
-static int ata_lock(dev_t dev, void *data)
+static void ata_probe(dev_t dev)
 {
-	/* FIXME: we want to pin hwif down */
-	return 0;
-}
-
-static struct kobject *ata_probe(dev_t dev, int *part, void *data)
-{
-	ide_hwif_t *hwif = data;
-	int unit = *part >> PARTN_BITS;
-	ide_drive_t *drive = hwif->devices[unit];
-
-	if ((drive->dev_flags & IDE_DFLAG_PRESENT) == 0)
-		return NULL;
-
-	if (drive->media == ide_disk)
-		request_module("ide-disk");
-	if (drive->media == ide_cdrom || drive->media == ide_optical)
-		request_module("ide-cd");
-	if (drive->media == ide_tape)
-		request_module("ide-tape");
-	if (drive->media == ide_floppy)
-		request_module("ide-floppy");
-
-	return NULL;
+	request_module("ide-disk");
+	request_module("ide-cd");
+	request_module("ide-tape");
+	request_module("ide-floppy");
 }
 
 void ide_init_disk(struct gendisk *disk, ide_drive_t *drive)
@@ -967,7 +948,7 @@ static int hwif_init(ide_hwif_t *hwif)
 		return 0;
 	}
 
-	if (register_blkdev(hwif->major, hwif->name))
+	if (__register_blkdev(hwif->major, hwif->name, ata_probe))
 		return 0;
 
 	if (!hwif->sg_max_nents)
@@ -989,8 +970,6 @@ static int hwif_init(ide_hwif_t *hwif)
 		goto out;
 	}
 
-	blk_register_region(MKDEV(hwif->major, 0), MAX_DRIVES << PARTN_BITS,
-			    THIS_MODULE, ata_probe, ata_lock, hwif);
 	return 1;
 
 out:
@@ -1582,7 +1561,6 @@ static void ide_unregister(ide_hwif_t *hwif)
 	/*
 	 * Remove us from the kernel's knowledge
 	 */
-	blk_unregister_region(MKDEV(hwif->major, 0), MAX_DRIVES<<PARTN_BITS);
 	kfree(hwif->sg_table);
 	unregister_blkdev(hwif->major, hwif->name);
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/19] floppy: use a separate gendisk for each media format
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (12 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 13/19] ide: switch to __register_blkdev for command set probing Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 15/19] amiflop: use separate gendisks for Amiga vs MS-DOS mode Christoph Hellwig
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

The floppy driver usually autodetects the media when used with the
normal /dev/fd? devices, which also are the only nodes created by udev.
But it also supports various aliases that force a given media format.
That is currently supported using the blk_register_region framework
which finds the floppy gendisk even for a 'mismatched' dev_t.  The
problem with this (besides the code complexity) is that it creates
multiple struct block_device instances for the whole device of a
single gendisk, which can lead to interesting issues in code not
aware of that fact.

To fix this just create a separate gendisk for each of the aliases
if they are accessed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/floppy.c | 154 ++++++++++++++++++++++++++---------------
 1 file changed, 97 insertions(+), 57 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index a563b023458a8b..adf2b4dbd1ea09 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -402,7 +402,6 @@ static struct floppy_drive_params drive_params[N_DRIVE];
 static struct floppy_drive_struct drive_state[N_DRIVE];
 static struct floppy_write_errors write_errors[N_DRIVE];
 static struct timer_list motor_off_timer[N_DRIVE];
-static struct gendisk *disks[N_DRIVE];
 static struct blk_mq_tag_set tag_sets[N_DRIVE];
 static struct block_device *opened_bdev[N_DRIVE];
 static DEFINE_MUTEX(open_lock);
@@ -477,6 +476,8 @@ static struct floppy_struct floppy_type[32] = {
 	{ 3200,20,2,80,0,0x1C,0x00,0xCF,0x2C,"H1600" }, /* 31 1.6MB 3.5"    */
 };
 
+static struct gendisk *disks[N_DRIVE][ARRAY_SIZE(floppy_type)];
+
 #define SECTSIZE (_FD_SECTSIZE(*floppy))
 
 /* Auto-detection: Disk type used until the next media change occurs. */
@@ -4109,7 +4110,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
 
 	new_dev = MINOR(bdev->bd_dev);
 	drive_state[drive].fd_device = new_dev;
-	set_capacity(disks[drive], floppy_sizes[new_dev]);
+	set_capacity(disks[drive][ITYPE(new_dev)], floppy_sizes[new_dev]);
 	if (old_dev != -1 && old_dev != new_dev) {
 		if (buffer_drive == drive)
 			buffer_track = -1;
@@ -4577,15 +4578,58 @@ static bool floppy_available(int drive)
 	return true;
 }
 
-static struct kobject *floppy_find(dev_t dev, int *part, void *data)
+static int floppy_alloc_disk(unsigned int drive, unsigned int type)
 {
-	int drive = (*part & 3) | ((*part & 0x80) >> 5);
-	if (drive >= N_DRIVE || !floppy_available(drive))
-		return NULL;
-	if (((*part >> 2) & 0x1f) >= ARRAY_SIZE(floppy_type))
-		return NULL;
-	*part = 0;
-	return get_disk_and_module(disks[drive]);
+	struct gendisk *disk;
+	int err;
+
+	disk = alloc_disk(1);
+	if (!disk)
+		return -ENOMEM;
+
+	disk->queue = blk_mq_init_queue(&tag_sets[drive]);
+	if (IS_ERR(disk->queue)) {
+		err = PTR_ERR(disk);
+		disk->queue = NULL;
+		put_disk(disk);
+		return err;
+	}
+
+	blk_queue_bounce_limit(disk->queue, BLK_BOUNCE_HIGH);
+	blk_queue_max_hw_sectors(disk->queue, 64);
+	disk->major = FLOPPY_MAJOR;
+	disk->first_minor = TOMINOR(drive) | (type << 2);
+	disk->fops = &floppy_fops;
+	disk->events = DISK_EVENT_MEDIA_CHANGE;
+	if (type)
+		sprintf(disk->disk_name, "fd%d_type%d", drive, type);
+	else
+		sprintf(disk->disk_name, "fd%d", drive);
+	/* to be cleaned up... */
+	disk->private_data = (void *)(long)drive;
+	disk->flags |= GENHD_FL_REMOVABLE;
+
+	disks[drive][type] = disk;
+	return 0;
+}
+
+static DEFINE_MUTEX(floppy_probe_lock);
+
+static void floppy_probe(dev_t dev)
+{
+	unsigned int drive = (MINOR(dev) & 3) | ((MINOR(dev) & 0x80) >> 5);
+	unsigned int type = (MINOR(dev) >> 2) & 0x1f;
+
+	if (drive >= N_DRIVE || !floppy_available(drive) ||
+	    type >= ARRAY_SIZE(floppy_type))
+		return;
+
+	mutex_lock(&floppy_probe_lock);
+	if (!disks[drive][type]) {
+		if (floppy_alloc_disk(drive, type) == 0)
+			add_disk(disks[drive][type]);
+	}
+	mutex_unlock(&floppy_probe_lock);
 }
 
 static int __init do_floppy_init(void)
@@ -4607,33 +4651,25 @@ static int __init do_floppy_init(void)
 		return -ENOMEM;
 
 	for (drive = 0; drive < N_DRIVE; drive++) {
-		disks[drive] = alloc_disk(1);
-		if (!disks[drive]) {
-			err = -ENOMEM;
+		memset(&tag_sets[drive], 0, sizeof(tag_sets[drive]));
+		tag_sets[drive].ops = &floppy_mq_ops;
+		tag_sets[drive].nr_hw_queues = 1;
+		tag_sets[drive].nr_maps = 1;
+		tag_sets[drive].queue_depth = 2;
+		tag_sets[drive].numa_node = NUMA_NO_NODE;
+		tag_sets[drive].flags = BLK_MQ_F_SHOULD_MERGE;
+		err = blk_mq_alloc_tag_set(&tag_sets[drive]);
+		if (err)
 			goto out_put_disk;
-		}
 
-		disks[drive]->queue = blk_mq_init_sq_queue(&tag_sets[drive],
-							   &floppy_mq_ops, 2,
-							   BLK_MQ_F_SHOULD_MERGE);
-		if (IS_ERR(disks[drive]->queue)) {
-			err = PTR_ERR(disks[drive]->queue);
-			disks[drive]->queue = NULL;
+		err = floppy_alloc_disk(drive, 0);
+		if (err)
 			goto out_put_disk;
-		}
-
-		blk_queue_bounce_limit(disks[drive]->queue, BLK_BOUNCE_HIGH);
-		blk_queue_max_hw_sectors(disks[drive]->queue, 64);
-		disks[drive]->major = FLOPPY_MAJOR;
-		disks[drive]->first_minor = TOMINOR(drive);
-		disks[drive]->fops = &floppy_fops;
-		disks[drive]->events = DISK_EVENT_MEDIA_CHANGE;
-		sprintf(disks[drive]->disk_name, "fd%d", drive);
 
 		timer_setup(&motor_off_timer[drive], motor_off_callback, 0);
 	}
 
-	err = register_blkdev(FLOPPY_MAJOR, "fd");
+	err = __register_blkdev(FLOPPY_MAJOR, "fd", floppy_probe);
 	if (err)
 		goto out_put_disk;
 
@@ -4641,9 +4677,6 @@ static int __init do_floppy_init(void)
 	if (err)
 		goto out_unreg_blkdev;
 
-	blk_register_region(MKDEV(FLOPPY_MAJOR, 0), 256, THIS_MODULE,
-			    floppy_find, NULL, NULL);
-
 	for (i = 0; i < 256; i++)
 		if (ITYPE(i))
 			floppy_sizes[i] = floppy_type[ITYPE(i)].size;
@@ -4671,7 +4704,7 @@ static int __init do_floppy_init(void)
 	if (fdc_state[0].address == -1) {
 		cancel_delayed_work(&fd_timeout);
 		err = -ENODEV;
-		goto out_unreg_region;
+		goto out_unreg_driver;
 	}
 #if N_FDC > 1
 	fdc_state[1].address = FDC2;
@@ -4682,7 +4715,7 @@ static int __init do_floppy_init(void)
 	if (err) {
 		cancel_delayed_work(&fd_timeout);
 		err = -EBUSY;
-		goto out_unreg_region;
+		goto out_unreg_driver;
 	}
 
 	/* initialise drive state */
@@ -4759,10 +4792,8 @@ static int __init do_floppy_init(void)
 		if (err)
 			goto out_remove_drives;
 
-		/* to be cleaned up... */
-		disks[drive]->private_data = (void *)(long)drive;
-		disks[drive]->flags |= GENHD_FL_REMOVABLE;
-		device_add_disk(&floppy_device[drive].dev, disks[drive], NULL);
+		device_add_disk(&floppy_device[drive].dev, disks[drive][0],
+				NULL);
 	}
 
 	return 0;
@@ -4770,30 +4801,27 @@ static int __init do_floppy_init(void)
 out_remove_drives:
 	while (drive--) {
 		if (floppy_available(drive)) {
-			del_gendisk(disks[drive]);
+			del_gendisk(disks[drive][0]);
 			platform_device_unregister(&floppy_device[drive]);
 		}
 	}
 out_release_dma:
 	if (atomic_read(&usage_count))
 		floppy_release_irq_and_dma();
-out_unreg_region:
-	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
+out_unreg_driver:
 	platform_driver_unregister(&floppy_driver);
 out_unreg_blkdev:
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 out_put_disk:
 	destroy_workqueue(floppy_wq);
 	for (drive = 0; drive < N_DRIVE; drive++) {
-		if (!disks[drive])
+		if (!disks[drive][0])
 			break;
-		if (disks[drive]->queue) {
-			del_timer_sync(&motor_off_timer[drive]);
-			blk_cleanup_queue(disks[drive]->queue);
-			disks[drive]->queue = NULL;
-			blk_mq_free_tag_set(&tag_sets[drive]);
-		}
-		put_disk(disks[drive]);
+		del_timer_sync(&motor_off_timer[drive]);
+		blk_cleanup_queue(disks[drive][0]->queue);
+		disks[drive][0]->queue = NULL;
+		blk_mq_free_tag_set(&tag_sets[drive]);
+		put_disk(disks[drive][0]);
 	}
 	return err;
 }
@@ -5004,9 +5032,8 @@ module_init(floppy_module_init);
 
 static void __exit floppy_module_exit(void)
 {
-	int drive;
+	int drive, i;
 
-	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 	platform_driver_unregister(&floppy_driver);
 
@@ -5016,10 +5043,16 @@ static void __exit floppy_module_exit(void)
 		del_timer_sync(&motor_off_timer[drive]);
 
 		if (floppy_available(drive)) {
-			del_gendisk(disks[drive]);
+			for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
+				if (disks[drive][i])
+					del_gendisk(disks[drive][i]);
+			}
 			platform_device_unregister(&floppy_device[drive]);
 		}
-		blk_cleanup_queue(disks[drive]->queue);
+		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
+			if (disks[drive][i])
+				blk_cleanup_queue(disks[drive][i]->queue);
+		}
 		blk_mq_free_tag_set(&tag_sets[drive]);
 
 		/*
@@ -5027,10 +5060,17 @@ static void __exit floppy_module_exit(void)
 		 * queue reference in put_disk().
 		 */
 		if (!(allowed_drive_mask & (1 << drive)) ||
-		    fdc_state[FDC(drive)].version == FDC_NONE)
-			disks[drive]->queue = NULL;
+		    fdc_state[FDC(drive)].version == FDC_NONE) {
+			for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
+				if (disks[drive][i])
+					disks[drive][i]->queue = NULL;
+			}
+		}
 
-		put_disk(disks[drive]);
+		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
+			if (disks[drive][i])
+				put_disk(disks[drive][i]);
+		}
 	}
 
 	cancel_delayed_work_sync(&fd_timeout);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 15/19] amiflop: use separate gendisks for Amiga vs MS-DOS mode
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (13 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 14/19] floppy: use a separate gendisk for each media format Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 16/19] ataflop: use a separate gendisk for each media format Christoph Hellwig
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Use separate gendisks (which share a tag_set) for the native Amgiga vs
the MS-DOS mode instead of redirecting the gendisk lookup using a probe
callback.  This avoids potential problems with aliased block_device
instances and will eventually allow for removing the blk_register_region
framework.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/amiflop.c | 98 +++++++++++++++++++++++------------------
 1 file changed, 55 insertions(+), 43 deletions(-)

diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 226219da3da6a7..de2bad8d1512f2 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -201,7 +201,7 @@ struct amiga_floppy_struct {
 	int busy;			/* true when drive is active */
 	int dirty;			/* true when trackbuf is not on disk */
 	int status;			/* current error code for unit */
-	struct gendisk *gendisk;
+	struct gendisk *gendisk[2];
 	struct blk_mq_tag_set tag_set;
 };
 
@@ -1669,6 +1669,11 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
 		return -EBUSY;
 	}
 
+	if (unit[drive].type->code == FD_NODRIVE) {
+		mutex_unlock(&amiflop_mutex);
+		return -ENXIO;
+	}
+
 	if (mode & (FMODE_READ|FMODE_WRITE)) {
 		check_disk_change(bdev);
 		if (mode & FMODE_WRITE) {
@@ -1695,7 +1700,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
 	unit[drive].dtype=&data_types[system];
 	unit[drive].blocks=unit[drive].type->heads*unit[drive].type->tracks*
 		data_types[system].sects*unit[drive].type->sect_mult;
-	set_capacity(unit[drive].gendisk, unit[drive].blocks);
+	set_capacity(unit[drive].gendisk[system], unit[drive].blocks);
 
 	printk(KERN_INFO "fd%d: accessing %s-disk with %s-layout\n",drive,
 	       unit[drive].type->name, data_types[system].name);
@@ -1772,36 +1777,68 @@ static const struct blk_mq_ops amiflop_mq_ops = {
 	.queue_rq = amiflop_queue_rq,
 };
 
-static struct gendisk *fd_alloc_disk(int drive)
+static int fd_alloc_disk(int drive, int system)
 {
 	struct gendisk *disk;
 
 	disk = alloc_disk(1);
 	if (!disk)
 		goto out;
-
-	disk->queue = blk_mq_init_sq_queue(&unit[drive].tag_set, &amiflop_mq_ops,
-						2, BLK_MQ_F_SHOULD_MERGE);
-	if (IS_ERR(disk->queue)) {
-		disk->queue = NULL;
+	disk->queue = blk_mq_init_queue(&unit[drive].tag_set);
+	if (IS_ERR(disk->queue))
 		goto out_put_disk;
-	}
 
+	disk->major = FLOPPY_MAJOR;
+	disk->first_minor = drive + system;
+	disk->fops = &floppy_fops;
+	disk->events = DISK_EVENT_MEDIA_CHANGE;
+	if (system)
+		sprintf(disk->disk_name, "fd%d_msdos", drive);
+	else
+		sprintf(disk->disk_name, "fd%d", drive);
+	disk->private_data = &unit[drive];
+	set_capacity(disk, 880 * 2);
+
+	unit[drive].gendisk[system] = disk;
+	add_disk(disk);
+	return 0;
+
+out_put_disk:
+	disk->queue = NULL;
+	put_disk(disk);
+out:
+	return -ENOMEM;
+}
+
+static int fd_alloc_drive(int drive)
+{
 	unit[drive].trackbuf = kmalloc(FLOPPY_MAX_SECTORS * 512, GFP_KERNEL);
 	if (!unit[drive].trackbuf)
-		goto out_cleanup_queue;
+		goto out;
 
-	return disk;
+	memset(&unit[drive].tag_set, 0, sizeof(unit[drive].tag_set));
+	unit[drive].tag_set.ops = &amiflop_mq_ops;
+	unit[drive].tag_set.nr_hw_queues = 1;
+	unit[drive].tag_set.nr_maps = 1;
+	unit[drive].tag_set.queue_depth = 2;
+	unit[drive].tag_set.numa_node = NUMA_NO_NODE;
+	unit[drive].tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
+	if (blk_mq_alloc_tag_set(&unit[drive].tag_set))
+		goto out_cleanup_trackbuf;
 
-out_cleanup_queue:
-	blk_cleanup_queue(disk->queue);
-	disk->queue = NULL;
+	pr_cont(" fd%d", drive);
+
+	if (fd_alloc_disk(drive, 0) || fd_alloc_disk(drive, 1))
+		goto out_cleanup_tagset;
+	return 0;
+
+out_cleanup_tagset:
 	blk_mq_free_tag_set(&unit[drive].tag_set);
-out_put_disk:
-	put_disk(disk);
+out_cleanup_trackbuf:
+	kfree(unit[drive].trackbuf);
 out:
 	unit[drive].type->code = FD_NODRIVE;
-	return NULL;
+	return -ENOMEM;
 }
 
 static int __init fd_probe_drives(void)
@@ -1812,29 +1849,16 @@ static int __init fd_probe_drives(void)
 	drives=0;
 	nomem=0;
 	for(drive=0;drive<FD_MAX_UNITS;drive++) {
-		struct gendisk *disk;
 		fd_probe(drive);
 		if (unit[drive].type->code == FD_NODRIVE)
 			continue;
 
-		disk = fd_alloc_disk(drive);
-		if (!disk) {
+		if (fd_alloc_drive(drive) < 0) {
 			pr_cont(" no mem for fd%d", drive);
 			nomem = 1;
 			continue;
 		}
-		unit[drive].gendisk = disk;
 		drives++;
-
-		pr_cont(" fd%d",drive);
-		disk->major = FLOPPY_MAJOR;
-		disk->first_minor = drive;
-		disk->fops = &floppy_fops;
-		disk->events = DISK_EVENT_MEDIA_CHANGE;
-		sprintf(disk->disk_name, "fd%d", drive);
-		disk->private_data = &unit[drive];
-		set_capacity(disk, 880*2);
-		add_disk(disk);
 	}
 	if ((drives > 0) || (nomem == 0)) {
 		if (drives == 0)
@@ -1846,15 +1870,6 @@ static int __init fd_probe_drives(void)
 	return -ENOMEM;
 }
  
-static struct kobject *floppy_find(dev_t dev, int *part, void *data)
-{
-	int drive = *part & 3;
-	if (unit[drive].type->code == FD_NODRIVE)
-		return NULL;
-	*part = 0;
-	return get_disk_and_module(unit[drive].gendisk);
-}
-
 static int __init amiga_floppy_probe(struct platform_device *pdev)
 {
 	int i, ret;
@@ -1884,9 +1899,6 @@ static int __init amiga_floppy_probe(struct platform_device *pdev)
 	if (fd_probe_drives() < 1) /* No usable drives */
 		goto out_probe;
 
-	blk_register_region(MKDEV(FLOPPY_MAJOR, 0), 256, THIS_MODULE,
-				floppy_find, NULL, NULL);
-
 	/* initialize variables */
 	timer_setup(&motor_on_timer, motor_on_callback, 0);
 	motor_on_timer.expires = 0;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 16/19] ataflop: use a separate gendisk for each media format
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (14 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 15/19] amiflop: use separate gendisks for Amiga vs MS-DOS mode Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 17/19] z2ram: reindent Christoph Hellwig
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

The Atari floppy driver usually autodetects the media when used with the
ormal /dev/fd? devices, which also are the only nodes created by udev.
But it also supports various aliases that force a given media format.
That is currently supported using the blk_register_region framework
which finds the floppy gendisk even for a 'mismatched' dev_t.  The
problem with this (besides the code complexity) is that it creates
multiple struct block_device instances for the whole device of a
single gendisk, which can lead to interesting issues in code not
aware of that fact.

To fix this just create a separate gendisk for each of the aliases
if they are accessed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/ataflop.c | 135 +++++++++++++++++++++++++---------------
 1 file changed, 86 insertions(+), 49 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index a50e13af030526..b5acc85328c329 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -297,7 +297,7 @@ static struct atari_floppy_struct {
 	unsigned int wpstat;	/* current state of WP signal (for
 				   disk change detection) */
 	int flags;		/* flags */
-	struct gendisk *disk;
+	struct gendisk *disk[NUM_DISK_MINORS];
 	int ref;
 	int type;
 	struct blk_mq_tag_set tag_set;
@@ -723,12 +723,16 @@ static void fd_error( void )
 
 static int do_format(int drive, int type, struct atari_format_descr *desc)
 {
-	struct request_queue *q = unit[drive].disk->queue;
+	struct request_queue *q;
 	unsigned char	*p;
 	int sect, nsect;
 	unsigned long	flags;
 	int ret;
 
+	if (type)
+		type--;
+
+	q = unit[drive].disk[type]->queue;
 	blk_mq_freeze_queue(q);
 	blk_mq_quiesce_queue(q);
 
@@ -738,7 +742,7 @@ static int do_format(int drive, int type, struct atari_format_descr *desc)
 	local_irq_restore(flags);
 
 	if (type) {
-		if (--type >= NUM_DISK_MINORS ||
+		if (type >= NUM_DISK_MINORS ||
 		    minor2disktype[type].drive_types > DriveType) {
 			ret = -EINVAL;
 			goto out;
@@ -1154,7 +1158,7 @@ static void fd_rwsec_done1(int status)
 			    if (SUDT[-1].blocks > ReqBlock) {
 				/* try another disk type */
 				SUDT--;
-				set_capacity(unit[SelectedDrive].disk,
+				set_capacity(unit[SelectedDrive].disk[0],
 							SUDT->blocks);
 			    } else
 				Probing = 0;
@@ -1169,7 +1173,7 @@ static void fd_rwsec_done1(int status)
 /* record not found, but not probing. Maybe stretch wrong ? Restart probing */
 			if (SUD.autoprobe) {
 				SUDT = atari_disk_type + StartDiskType[DriveType];
-				set_capacity(unit[SelectedDrive].disk,
+				set_capacity(unit[SelectedDrive].disk[0],
 							SUDT->blocks);
 				Probing = 1;
 			}
@@ -1515,7 +1519,7 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
 		if (!UDT) {
 			Probing = 1;
 			UDT = atari_disk_type + StartDiskType[DriveType];
-			set_capacity(floppy->disk, UDT->blocks);
+			set_capacity(bd->rq->rq_disk, UDT->blocks);
 			UD.autoprobe = 1;
 		}
 	} 
@@ -1533,7 +1537,7 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
 		}
 		type = minor2disktype[type].index;
 		UDT = &atari_disk_type[type];
-		set_capacity(floppy->disk, UDT->blocks);
+		set_capacity(bd->rq->rq_disk, UDT->blocks);
 		UD.autoprobe = 0;
 	}
 
@@ -1658,7 +1662,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode,
 				    printk (KERN_INFO "floppy%d: setting %s %p!\n",
 				        drive, dtp->name, dtp);
 				UDT = dtp;
-				set_capacity(floppy->disk, UDT->blocks);
+				set_capacity(disk, UDT->blocks);
 
 				if (cmd == FDDEFPRM) {
 				  /* save settings as permanent default type */
@@ -1702,7 +1706,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode,
 			return -EINVAL;
 
 		UDT = dtp;
-		set_capacity(floppy->disk, UDT->blocks);
+		set_capacity(disk, UDT->blocks);
 
 		return 0;
 	case FDMSGON:
@@ -1725,7 +1729,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode,
 		UDT = NULL;
 		/* MSch: invalidate default_params */
 		default_params[drive].blocks  = 0;
-		set_capacity(floppy->disk, MAX_DISK_SIZE * 2);
+		set_capacity(disk, MAX_DISK_SIZE * 2);
 		fallthrough;
 	case FDFMTEND:
 	case FDFLUSH:
@@ -1961,14 +1965,50 @@ static const struct blk_mq_ops ataflop_mq_ops = {
 	.commit_rqs = ataflop_commit_rqs,
 };
 
-static struct kobject *floppy_find(dev_t dev, int *part, void *data)
+static int ataflop_alloc_disk(unsigned int drive, unsigned int type)
 {
-	int drive = *part & 3;
-	int type  = *part >> 2;
+	struct gendisk *disk;
+	int ret;
+
+	disk = alloc_disk(1);
+	if (!disk)
+		return -ENOMEM;
+
+	disk->queue = blk_mq_init_queue(&unit[drive].tag_set);
+	if (IS_ERR(disk->queue)) {
+		ret = PTR_ERR(disk->queue);
+		disk->queue = NULL;
+		put_disk(disk);
+		return ret;
+	}
+
+	disk->major = FLOPPY_MAJOR;
+	disk->first_minor = drive + (type << 2);
+	sprintf(disk->disk_name, "fd%d", drive);
+	disk->fops = &floppy_fops;
+	disk->events = DISK_EVENT_MEDIA_CHANGE;
+	disk->private_data = &unit[drive];
+	set_capacity(disk, MAX_DISK_SIZE * 2);
+
+	unit[drive].disk[type] = disk;
+	return 0;
+}
+
+static DEFINE_MUTEX(ataflop_probe_lock);
+
+static void ataflop_probe(dev_t dev)
+{
+	int drive = MINOR(dev) & 3;
+	int type  = MINOR(dev) >> 2;
+
 	if (drive >= FD_MAX_UNITS || type > NUM_DISK_MINORS)
-		return NULL;
-	*part = 0;
-	return get_disk_and_module(unit[drive].disk);
+		return;
+	mutex_lock(&ataflop_probe_lock);
+	if (!unit[drive].disk[type]) {
+		if (ataflop_alloc_disk(drive, type) == 0)
+			add_disk(unit[drive].disk[type]);
+	}
+	mutex_unlock(&ataflop_probe_lock);
 }
 
 static int __init atari_floppy_init (void)
@@ -1980,23 +2020,26 @@ static int __init atari_floppy_init (void)
 		/* Amiga, Mac, ... don't have Atari-compatible floppy :-) */
 		return -ENODEV;
 
-	if (register_blkdev(FLOPPY_MAJOR,"fd"))
-		return -EBUSY;
+	mutex_lock(&ataflop_probe_lock);
+	ret = __register_blkdev(FLOPPY_MAJOR, "fd", ataflop_probe);
+	if (ret)
+		goto out_unlock;
 
 	for (i = 0; i < FD_MAX_UNITS; i++) {
-		unit[i].disk = alloc_disk(1);
-		if (!unit[i].disk) {
-			ret = -ENOMEM;
+		memset(&unit[i].tag_set, 0, sizeof(unit[i].tag_set));
+		unit[i].tag_set.ops = &ataflop_mq_ops;
+		unit[i].tag_set.nr_hw_queues = 1;
+		unit[i].tag_set.nr_maps = 1;
+		unit[i].tag_set.queue_depth = 2;
+		unit[i].tag_set.numa_node = NUMA_NO_NODE;
+		unit[i].tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
+		ret = blk_mq_alloc_tag_set(&unit[i].tag_set);
+		if (ret)
 			goto err;
-		}
 
-		unit[i].disk->queue = blk_mq_init_sq_queue(&unit[i].tag_set,
-							   &ataflop_mq_ops, 2,
-							   BLK_MQ_F_SHOULD_MERGE);
-		if (IS_ERR(unit[i].disk->queue)) {
-			put_disk(unit[i].disk);
-			ret = PTR_ERR(unit[i].disk->queue);
-			unit[i].disk->queue = NULL;
+		ret = ataflop_alloc_disk(i, 0);
+		if (ret) {
+			blk_mq_free_tag_set(&unit[i].tag_set);
 			goto err;
 		}
 	}
@@ -2026,19 +2069,9 @@ static int __init atari_floppy_init (void)
 	for (i = 0; i < FD_MAX_UNITS; i++) {
 		unit[i].track = -1;
 		unit[i].flags = 0;
-		unit[i].disk->major = FLOPPY_MAJOR;
-		unit[i].disk->first_minor = i;
-		sprintf(unit[i].disk->disk_name, "fd%d", i);
-		unit[i].disk->fops = &floppy_fops;
-		unit[i].disk->events = DISK_EVENT_MEDIA_CHANGE;
-		unit[i].disk->private_data = &unit[i];
-		set_capacity(unit[i].disk, MAX_DISK_SIZE * 2);
-		add_disk(unit[i].disk);
+		add_disk(unit[i].disk[0]);
 	}
 
-	blk_register_region(MKDEV(FLOPPY_MAJOR, 0), 256, THIS_MODULE,
-				floppy_find, NULL, NULL);
-
 	printk(KERN_INFO "Atari floppy driver: max. %cD, %strack buffering\n",
 	       DriveType == 0 ? 'D' : DriveType == 1 ? 'H' : 'E',
 	       UseTrackbuffer ? "" : "no ");
@@ -2048,14 +2081,14 @@ static int __init atari_floppy_init (void)
 
 err:
 	while (--i >= 0) {
-		struct gendisk *disk = unit[i].disk;
-
-		blk_cleanup_queue(disk->queue);
+		blk_cleanup_queue(unit[i].disk[0]->queue);
+		put_disk(unit[i].disk[0]);
 		blk_mq_free_tag_set(&unit[i].tag_set);
-		put_disk(unit[i].disk);
 	}
 
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
+out_unlock:
+	mutex_unlock(&ataflop_probe_lock);
 	return ret;
 }
 
@@ -2100,13 +2133,17 @@ __setup("floppy=", atari_floppy_setup);
 
 static void __exit atari_floppy_exit(void)
 {
-	int i;
-	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
+	int i, type;
+
 	for (i = 0; i < FD_MAX_UNITS; i++) {
-		del_gendisk(unit[i].disk);
-		blk_cleanup_queue(unit[i].disk->queue);
+		for (type = 0; type < NUM_DISK_MINORS; type++) {
+			if (!unit[i].disk[type])
+				continue;
+			del_gendisk(unit[i].disk[type]);
+			blk_cleanup_queue(unit[i].disk[type]->queue);
+			put_disk(unit[i].disk[type]);
+		}
 		blk_mq_free_tag_set(&unit[i].tag_set);
-		put_disk(unit[i].disk);
 	}
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 17/19] z2ram: reindent
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (15 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 16/19] ataflop: use a separate gendisk for each media format Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  9:12   ` Joe Perches
  2020-08-26  6:24 ` [PATCH 18/19] z2ram: use separate gendisk for the different modes Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 19/19] block: switch gendisk lookup to a simple xarray Christoph Hellwig
  18 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

reindent the driver using Lident as the code style was far away from
normal Linux code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/z2ram.c | 497 ++++++++++++++++++++----------------------
 1 file changed, 237 insertions(+), 260 deletions(-)

diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c
index 0e734802ee7cc6..566c653399d8d3 100644
--- a/drivers/block/z2ram.c
+++ b/drivers/block/z2ram.c
@@ -42,7 +42,6 @@
 
 #include <linux/zorro.h>
 
-
 #define Z2MINOR_COMBINED      (0)
 #define Z2MINOR_Z2ONLY        (1)
 #define Z2MINOR_CHIPONLY      (2)
@@ -52,15 +51,15 @@
 #define Z2MINOR_MEMLIST4      (7)
 #define Z2MINOR_COUNT         (8) /* Move this down when adding a new minor */
 
-#define Z2RAM_CHUNK1024       ( Z2RAM_CHUNKSIZE >> 10 )
+#define Z2RAM_CHUNK1024       (Z2RAM_CHUNKSIZE >> 10)
 
 static DEFINE_MUTEX(z2ram_mutex);
-static u_long *z2ram_map    = NULL;
-static u_long z2ram_size    = 0;
-static int z2_count         = 0;
-static int chip_count       = 0;
-static int list_count       = 0;
-static int current_device   = -1;
+static u_long *z2ram_map = NULL;
+static u_long z2ram_size = 0;
+static int z2_count = 0;
+static int chip_count = 0;
+static int list_count = 0;
+static int current_device = -1;
 
 static DEFINE_SPINLOCK(z2ram_lock);
 
@@ -71,13 +70,12 @@ static blk_status_t z2_queue_rq(struct blk_mq_hw_ctx *hctx,
 {
 	struct request *req = bd->rq;
 	unsigned long start = blk_rq_pos(req) << 9;
-	unsigned long len  = blk_rq_cur_bytes(req);
+	unsigned long len = blk_rq_cur_bytes(req);
 
 	blk_mq_start_request(req);
 
 	if (start + len > z2ram_size) {
-		pr_err(DEVICE_NAME ": bad access: block=%llu, "
-		       "count=%u\n",
+		pr_err(DEVICE_NAME ": bad access: block=%llu, count=%u\n",
 		       (unsigned long long)blk_rq_pos(req),
 		       blk_rq_cur_sectors(req));
 		return BLK_STS_IOERR;
@@ -92,7 +90,7 @@ static blk_status_t z2_queue_rq(struct blk_mq_hw_ctx *hctx,
 
 		if (len < size)
 			size = len;
-		addr += z2ram_map[ start >> Z2RAM_CHUNKSHIFT ];
+		addr += z2ram_map[start >> Z2RAM_CHUNKSHIFT];
 		if (rq_data_dir(req) == READ)
 			memcpy(buffer, (char *)addr, size);
 		else
@@ -106,228 +104,213 @@ static blk_status_t z2_queue_rq(struct blk_mq_hw_ctx *hctx,
 	return BLK_STS_OK;
 }
 
-static void
-get_z2ram( void )
+static void get_z2ram(void)
 {
-    int i;
-
-    for ( i = 0; i < Z2RAM_SIZE / Z2RAM_CHUNKSIZE; i++ )
-    {
-	if ( test_bit( i, zorro_unused_z2ram ) )
-	{
-	    z2_count++;
-	    z2ram_map[z2ram_size++] = (unsigned long)ZTWO_VADDR(Z2RAM_START) +
-				      (i << Z2RAM_CHUNKSHIFT);
-	    clear_bit( i, zorro_unused_z2ram );
+	int i;
+
+	for (i = 0; i < Z2RAM_SIZE / Z2RAM_CHUNKSIZE; i++) {
+		if (test_bit(i, zorro_unused_z2ram)) {
+			z2_count++;
+			z2ram_map[z2ram_size++] =
+			    (unsigned long)ZTWO_VADDR(Z2RAM_START) +
+			    (i << Z2RAM_CHUNKSHIFT);
+			clear_bit(i, zorro_unused_z2ram);
+		}
 	}
-    }
 
-    return;
+	return;
 }
 
-static void
-get_chipram( void )
+static void get_chipram(void)
 {
 
-    while ( amiga_chip_avail() > ( Z2RAM_CHUNKSIZE * 4 ) )
-    {
-	chip_count++;
-	z2ram_map[ z2ram_size ] =
-	    (u_long)amiga_chip_alloc( Z2RAM_CHUNKSIZE, "z2ram" );
+	while (amiga_chip_avail() > (Z2RAM_CHUNKSIZE * 4)) {
+		chip_count++;
+		z2ram_map[z2ram_size] =
+		    (u_long) amiga_chip_alloc(Z2RAM_CHUNKSIZE, "z2ram");
 
-	if ( z2ram_map[ z2ram_size ] == 0 )
-	{
-	    break;
+		if (z2ram_map[z2ram_size] == 0) {
+			break;
+		}
+
+		z2ram_size++;
 	}
 
-	z2ram_size++;
-    }
-	
-    return;
+	return;
 }
 
 static int z2_open(struct block_device *bdev, fmode_t mode)
 {
-    int device;
-    int max_z2_map = ( Z2RAM_SIZE / Z2RAM_CHUNKSIZE ) *
-	sizeof( z2ram_map[0] );
-    int max_chip_map = ( amiga_chip_size / Z2RAM_CHUNKSIZE ) *
-	sizeof( z2ram_map[0] );
-    int rc = -ENOMEM;
-
-    device = MINOR(bdev->bd_dev);
-
-    mutex_lock(&z2ram_mutex);
-    if ( current_device != -1 && current_device != device )
-    {
-	rc = -EBUSY;
-	goto err_out;
-    }
-
-    if ( current_device == -1 )
-    {
-	z2_count   = 0;
-	chip_count = 0;
-	list_count = 0;
-	z2ram_size = 0;
-
-	/* Use a specific list entry. */
-	if (device >= Z2MINOR_MEMLIST1 && device <= Z2MINOR_MEMLIST4) {
-		int index = device - Z2MINOR_MEMLIST1 + 1;
-		unsigned long size, paddr, vaddr;
-
-		if (index >= m68k_realnum_memory) {
-			printk( KERN_ERR DEVICE_NAME
-				": no such entry in z2ram_map\n" );
-		        goto err_out;
-		}
-
-		paddr = m68k_memory[index].addr;
-		size = m68k_memory[index].size & ~(Z2RAM_CHUNKSIZE-1);
-
-#ifdef __powerpc__
-		/* FIXME: ioremap doesn't build correct memory tables. */
-		{
-			vfree(vmalloc (size));
-		}
+	int device;
+	int max_z2_map = (Z2RAM_SIZE / Z2RAM_CHUNKSIZE) * sizeof(z2ram_map[0]);
+	int max_chip_map = (amiga_chip_size / Z2RAM_CHUNKSIZE) *
+	    sizeof(z2ram_map[0]);
+	int rc = -ENOMEM;
 
-		vaddr = (unsigned long)ioremap_wt(paddr, size);
+	device = MINOR(bdev->bd_dev);
 
-#else
-		vaddr = (unsigned long)z_remap_nocache_nonser(paddr, size);
-#endif
-		z2ram_map = 
-			kmalloc_array(size / Z2RAM_CHUNKSIZE,
-                                      sizeof(z2ram_map[0]),
-                                      GFP_KERNEL);
-		if ( z2ram_map == NULL )
-		{
-		    printk( KERN_ERR DEVICE_NAME
-			": cannot get mem for z2ram_map\n" );
-		    goto err_out;
-		}
-
-		while (size) {
-			z2ram_map[ z2ram_size++ ] = vaddr;
-			size -= Z2RAM_CHUNKSIZE;
-			vaddr += Z2RAM_CHUNKSIZE;
-			list_count++;
-		}
+	mutex_lock(&z2ram_mutex);
+	if (current_device != -1 && current_device != device) {
+		rc = -EBUSY;
+		goto err_out;
+	}
 
-		if ( z2ram_size != 0 )
-		    printk( KERN_INFO DEVICE_NAME
-			": using %iK List Entry %d Memory\n",
-			list_count * Z2RAM_CHUNK1024, index );
-	} else
-
-	switch ( device )
-	{
-	    case Z2MINOR_COMBINED:
-
-		z2ram_map = kmalloc( max_z2_map + max_chip_map, GFP_KERNEL );
-		if ( z2ram_map == NULL )
-		{
-		    printk( KERN_ERR DEVICE_NAME
-			": cannot get mem for z2ram_map\n" );
-		    goto err_out;
-		}
+	if (current_device == -1) {
+		z2_count = 0;
+		chip_count = 0;
+		list_count = 0;
+		z2ram_size = 0;
 
-		get_z2ram();
-		get_chipram();
-
-		if ( z2ram_size != 0 )
-		    printk( KERN_INFO DEVICE_NAME 
-			": using %iK Zorro II RAM and %iK Chip RAM (Total %dK)\n",
-			z2_count * Z2RAM_CHUNK1024,
-			chip_count * Z2RAM_CHUNK1024,
-			( z2_count + chip_count ) * Z2RAM_CHUNK1024 );
-
-	    break;
-
-    	    case Z2MINOR_Z2ONLY:
-		z2ram_map = kmalloc( max_z2_map, GFP_KERNEL );
-		if ( z2ram_map == NULL )
-		{
-		    printk( KERN_ERR DEVICE_NAME
-			": cannot get mem for z2ram_map\n" );
-		    goto err_out;
-		}
+		/* Use a specific list entry. */
+		if (device >= Z2MINOR_MEMLIST1 && device <= Z2MINOR_MEMLIST4) {
+			int index = device - Z2MINOR_MEMLIST1 + 1;
+			unsigned long size, paddr, vaddr;
 
-		get_z2ram();
+			if (index >= m68k_realnum_memory) {
+				printk(KERN_ERR DEVICE_NAME
+				       ": no such entry in z2ram_map\n");
+				goto err_out;
+			}
 
-		if ( z2ram_size != 0 )
-		    printk( KERN_INFO DEVICE_NAME 
-			": using %iK of Zorro II RAM\n",
-			z2_count * Z2RAM_CHUNK1024 );
+			paddr = m68k_memory[index].addr;
+			size = m68k_memory[index].size & ~(Z2RAM_CHUNKSIZE - 1);
 
-	    break;
+#ifdef __powerpc__
+			/*
+			 * FIXME: ioremap doesn't build correct memory tables.
+			 */
+			vfree(vmalloc(size));
+			vaddr = (unsigned long)ioremap_wt(paddr, size);
 
-	    case Z2MINOR_CHIPONLY:
-		z2ram_map = kmalloc( max_chip_map, GFP_KERNEL );
-		if ( z2ram_map == NULL )
-		{
-		    printk( KERN_ERR DEVICE_NAME
-			": cannot get mem for z2ram_map\n" );
-		    goto err_out;
+#else
+			vaddr =
+			    (unsigned long)z_remap_nocache_nonser(paddr, size);
+#endif
+			z2ram_map =
+			    kmalloc_array(size / Z2RAM_CHUNKSIZE,
+					  sizeof(z2ram_map[0]), GFP_KERNEL);
+			if (z2ram_map == NULL) {
+				printk(KERN_ERR DEVICE_NAME
+				       ": cannot get mem for z2ram_map\n");
+				goto err_out;
+			}
+
+			while (size) {
+				z2ram_map[z2ram_size++] = vaddr;
+				size -= Z2RAM_CHUNKSIZE;
+				vaddr += Z2RAM_CHUNKSIZE;
+				list_count++;
+			}
+
+			if (z2ram_size != 0)
+				printk(KERN_INFO DEVICE_NAME
+				       ": using %iK List Entry %d Memory\n",
+				       list_count * Z2RAM_CHUNK1024, index);
+		} else
+			switch (device) {
+			case Z2MINOR_COMBINED:
+
+				z2ram_map =
+				    kmalloc(max_z2_map + max_chip_map,
+					    GFP_KERNEL);
+				if (z2ram_map == NULL) {
+					printk(KERN_ERR DEVICE_NAME
+					       ": cannot get mem for z2ram_map\n");
+					goto err_out;
+				}
+
+				get_z2ram();
+				get_chipram();
+
+				if (z2ram_size != 0)
+					printk(KERN_INFO DEVICE_NAME
+					       ": using %iK Zorro II RAM and %iK Chip RAM (Total %dK)\n",
+					       z2_count * Z2RAM_CHUNK1024,
+					       chip_count * Z2RAM_CHUNK1024,
+					       (z2_count +
+						chip_count) * Z2RAM_CHUNK1024);
+
+				break;
+
+			case Z2MINOR_Z2ONLY:
+				z2ram_map = kmalloc(max_z2_map, GFP_KERNEL);
+				if (z2ram_map == NULL) {
+					printk(KERN_ERR DEVICE_NAME
+					       ": cannot get mem for z2ram_map\n");
+					goto err_out;
+				}
+
+				get_z2ram();
+
+				if (z2ram_size != 0)
+					printk(KERN_INFO DEVICE_NAME
+					       ": using %iK of Zorro II RAM\n",
+					       z2_count * Z2RAM_CHUNK1024);
+
+				break;
+
+			case Z2MINOR_CHIPONLY:
+				z2ram_map = kmalloc(max_chip_map, GFP_KERNEL);
+				if (z2ram_map == NULL) {
+					printk(KERN_ERR DEVICE_NAME
+					       ": cannot get mem for z2ram_map\n");
+					goto err_out;
+				}
+
+				get_chipram();
+
+				if (z2ram_size != 0)
+					printk(KERN_INFO DEVICE_NAME
+					       ": using %iK Chip RAM\n",
+					       chip_count * Z2RAM_CHUNK1024);
+
+				break;
+
+			default:
+				rc = -ENODEV;
+				goto err_out;
+
+				break;
+			}
+
+		if (z2ram_size == 0) {
+			printk(KERN_NOTICE DEVICE_NAME
+			       ": no unused ZII/Chip RAM found\n");
+			goto err_out_kfree;
 		}
 
-		get_chipram();
-
-		if ( z2ram_size != 0 )
-		    printk( KERN_INFO DEVICE_NAME 
-			": using %iK Chip RAM\n",
-			chip_count * Z2RAM_CHUNK1024 );
-		    
-	    break;
-
-	    default:
-		rc = -ENODEV;
-		goto err_out;
-	
-	    break;
-	}
-
-	if ( z2ram_size == 0 )
-	{
-	    printk( KERN_NOTICE DEVICE_NAME
-		": no unused ZII/Chip RAM found\n" );
-	    goto err_out_kfree;
+		current_device = device;
+		z2ram_size <<= Z2RAM_CHUNKSHIFT;
+		set_capacity(z2ram_gendisk, z2ram_size >> 9);
 	}
 
-	current_device = device;
-	z2ram_size <<= Z2RAM_CHUNKSHIFT;
-	set_capacity(z2ram_gendisk, z2ram_size >> 9);
-    }
-
-    mutex_unlock(&z2ram_mutex);
-    return 0;
+	mutex_unlock(&z2ram_mutex);
+	return 0;
 
 err_out_kfree:
-    kfree(z2ram_map);
+	kfree(z2ram_map);
 err_out:
-    mutex_unlock(&z2ram_mutex);
-    return rc;
+	mutex_unlock(&z2ram_mutex);
+	return rc;
 }
 
-static void
-z2_release(struct gendisk *disk, fmode_t mode)
+static void z2_release(struct gendisk *disk, fmode_t mode)
 {
-    mutex_lock(&z2ram_mutex);
-    if ( current_device == -1 ) {
-    	mutex_unlock(&z2ram_mutex);
-    	return;
-    }
-    mutex_unlock(&z2ram_mutex);
-    /*
-     * FIXME: unmap memory
-     */
+	mutex_lock(&z2ram_mutex);
+	if (current_device == -1) {
+		mutex_unlock(&z2ram_mutex);
+		return;
+	}
+	mutex_unlock(&z2ram_mutex);
+	/*
+	 * FIXME: unmap memory
+	 */
 }
 
-static const struct block_device_operations z2_fops =
-{
-	.owner		= THIS_MODULE,
-	.open		= z2_open,
-	.release	= z2_release,
+static const struct block_device_operations z2_fops = {
+	.owner = THIS_MODULE,
+	.open = z2_open,
+	.release = z2_release,
 };
 
 static struct kobject *z2_find(dev_t dev, int *part, void *data)
@@ -340,89 +323,83 @@ static struct request_queue *z2_queue;
 static struct blk_mq_tag_set tag_set;
 
 static const struct blk_mq_ops z2_mq_ops = {
-	.queue_rq	= z2_queue_rq,
+	.queue_rq = z2_queue_rq,
 };
 
-static int __init 
-z2_init(void)
+static int __init z2_init(void)
 {
-    int ret;
+	int ret;
 
-    if (!MACH_IS_AMIGA)
-	return -ENODEV;
+	if (!MACH_IS_AMIGA)
+		return -ENODEV;
 
-    ret = -EBUSY;
-    if (register_blkdev(Z2RAM_MAJOR, DEVICE_NAME))
-	goto err;
+	ret = -EBUSY;
+	if (register_blkdev(Z2RAM_MAJOR, DEVICE_NAME))
+		goto err;
 
-    ret = -ENOMEM;
-    z2ram_gendisk = alloc_disk(1);
-    if (!z2ram_gendisk)
-	goto out_disk;
+	ret = -ENOMEM;
+	z2ram_gendisk = alloc_disk(1);
+	if (!z2ram_gendisk)
+		goto out_disk;
 
-    z2_queue = blk_mq_init_sq_queue(&tag_set, &z2_mq_ops, 16,
+	z2_queue = blk_mq_init_sq_queue(&tag_set, &z2_mq_ops, 16,
 					BLK_MQ_F_SHOULD_MERGE);
-    if (IS_ERR(z2_queue)) {
-	ret = PTR_ERR(z2_queue);
-	z2_queue = NULL;
-	goto out_queue;
-    }
+	if (IS_ERR(z2_queue)) {
+		ret = PTR_ERR(z2_queue);
+		z2_queue = NULL;
+		goto out_queue;
+	}
 
-    z2ram_gendisk->major = Z2RAM_MAJOR;
-    z2ram_gendisk->first_minor = 0;
-    z2ram_gendisk->fops = &z2_fops;
-    sprintf(z2ram_gendisk->disk_name, "z2ram");
+	z2ram_gendisk->major = Z2RAM_MAJOR;
+	z2ram_gendisk->first_minor = 0;
+	z2ram_gendisk->fops = &z2_fops;
+	sprintf(z2ram_gendisk->disk_name, "z2ram");
 
-    z2ram_gendisk->queue = z2_queue;
-    add_disk(z2ram_gendisk);
-    blk_register_region(MKDEV(Z2RAM_MAJOR, 0), Z2MINOR_COUNT, THIS_MODULE,
-				z2_find, NULL, NULL);
+	z2ram_gendisk->queue = z2_queue;
+	add_disk(z2ram_gendisk);
+	blk_register_region(MKDEV(Z2RAM_MAJOR, 0), Z2MINOR_COUNT, THIS_MODULE,
+			    z2_find, NULL, NULL);
 
-    return 0;
+	return 0;
 
 out_queue:
-    put_disk(z2ram_gendisk);
+	put_disk(z2ram_gendisk);
 out_disk:
-    unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
+	unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
 err:
-    return ret;
+	return ret;
 }
 
 static void __exit z2_exit(void)
 {
-    int i, j;
-    blk_unregister_region(MKDEV(Z2RAM_MAJOR, 0), Z2MINOR_COUNT);
-    unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
-    del_gendisk(z2ram_gendisk);
-    put_disk(z2ram_gendisk);
-    blk_cleanup_queue(z2_queue);
-    blk_mq_free_tag_set(&tag_set);
-
-    if ( current_device != -1 )
-    {
-	i = 0;
-
-	for ( j = 0 ; j < z2_count; j++ )
-	{
-	    set_bit( i++, zorro_unused_z2ram ); 
-	}
+	int i, j;
+	blk_unregister_region(MKDEV(Z2RAM_MAJOR, 0), Z2MINOR_COUNT);
+	unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
+	del_gendisk(z2ram_gendisk);
+	put_disk(z2ram_gendisk);
+	blk_cleanup_queue(z2_queue);
+	blk_mq_free_tag_set(&tag_set);
+
+	if (current_device != -1) {
+		i = 0;
+
+		for (j = 0; j < z2_count; j++) {
+			set_bit(i++, zorro_unused_z2ram);
+		}
 
-	for ( j = 0 ; j < chip_count; j++ )
-	{
-	    if ( z2ram_map[ i ] )
-	    {
-		amiga_chip_free( (void *) z2ram_map[ i++ ] );
-	    }
-	}
+		for (j = 0; j < chip_count; j++) {
+			if (z2ram_map[i]) {
+				amiga_chip_free((void *)z2ram_map[i++]);
+			}
+		}
 
-	if ( z2ram_map != NULL )
-	{
-	    kfree( z2ram_map );
+		if (z2ram_map != NULL) {
+			kfree(z2ram_map);
+		}
 	}
-    }
 
-    return;
-} 
+	return;
+}
 
 module_init(z2_init);
 module_exit(z2_exit);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 18/19] z2ram: use separate gendisk for the different modes
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (16 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 17/19] z2ram: reindent Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  2020-08-26  6:24 ` [PATCH 19/19] block: switch gendisk lookup to a simple xarray Christoph Hellwig
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Use separate gendisks (which share a tag_set) for the different operating
modes instead of redirecting the gendisk lookup using a probe callback.
This avoids potential problems with aliased block_device instances and
will eventually allow for removing the blk_register_region framework.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/z2ram.c | 100 ++++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 42 deletions(-)

diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c
index 566c653399d8d3..2bf059ba95603c 100644
--- a/drivers/block/z2ram.c
+++ b/drivers/block/z2ram.c
@@ -63,7 +63,7 @@ static int current_device = -1;
 
 static DEFINE_SPINLOCK(z2ram_lock);
 
-static struct gendisk *z2ram_gendisk;
+static struct gendisk *z2ram_gendisk[Z2MINOR_COUNT];
 
 static blk_status_t z2_queue_rq(struct blk_mq_hw_ctx *hctx,
 				const struct blk_mq_queue_data *bd)
@@ -281,7 +281,7 @@ static int z2_open(struct block_device *bdev, fmode_t mode)
 
 		current_device = device;
 		z2ram_size <<= Z2RAM_CHUNKSHIFT;
-		set_capacity(z2ram_gendisk, z2ram_size >> 9);
+		set_capacity(z2ram_gendisk[device], z2ram_size >> 9);
 	}
 
 	mutex_unlock(&z2ram_mutex);
@@ -313,71 +313,87 @@ static const struct block_device_operations z2_fops = {
 	.release = z2_release,
 };
 
-static struct kobject *z2_find(dev_t dev, int *part, void *data)
-{
-	*part = 0;
-	return get_disk_and_module(z2ram_gendisk);
-}
-
-static struct request_queue *z2_queue;
 static struct blk_mq_tag_set tag_set;
 
 static const struct blk_mq_ops z2_mq_ops = {
 	.queue_rq = z2_queue_rq,
 };
 
+static int z2ram_register_disk(int minor)
+{
+	struct request_queue *q;
+	struct gendisk *disk;
+
+	disk = alloc_disk(1);
+	if (!disk)
+		return -ENOMEM;
+
+	q = blk_mq_init_queue(&tag_set);
+	if (IS_ERR(q)) {
+		put_disk(disk);
+		return PTR_ERR(q);
+	}
+
+	disk->major = Z2RAM_MAJOR;
+	disk->first_minor = minor;
+	disk->fops = &z2_fops;
+	if (minor)
+		sprintf(disk->disk_name, "z2ram%d", minor);
+	else
+		sprintf(disk->disk_name, "z2ram");
+	disk->queue = q;
+
+	z2ram_gendisk[minor] = disk;
+	add_disk(disk);
+	return 0;
+}
+
 static int __init z2_init(void)
 {
-	int ret;
+	int ret, i;
 
 	if (!MACH_IS_AMIGA)
 		return -ENODEV;
 
-	ret = -EBUSY;
 	if (register_blkdev(Z2RAM_MAJOR, DEVICE_NAME))
-		goto err;
-
-	ret = -ENOMEM;
-	z2ram_gendisk = alloc_disk(1);
-	if (!z2ram_gendisk)
-		goto out_disk;
-
-	z2_queue = blk_mq_init_sq_queue(&tag_set, &z2_mq_ops, 16,
-					BLK_MQ_F_SHOULD_MERGE);
-	if (IS_ERR(z2_queue)) {
-		ret = PTR_ERR(z2_queue);
-		z2_queue = NULL;
-		goto out_queue;
+		return -EBUSY;
+
+	tag_set.ops = &z2_mq_ops;
+	tag_set.nr_hw_queues = 1;
+	tag_set.nr_maps = 1;
+	tag_set.queue_depth = 16;
+	tag_set.numa_node = NUMA_NO_NODE;
+	tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
+	ret = blk_mq_alloc_tag_set(&tag_set);
+	if (ret)
+		goto out_unregister_blkdev;
+
+	for (i = 0; i < Z2MINOR_COUNT; i++) {
+		ret = z2ram_register_disk(i);
+		if (ret && i == 0)
+			goto out_free_tagset;
 	}
 
-	z2ram_gendisk->major = Z2RAM_MAJOR;
-	z2ram_gendisk->first_minor = 0;
-	z2ram_gendisk->fops = &z2_fops;
-	sprintf(z2ram_gendisk->disk_name, "z2ram");
-
-	z2ram_gendisk->queue = z2_queue;
-	add_disk(z2ram_gendisk);
-	blk_register_region(MKDEV(Z2RAM_MAJOR, 0), Z2MINOR_COUNT, THIS_MODULE,
-			    z2_find, NULL, NULL);
-
 	return 0;
 
-out_queue:
-	put_disk(z2ram_gendisk);
-out_disk:
+out_free_tagset:
+	blk_mq_free_tag_set(&tag_set);
+out_unregister_blkdev:
 	unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
-err:
 	return ret;
 }
 
 static void __exit z2_exit(void)
 {
 	int i, j;
-	blk_unregister_region(MKDEV(Z2RAM_MAJOR, 0), Z2MINOR_COUNT);
+
 	unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
-	del_gendisk(z2ram_gendisk);
-	put_disk(z2ram_gendisk);
-	blk_cleanup_queue(z2_queue);
+
+	for (i = 0; i < Z2MINOR_COUNT; i++) {
+		del_gendisk(z2ram_gendisk[i]);
+		blk_cleanup_queue(z2ram_gendisk[i]->queue);
+		put_disk(z2ram_gendisk[i]);
+	}
 	blk_mq_free_tag_set(&tag_set);
 
 	if (current_device != -1) {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 19/19] block: switch gendisk lookup to a simple xarray
  2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
                   ` (17 preceding siblings ...)
  2020-08-26  6:24 ` [PATCH 18/19] z2ram: use separate gendisk for the different modes Christoph Hellwig
@ 2020-08-26  6:24 ` Christoph Hellwig
  18 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-26  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Now that bdev_map is only used for finding gendisks, we can use
a simple xarray instead of the regions tracking structure for it.

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

diff --git a/block/genhd.c b/block/genhd.c
index 4cbeff3ec1ef5a..80afacaf15f740 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -27,15 +27,7 @@
 
 static struct kobject *block_depr;
 
-struct bdev_map {
-	struct bdev_map *next;
-	dev_t dev;
-	unsigned long range;
-	struct module *owner;
-	struct kobject *(*probe)(dev_t, int *, void *);
-	int (*lock)(dev_t, void *);
-	void *data;
-} *bdev_map[255];
+static DEFINE_XARRAY(bdev_map);
 static DEFINE_MUTEX(bdev_map_lock);
 
 /* for extended dynamic devt allocation, currently only one major is used */
@@ -649,85 +641,26 @@ static char *bdevt_str(dev_t devt, char *buf)
 	return buf;
 }
 
-/*
- * Register device numbers dev..(dev+range-1)
- * range must be nonzero
- * The hash chain is sorted on range, so that subranges can override.
- */
-void blk_register_region(dev_t devt, unsigned long range, struct module *module,
-			 struct kobject *(*probe)(dev_t, int *, void *),
-			 int (*lock)(dev_t, void *), void *data)
-{
-	unsigned n = MAJOR(devt + range - 1) - MAJOR(devt) + 1;
-	unsigned index = MAJOR(devt);
-	unsigned i;
-	struct bdev_map *p;
-
-	n = min(n, 255u);
-	p = kmalloc_array(n, sizeof(struct bdev_map), GFP_KERNEL);
-	if (p == NULL)
-		return;
-
-	for (i = 0; i < n; i++, p++) {
-		p->owner = module;
-		p->probe = probe;
-		p->lock = lock;
-		p->dev = devt;
-		p->range = range;
-		p->data = data;
-	}
-
-	mutex_lock(&bdev_map_lock);
-	for (i = 0, p -= n; i < n; i++, p++, index++) {
-		struct bdev_map **s = &bdev_map[index % 255];
-		while (*s && (*s)->range < range)
-			s = &(*s)->next;
-		p->next = *s;
-		*s = p;
-	}
-	mutex_unlock(&bdev_map_lock);
-}
-EXPORT_SYMBOL(blk_register_region);
-
-void blk_unregister_region(dev_t devt, unsigned long range)
+static void blk_register_region(struct gendisk *disk)
 {
-	unsigned n = MAJOR(devt + range - 1) - MAJOR(devt) + 1;
-	unsigned index = MAJOR(devt);
-	unsigned i;
-	struct bdev_map *found = NULL;
+	int i;
 
 	mutex_lock(&bdev_map_lock);
-	for (i = 0; i < min(n, 255u); i++, index++) {
-		struct bdev_map **s;
-		for (s = &bdev_map[index % 255]; *s; s = &(*s)->next) {
-			struct bdev_map *p = *s;
-			if (p->dev == devt && p->range == range) {
-				*s = p->next;
-				if (!found)
-					found = p;
-				break;
-			}
-		}
+	for (i = 0; i < disk->minors; i++) {
+		if (xa_insert(&bdev_map, disk_devt(disk) + i, disk, GFP_KERNEL))
+			WARN_ON_ONCE(1);
 	}
 	mutex_unlock(&bdev_map_lock);
-	kfree(found);
 }
-EXPORT_SYMBOL(blk_unregister_region);
 
-static struct kobject *exact_match(dev_t devt, int *partno, void *data)
+static void blk_unregister_region(struct gendisk *disk)
 {
-	struct gendisk *p = data;
-
-	return &disk_to_dev(p)->kobj;
-}
-
-static int exact_lock(dev_t devt, void *data)
-{
-	struct gendisk *p = data;
+	int i;
 
-	if (!get_disk_and_module(p))
-		return -1;
-	return 0;
+	mutex_lock(&bdev_map_lock);
+	for (i = 0; i < disk->minors; i++)
+		xa_erase(&bdev_map, disk_devt(disk) + i);
+	mutex_lock(&bdev_map_lock);
 }
 
 static void register_disk(struct device *parent, struct gendisk *disk,
@@ -878,8 +811,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
 		ret = bdi_register(bdi, "%u:%u", MAJOR(devt), MINOR(devt));
 		WARN_ON(ret);
 		bdi_set_owner(bdi, dev);
-		blk_register_region(disk_devt(disk), disk->minors, NULL,
-				    exact_match, exact_lock, disk);
+		blk_register_region(disk);
 	}
 	register_disk(parent, disk, groups);
 	if (register_queue)
@@ -987,7 +919,7 @@ void del_gendisk(struct gendisk *disk)
 		 */
 		bdi_unregister(disk->queue->backing_dev_info);
 		blk_unregister_queue(disk);
-		blk_unregister_region(disk_devt(disk), disk->minors);
+		blk_unregister_region(disk);
 	} else {
 		blk_unregister_queue(disk);
 	}
@@ -1057,54 +989,22 @@ static void request_gendisk_module(dev_t devt)
 		request_module("block-major-%d", MAJOR(devt));
 }
 
-static struct gendisk *lookup_gendisk(dev_t dev, int *partno)
+static bool get_disk_and_module(struct gendisk *disk)
 {
-	struct kobject *kobj;
-	struct bdev_map *p;
-	unsigned long best = ~0UL;
-
-retry:
-	mutex_lock(&bdev_map_lock);
-	for (p = bdev_map[MAJOR(dev) % 255]; p; p = p->next) {
-		struct kobject *(*probe)(dev_t, int *, void *);
-		struct module *owner;
-		void *data;
-
-		if (p->dev > dev || p->dev + p->range - 1 < dev)
-			continue;
-		if (p->range - 1 >= best)
-			break;
-		if (!try_module_get(p->owner))
-			continue;
-		owner = p->owner;
-		data = p->data;
-		probe = p->probe;
-		best = p->range - 1;
-		*partno = dev - p->dev;
-
-		if (!probe) {
-			mutex_unlock(&bdev_map_lock);
-			module_put(owner);
-			request_gendisk_module(dev);
-			goto retry;
-		}
+	struct module *owner;
 
-		if (p->lock && p->lock(dev, data) < 0) {
-			module_put(owner);
-			continue;
-		}
-		mutex_unlock(&bdev_map_lock);
-		kobj = probe(dev, partno, data);
-		/* Currently ->owner protects _only_ ->probe() itself. */
+	if (!disk->fops)
+		return false;
+	owner = disk->fops->owner;
+	if (owner && !try_module_get(owner))
+		return false;
+	if (!kobject_get_unless_zero(&disk_to_dev(disk)->kobj)) {
 		module_put(owner);
-		if (kobj)
-			return dev_to_disk(kobj_to_dev(kobj));
-		goto retry;
+		return false;
 	}
-	mutex_unlock(&bdev_map_lock);
-	return NULL;
-}
+	return true;
 
+}
 
 /**
  * get_gendisk - get partitioning information for a given device
@@ -1123,7 +1023,19 @@ struct gendisk *get_gendisk(dev_t devt, int *partno)
 	might_sleep();
 
 	if (MAJOR(devt) != BLOCK_EXT_MAJOR) {
-		disk = lookup_gendisk(devt, partno);
+		mutex_lock(&bdev_map_lock);
+		disk = xa_load(&bdev_map, devt);
+		if (!disk) {
+			mutex_unlock(&bdev_map_lock);
+			request_gendisk_module(devt);
+			mutex_lock(&bdev_map_lock);
+			disk = xa_load(&bdev_map, devt);
+		}
+		if (disk && !get_disk_and_module(disk))
+			disk = NULL;
+		if (disk)
+			*partno = devt - disk_devt(disk);
+		mutex_unlock(&bdev_map_lock);
 	} else {
 		struct hd_struct *part;
 
@@ -1327,21 +1239,6 @@ static const struct seq_operations partitions_op = {
 };
 #endif
 
-static void bdev_map_init(void)
-{
-	struct bdev_map *base;
-	int i;
-
-	base = kzalloc(sizeof(*base), GFP_KERNEL);
-	if (!base)
-		panic("cannot allocate bdev_map");
-
-	base->dev = 1;
-	base->range = ~0 ;
-	for (i = 0; i < 255; i++)
-		bdev_map[i] = base;
-}
-
 static int __init genhd_device_init(void)
 {
 	int error;
@@ -1350,7 +1247,6 @@ static int __init genhd_device_init(void)
 	error = class_register(&block_class);
 	if (unlikely(error))
 		return error;
-	bdev_map_init();
 	blk_dev_init();
 
 	register_blkdev(BLOCK_EXT_MAJOR, "blkext");
@@ -1896,35 +1792,6 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
 }
 EXPORT_SYMBOL(__alloc_disk_node);
 
-/**
- * get_disk_and_module - increments the gendisk and gendisk fops module refcount
- * @disk: the struct gendisk to increment the refcount for
- *
- * This increments the refcount for the struct gendisk, and the gendisk's
- * fops module owner.
- *
- * Context: Any context.
- */
-struct kobject *get_disk_and_module(struct gendisk *disk)
-{
-	struct module *owner;
-	struct kobject *kobj;
-
-	if (!disk->fops)
-		return NULL;
-	owner = disk->fops->owner;
-	if (owner && !try_module_get(owner))
-		return NULL;
-	kobj = kobject_get_unless_zero(&disk_to_dev(disk)->kobj);
-	if (kobj == NULL) {
-		module_put(owner);
-		return NULL;
-	}
-	return kobj;
-
-}
-EXPORT_SYMBOL(get_disk_and_module);
-
 /**
  * put_disk - decrements the gendisk refcount
  * @disk: the struct gendisk to decrement the refcount for
@@ -1961,7 +1828,6 @@ void put_disk_and_module(struct gendisk *disk)
 		module_put(owner);
 	}
 }
-EXPORT_SYMBOL(put_disk_and_module);
 
 static void set_disk_ro_uevent(struct gendisk *gd, int ro)
 {
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index c0bde190a3dd0c..c6bdc8ce69bd87 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -341,15 +341,8 @@ int blk_add_partitions(struct gendisk *disk, struct block_device *bdev);
 int blk_drop_partitions(struct block_device *bdev);
 
 extern struct gendisk *__alloc_disk_node(int minors, int node_id);
-extern struct kobject *get_disk_and_module(struct gendisk *disk);
 extern void put_disk(struct gendisk *disk);
 extern void put_disk_and_module(struct gendisk *disk);
-extern void blk_register_region(dev_t devt, unsigned long range,
-			struct module *module,
-			struct kobject *(*probe)(dev_t, int *, void *),
-			int (*lock)(dev_t, void *),
-			void *data);
-extern void blk_unregister_region(dev_t devt, unsigned long range);
 
 #define alloc_disk_node(minors, node_id)				\
 ({									\
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/19] char_dev: replace cdev_map with an xarray
  2020-08-26  6:24 ` [PATCH 01/19] char_dev: replace cdev_map with an xarray Christoph Hellwig
@ 2020-08-26  8:19   ` Greg Kroah-Hartman
  2020-08-27  8:53     ` Christoph Hellwig
  2020-08-27  7:25   ` Hannes Reinecke
  1 sibling, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-26  8:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Rafael J. Wysocki, Denis Efremov, David S. Miller,
	Song Liu, Al Viro, linux-block, linux-kernel, linux-ide,
	linux-raid, linux-scsi, linux-m68k

On Wed, Aug 26, 2020 at 08:24:28AM +0200, Christoph Hellwig wrote:
> None of the complicated overlapping regions bits of the kobj_map are
> required for the character device lookup, so just a trivial xarray
> instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Really?  This is ok to use and just as fast?  If so, wonderful, it would
be great to clean up kobj_map users.

But I don't think you got all of the needed bits here:

> ---
>  fs/char_dev.c | 94 +++++++++++++++++++++++++--------------------------
>  fs/dcache.c   |  1 -
>  fs/internal.h |  5 ---
>  3 files changed, 46 insertions(+), 54 deletions(-)
> 
> diff --git a/fs/char_dev.c b/fs/char_dev.c
> index ba0ded7842a779..6c4d6c4938f14b 100644
> --- a/fs/char_dev.c
> +++ b/fs/char_dev.c
> @@ -17,7 +17,6 @@
>  #include <linux/seq_file.h>
>  
>  #include <linux/kobject.h>
> -#include <linux/kobj_map.h>
>  #include <linux/cdev.h>
>  #include <linux/mutex.h>
>  #include <linux/backing-dev.h>
> @@ -25,8 +24,7 @@
>  
>  #include "internal.h"
>  
> -static struct kobj_map *cdev_map;
> -
> +static DEFINE_XARRAY(cdev_map);
>  static DEFINE_MUTEX(chrdevs_lock);
>  
>  #define CHRDEV_MAJOR_HASH_SIZE 255
> @@ -367,6 +365,29 @@ void cdev_put(struct cdev *p)
>  	}
>  }
>  
> +static struct cdev *cdev_lookup(dev_t dev)
> +{
> +	struct cdev *cdev;
> +
> +retry:
> +	mutex_lock(&chrdevs_lock);
> +	cdev = xa_load(&cdev_map, dev);
> +	if (!cdev) {
> +		mutex_unlock(&chrdevs_lock);
> +
> +		if (request_module("char-major-%d-%d",
> +				   MAJOR(dev), MINOR(dev)) > 0)
> +			/* Make old-style 2.4 aliases work */
> +			request_module("char-major-%d", MAJOR(dev));
> +		goto retry;
> +	}
> +
> +	if (!cdev_get(cdev))
> +		cdev = NULL;
> +	mutex_unlock(&chrdevs_lock);
> +	return cdev;
> +}
> +
>  /*
>   * Called every time a character special file is opened
>   */
> @@ -380,13 +401,10 @@ static int chrdev_open(struct inode *inode, struct file *filp)
>  	spin_lock(&cdev_lock);
>  	p = inode->i_cdev;
>  	if (!p) {
> -		struct kobject *kobj;
> -		int idx;
>  		spin_unlock(&cdev_lock);
> -		kobj = kobj_lookup(cdev_map, inode->i_rdev, &idx);
> -		if (!kobj)
> +		new = cdev_lookup(inode->i_rdev);
> +		if (!new)
>  			return -ENXIO;
> -		new = container_of(kobj, struct cdev, kobj);
>  		spin_lock(&cdev_lock);
>  		/* Check i_cdev again in case somebody beat us to it while
>  		   we dropped the lock. */
> @@ -454,18 +472,6 @@ const struct file_operations def_chr_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> -static struct kobject *exact_match(dev_t dev, int *part, void *data)
> -{
> -	struct cdev *p = data;
> -	return &p->kobj;
> -}
> -
> -static int exact_lock(dev_t dev, void *data)
> -{
> -	struct cdev *p = data;
> -	return cdev_get(p) ? 0 : -1;
> -}
> -
>  /**
>   * cdev_add() - add a char device to the system
>   * @p: the cdev structure for the device
> @@ -478,7 +484,7 @@ static int exact_lock(dev_t dev, void *data)
>   */
>  int cdev_add(struct cdev *p, dev_t dev, unsigned count)
>  {
> -	int error;
> +	int error, i;
>  
>  	p->dev = dev;
>  	p->count = count;
> @@ -486,14 +492,22 @@ int cdev_add(struct cdev *p, dev_t dev, unsigned count)
>  	if (WARN_ON(dev == WHITEOUT_DEV))
>  		return -EBUSY;
>  
> -	error = kobj_map(cdev_map, dev, count, NULL,
> -			 exact_match, exact_lock, p);
> -	if (error)
> -		return error;
> +	mutex_lock(&chrdevs_lock);
> +	for (i = 0; i < count; i++) {
> +		error = xa_insert(&cdev_map, dev + i, p, GFP_KERNEL);
> +		if (error)
> +			goto out_unwind;
> +	}
> +	mutex_unlock(&chrdevs_lock);
>  
>  	kobject_get(p->kobj.parent);

Can't you drop this kobject_get() too?

And also the "struct kobj" in struct cdev can be gone as well, as the
kobj_map was the only "real" user of this structure.  I know some
drivers liked to touch that field as well, but it never actually did
anything for them, so it was pointless, but it will take some 'make
allmodconfig' builds to flush them out.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 02/19] block: merge drivers/base/map.c into block/genhd.c
  2020-08-26  6:24 ` [PATCH 02/19] block: merge drivers/base/map.c into block/genhd.c Christoph Hellwig
@ 2020-08-26  8:19   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 40+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-26  8:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Rafael J. Wysocki, Denis Efremov, David S. Miller,
	Song Liu, Al Viro, linux-block, linux-kernel, linux-ide,
	linux-raid, linux-scsi, linux-m68k

On Wed, Aug 26, 2020 at 08:24:29AM +0200, Christoph Hellwig wrote:
> Now that there is just a single user of the kobj_map functionality left,
> merge it into the user to prepare for additional simplications.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

YES!!!

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-26  6:24 ` [PATCH 17/19] z2ram: reindent Christoph Hellwig
@ 2020-08-26  9:12   ` Joe Perches
  2020-08-26  9:49     ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 40+ messages in thread
From: Joe Perches @ 2020-08-26  9:12 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On Wed, 2020-08-26 at 08:24 +0200, Christoph Hellwig wrote:
> reindent the driver using Lident as the code style was far away from
> normal Linux code.

Why?  Does anyone use this anymore?

 ** z2ram - Amiga pseudo-driver to access 16bit-RAM in ZorroII space
 **         as a block device, to be used as a RAM disk or swap space
 ** Copyright (C) 1994 by Ingo Wilken (Ingo.Wilken@informatik.uni-oldenburg.de)




^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-26  9:12   ` Joe Perches
@ 2020-08-26  9:49     ` John Paul Adrian Glaubitz
  2020-08-26 14:51       ` Joe Perches
  0 siblings, 1 reply; 40+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-08-26  9:49 UTC (permalink / raw)
  To: Joe Perches
  Cc: Christoph Hellwig, Jens Axboe, Greg Kroah-Hartman,
	Rafael J. Wysocki, Denis Efremov, David S. Miller, Song Liu,
	Al Viro, linux-block, linux-kernel, linux-ide, linux-raid,
	linux-scsi, linux-m68k


> On Aug 26, 2020, at 11:21 AM, Joe Perches <joe@perches.com> wrote:
> 
> On Wed, 2020-08-26 at 08:24 +0200, Christoph Hellwig wrote:
>> reindent the driver using Lident as the code style was far away from
>> normal Linux code.
> 
> Why?  Does anyone use this anymore?

Yes, the Amiga and Linux/m68k is very well and alive. There is new hardware being developed and new drivers being developed and so on.

Please let’s don’t have another discussion about that. The code is maintained which is what counts and not whether any corporation is still making money with it.

Adrian

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-26  9:49     ` John Paul Adrian Glaubitz
@ 2020-08-26 14:51       ` Joe Perches
  2020-08-27 23:21         ` Finn Thain
  0 siblings, 1 reply; 40+ messages in thread
From: Joe Perches @ 2020-08-26 14:51 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Christoph Hellwig, Jens Axboe, Greg Kroah-Hartman,
	Rafael J. Wysocki, Denis Efremov, David S. Miller, Song Liu,
	Al Viro, linux-block, linux-kernel, linux-ide, linux-raid,
	linux-scsi, linux-m68k

On Wed, 2020-08-26 at 11:49 +0200, John Paul Adrian Glaubitz wrote:
> > On Aug 26, 2020, at 11:21 AM, Joe Perches <joe@perches.com> wrote:
> > 
> > On Wed, 2020-08-26 at 08:24 +0200, Christoph Hellwig wrote:
> > > reindent the driver using Lident as the code style was far away from
> > > normal Linux code.
> > 
> > Why?  Does anyone use this anymore?
> 
> Yes, the Amiga and Linux/m68k is very well and alive.

That's fine.  My question was why ancient code should be modified
if it's not in use.

>  There is new hardware being developed and new drivers being developed and so on.

OK, fine.

btw:

If style only changes are to be done on this code then
I believe these changes on top should also be made done
for style:
---
From 0eb1b25575abe52415ecb0139e14ae57ba4f57cb Mon Sep 17 00:00:00 2001
Message-Id: <0eb1b25575abe52415ecb0139e14ae57ba4f57cb.1598453361.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Wed, 26 Aug 2020 02:20:14 -0700
Subject: [PATCH] z2ram: Use more current coding style

Use pr_fmt and pr_<level>, continue, and remove tests against NULL.
Use more typical brace styles, add and remove them as appropriate.
Simplify logic in z2_open, rename err_out label to out, unindent a
large block by reversing the test and using goto.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/block/z2ram.c | 261 +++++++++++++++++++-----------------------
 1 file changed, 118 insertions(+), 143 deletions(-)

diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c
index 566c653399d8..ea490fe4417e 100644
--- a/drivers/block/z2ram.c
+++ b/drivers/block/z2ram.c
@@ -1,7 +1,7 @@
 /*
 ** z2ram - Amiga pseudo-driver to access 16bit-RAM in ZorroII space
 **         as a block device, to be used as a RAM disk or swap space
-** 
+**
 ** Copyright (C) 1994 by Ingo Wilken (Ingo.Wilken@informatik.uni-oldenburg.de)
 **
 ** ++Geert: support for zorro_unused_z2ram, better range checking
@@ -25,6 +25,8 @@
 ** implied warranty.
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #define DEVICE_NAME "Z2RAM"
 
 #include <linux/major.h>
@@ -75,7 +77,7 @@ static blk_status_t z2_queue_rq(struct blk_mq_hw_ctx *hctx,
 	blk_mq_start_request(req);
 
 	if (start + len > z2ram_size) {
-		pr_err(DEVICE_NAME ": bad access: block=%llu, count=%u\n",
+		pr_err("bad access: block=%llu, count=%u\n",
 		       (unsigned long long)blk_rq_pos(req),
 		       blk_rq_cur_sectors(req));
 		return BLK_STS_IOERR;
@@ -109,34 +111,28 @@ static void get_z2ram(void)
 	int i;
 
 	for (i = 0; i < Z2RAM_SIZE / Z2RAM_CHUNKSIZE; i++) {
-		if (test_bit(i, zorro_unused_z2ram)) {
-			z2_count++;
-			z2ram_map[z2ram_size++] =
-			    (unsigned long)ZTWO_VADDR(Z2RAM_START) +
-			    (i << Z2RAM_CHUNKSHIFT);
-			clear_bit(i, zorro_unused_z2ram);
-		}
+		if (!test_bit(i, zorro_unused_z2ram))
+			continue;
+		z2_count++;
+		z2ram_map[z2ram_size++] =
+			(unsigned long)ZTWO_VADDR(Z2RAM_START) +
+			(i << Z2RAM_CHUNKSHIFT);
+		clear_bit(i, zorro_unused_z2ram);
 	}
-
-	return;
 }
 
 static void get_chipram(void)
 {
-
 	while (amiga_chip_avail() > (Z2RAM_CHUNKSIZE * 4)) {
 		chip_count++;
 		z2ram_map[z2ram_size] =
 		    (u_long) amiga_chip_alloc(Z2RAM_CHUNKSIZE, "z2ram");
 
-		if (z2ram_map[z2ram_size] == 0) {
+		if (z2ram_map[z2ram_size] == 0)
 			break;
-		}
 
 		z2ram_size++;
 	}
-
-	return;
 }
 
 static int z2_open(struct block_device *bdev, fmode_t mode)
@@ -152,144 +148,127 @@ static int z2_open(struct block_device *bdev, fmode_t mode)
 	mutex_lock(&z2ram_mutex);
 	if (current_device != -1 && current_device != device) {
 		rc = -EBUSY;
-		goto err_out;
+		goto out;
 	}
 
-	if (current_device == -1) {
-		z2_count = 0;
-		chip_count = 0;
-		list_count = 0;
-		z2ram_size = 0;
-
-		/* Use a specific list entry. */
-		if (device >= Z2MINOR_MEMLIST1 && device <= Z2MINOR_MEMLIST4) {
-			int index = device - Z2MINOR_MEMLIST1 + 1;
-			unsigned long size, paddr, vaddr;
-
-			if (index >= m68k_realnum_memory) {
-				printk(KERN_ERR DEVICE_NAME
-				       ": no such entry in z2ram_map\n");
-				goto err_out;
-			}
+	if (current_device != -1) {
+		rc = 0;
+		goto out;
+	}
 
-			paddr = m68k_memory[index].addr;
-			size = m68k_memory[index].size & ~(Z2RAM_CHUNKSIZE - 1);
+	z2_count = 0;
+	chip_count = 0;
+	list_count = 0;
+	z2ram_size = 0;
 
-#ifdef __powerpc__
-			/*
-			 * FIXME: ioremap doesn't build correct memory tables.
-			 */
-			vfree(vmalloc(size));
-			vaddr = (unsigned long)ioremap_wt(paddr, size);
+	/* Use a specific list entry. */
+	if (device >= Z2MINOR_MEMLIST1 && device <= Z2MINOR_MEMLIST4) {
+		int index = device - Z2MINOR_MEMLIST1 + 1;
+		unsigned long size, paddr, vaddr;
+
+		if (index >= m68k_realnum_memory) {
+			pr_err("no such entry in z2ram_map\n");
+			goto out;
+		}
 
+		paddr = m68k_memory[index].addr;
+		size = m68k_memory[index].size & ~(Z2RAM_CHUNKSIZE - 1);
+
+#ifdef __powerpc__
+		/*
+		 * FIXME: ioremap doesn't build correct memory tables.
+		 */
+		vfree(vmalloc(size));
+		vaddr = (unsigned long)ioremap_wt(paddr, size);
 #else
-			vaddr =
-			    (unsigned long)z_remap_nocache_nonser(paddr, size);
+		vaddr = (unsigned long)z_remap_nocache_nonser(paddr, size);
 #endif
-			z2ram_map =
-			    kmalloc_array(size / Z2RAM_CHUNKSIZE,
+		z2ram_map = kmalloc_array(size / Z2RAM_CHUNKSIZE,
 					  sizeof(z2ram_map[0]), GFP_KERNEL);
-			if (z2ram_map == NULL) {
-				printk(KERN_ERR DEVICE_NAME
-				       ": cannot get mem for z2ram_map\n");
-				goto err_out;
+		if (!z2ram_map) {
+			pr_err("cannot get mem for z2ram_map\n");
+			goto out;
+		}
+
+		while (size) {
+			z2ram_map[z2ram_size++] = vaddr;
+			size -= Z2RAM_CHUNKSIZE;
+			vaddr += Z2RAM_CHUNKSIZE;
+			list_count++;
+		}
+
+		if (z2ram_size != 0)
+			pr_info("using %iK List Entry %d Memory\n",
+				list_count * Z2RAM_CHUNK1024, index);
+	} else {
+		switch (device) {
+		case Z2MINOR_COMBINED:
+			z2ram_map = kmalloc(max_z2_map + max_chip_map,
+					    GFP_KERNEL);
+			if (!z2ram_map) {
+				pr_err("cannot get mem for z2ram_map\n");
+				goto out;
 			}
 
-			while (size) {
-				z2ram_map[z2ram_size++] = vaddr;
-				size -= Z2RAM_CHUNKSIZE;
-				vaddr += Z2RAM_CHUNKSIZE;
-				list_count++;
+			get_z2ram();
+			get_chipram();
+
+			if (z2ram_size != 0)
+				pr_info("using %iK Zorro II RAM and %iK Chip RAM (Total %dK)\n",
+					z2_count * Z2RAM_CHUNK1024,
+					chip_count * Z2RAM_CHUNK1024,
+					(z2_count + chip_count) * Z2RAM_CHUNK1024);
+
+			break;
+
+		case Z2MINOR_Z2ONLY:
+			z2ram_map = kmalloc(max_z2_map, GFP_KERNEL);
+			if (!z2ram_map) {
+				pr_err("cannot get mem for z2ram_map\n");
+				goto out;
 			}
 
+			get_z2ram();
+
 			if (z2ram_size != 0)
-				printk(KERN_INFO DEVICE_NAME
-				       ": using %iK List Entry %d Memory\n",
-				       list_count * Z2RAM_CHUNK1024, index);
-		} else
-			switch (device) {
-			case Z2MINOR_COMBINED:
-
-				z2ram_map =
-				    kmalloc(max_z2_map + max_chip_map,
-					    GFP_KERNEL);
-				if (z2ram_map == NULL) {
-					printk(KERN_ERR DEVICE_NAME
-					       ": cannot get mem for z2ram_map\n");
-					goto err_out;
-				}
-
-				get_z2ram();
-				get_chipram();
-
-				if (z2ram_size != 0)
-					printk(KERN_INFO DEVICE_NAME
-					       ": using %iK Zorro II RAM and %iK Chip RAM (Total %dK)\n",
-					       z2_count * Z2RAM_CHUNK1024,
-					       chip_count * Z2RAM_CHUNK1024,
-					       (z2_count +
-						chip_count) * Z2RAM_CHUNK1024);
-
-				break;
-
-			case Z2MINOR_Z2ONLY:
-				z2ram_map = kmalloc(max_z2_map, GFP_KERNEL);
-				if (z2ram_map == NULL) {
-					printk(KERN_ERR DEVICE_NAME
-					       ": cannot get mem for z2ram_map\n");
-					goto err_out;
-				}
-
-				get_z2ram();
-
-				if (z2ram_size != 0)
-					printk(KERN_INFO DEVICE_NAME
-					       ": using %iK of Zorro II RAM\n",
-					       z2_count * Z2RAM_CHUNK1024);
-
-				break;
-
-			case Z2MINOR_CHIPONLY:
-				z2ram_map = kmalloc(max_chip_map, GFP_KERNEL);
-				if (z2ram_map == NULL) {
-					printk(KERN_ERR DEVICE_NAME
-					       ": cannot get mem for z2ram_map\n");
-					goto err_out;
-				}
-
-				get_chipram();
-
-				if (z2ram_size != 0)
-					printk(KERN_INFO DEVICE_NAME
-					       ": using %iK Chip RAM\n",
-					       chip_count * Z2RAM_CHUNK1024);
-
-				break;
-
-			default:
-				rc = -ENODEV;
-				goto err_out;
-
-				break;
+				pr_info("using %iK of Zorro II RAM\n",
+					z2_count * Z2RAM_CHUNK1024);
+
+			break;
+
+		case Z2MINOR_CHIPONLY:
+			z2ram_map = kmalloc(max_chip_map, GFP_KERNEL);
+			if (!z2ram_map) {
+				pr_err("cannot get mem for z2ram_map\n");
+				goto out;
 			}
 
-		if (z2ram_size == 0) {
-			printk(KERN_NOTICE DEVICE_NAME
-			       ": no unused ZII/Chip RAM found\n");
-			goto err_out_kfree;
+			get_chipram();
+
+			if (z2ram_size != 0)
+				pr_info("using %iK Chip RAM\n",
+					chip_count * Z2RAM_CHUNK1024);
+
+			break;
+
+		default:
+			rc = -ENODEV;
+			goto out;
 		}
+	}
 
-		current_device = device;
-		z2ram_size <<= Z2RAM_CHUNKSHIFT;
-		set_capacity(z2ram_gendisk, z2ram_size >> 9);
+	if (z2ram_size == 0) {
+		pr_notice("no unused ZII/Chip RAM found\n");
+		kfree(z2ram_map);
+		goto out;
 	}
 
-	mutex_unlock(&z2ram_mutex);
-	return 0;
+	current_device = device;
+	z2ram_size <<= Z2RAM_CHUNKSHIFT;
+	set_capacity(z2ram_gendisk, z2ram_size >> 9);
+	rc = 0;
 
-err_out_kfree:
-	kfree(z2ram_map);
-err_out:
+out:
 	mutex_unlock(&z2ram_mutex);
 	return rc;
 }
@@ -372,7 +351,6 @@ static int __init z2_init(void)
 
 static void __exit z2_exit(void)
 {
-	int i, j;
 	blk_unregister_region(MKDEV(Z2RAM_MAJOR, 0), Z2MINOR_COUNT);
 	unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
 	del_gendisk(z2ram_gendisk);
@@ -381,24 +359,21 @@ static void __exit z2_exit(void)
 	blk_mq_free_tag_set(&tag_set);
 
 	if (current_device != -1) {
+		int i, j;
+
 		i = 0;
 
-		for (j = 0; j < z2_count; j++) {
+		for (j = 0; j < z2_count; j++)
 			set_bit(i++, zorro_unused_z2ram);
-		}
 
 		for (j = 0; j < chip_count; j++) {
-			if (z2ram_map[i]) {
+			if (z2ram_map[i])
 				amiga_chip_free((void *)z2ram_map[i++]);
-			}
 		}
 
-		if (z2ram_map != NULL) {
+		if (z2ram_map)
 			kfree(z2ram_map);
-		}
 	}
-
-	return;
 }
 
 module_init(z2_init);
-- 
2.26.0



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/19] char_dev: replace cdev_map with an xarray
  2020-08-26  6:24 ` [PATCH 01/19] char_dev: replace cdev_map with an xarray Christoph Hellwig
  2020-08-26  8:19   ` Greg Kroah-Hartman
@ 2020-08-27  7:25   ` Hannes Reinecke
  2020-08-27  8:55     ` Christoph Hellwig
  1 sibling, 1 reply; 40+ messages in thread
From: Hannes Reinecke @ 2020-08-27  7:25 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On 8/26/20 8:24 AM, Christoph Hellwig wrote:
> None of the complicated overlapping regions bits of the kobj_map are
> required for the character device lookup, so just a trivial xarray
> instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/char_dev.c | 94 +++++++++++++++++++++++++--------------------------
>  fs/dcache.c   |  1 -
>  fs/internal.h |  5 ---
>  3 files changed, 46 insertions(+), 54 deletions(-)
> 
> diff --git a/fs/char_dev.c b/fs/char_dev.c
> index ba0ded7842a779..6c4d6c4938f14b 100644
> --- a/fs/char_dev.c
> +++ b/fs/char_dev.c
> @@ -17,7 +17,6 @@
>  #include <linux/seq_file.h>
>  
>  #include <linux/kobject.h>
> -#include <linux/kobj_map.h>
>  #include <linux/cdev.h>
>  #include <linux/mutex.h>
>  #include <linux/backing-dev.h>
> @@ -25,8 +24,7 @@
>  
>  #include "internal.h"
>  
> -static struct kobj_map *cdev_map;
> -
> +static DEFINE_XARRAY(cdev_map);
>  static DEFINE_MUTEX(chrdevs_lock);
>  
>  #define CHRDEV_MAJOR_HASH_SIZE 255
> @@ -367,6 +365,29 @@ void cdev_put(struct cdev *p)
>  	}
>  }
>  
> +static struct cdev *cdev_lookup(dev_t dev)
> +{
> +	struct cdev *cdev;
> +
> +retry:
> +	mutex_lock(&chrdevs_lock);
> +	cdev = xa_load(&cdev_map, dev);
> +	if (!cdev) {
> +		mutex_unlock(&chrdevs_lock);
> +
> +		if (request_module("char-major-%d-%d",
> +				   MAJOR(dev), MINOR(dev)) > 0)
> +			/* Make old-style 2.4 aliases work */
> +			request_module("char-major-%d", MAJOR(dev));
> +		goto retry;
> +	}
> +
> +	if (!cdev_get(cdev))
> +		cdev = NULL;
> +	mutex_unlock(&chrdevs_lock);
> +	return cdev;
> +}
> +
>  /*
>   * Called every time a character special file is opened
>   */
> @@ -380,13 +401,10 @@ static int chrdev_open(struct inode *inode, struct file *filp)
>  	spin_lock(&cdev_lock);
>  	p = inode->i_cdev;
>  	if (!p) {
> -		struct kobject *kobj;
> -		int idx;
>  		spin_unlock(&cdev_lock);
> -		kobj = kobj_lookup(cdev_map, inode->i_rdev, &idx);
> -		if (!kobj)
> +		new = cdev_lookup(inode->i_rdev);
> +		if (!new)
>  			return -ENXIO;
> -		new = container_of(kobj, struct cdev, kobj);
>  		spin_lock(&cdev_lock);
>  		/* Check i_cdev again in case somebody beat us to it while
>  		   we dropped the lock. */
> @@ -454,18 +472,6 @@ const struct file_operations def_chr_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> -static struct kobject *exact_match(dev_t dev, int *part, void *data)
> -{
> -	struct cdev *p = data;
> -	return &p->kobj;
> -}
> -
> -static int exact_lock(dev_t dev, void *data)
> -{
> -	struct cdev *p = data;
> -	return cdev_get(p) ? 0 : -1;
> -}
> -
>  /**
>   * cdev_add() - add a char device to the system
>   * @p: the cdev structure for the device
> @@ -478,7 +484,7 @@ static int exact_lock(dev_t dev, void *data)
>   */
>  int cdev_add(struct cdev *p, dev_t dev, unsigned count)
>  {
> -	int error;
> +	int error, i;
>  
>  	p->dev = dev;
>  	p->count = count;
> @@ -486,14 +492,22 @@ int cdev_add(struct cdev *p, dev_t dev, unsigned count)
>  	if (WARN_ON(dev == WHITEOUT_DEV))
>  		return -EBUSY;
>  
> -	error = kobj_map(cdev_map, dev, count, NULL,
> -			 exact_match, exact_lock, p);
> -	if (error)
> -		return error;
> +	mutex_lock(&chrdevs_lock);
> +	for (i = 0; i < count; i++) {
> +		error = xa_insert(&cdev_map, dev + i, p, GFP_KERNEL);
> +		if (error)
> +			goto out_unwind;
> +	}
> +	mutex_unlock(&chrdevs_lock);
>  
>  	kobject_get(p->kobj.parent);
> -
>  	return 0;
> +
> +out_unwind:
> +	while (--i >= 0)
> +		xa_erase(&cdev_map, dev + i);
> +	mutex_unlock(&chrdevs_lock);
> +	return error;
>  }
>  
>  /**
Do you really need the mutex?
Wouldn't xa_store_range() be better and avoid the mutex?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 07/19] ide: remove ide_{,un}register_region
  2020-08-26  6:24 ` [PATCH 07/19] ide: remove ide_{,un}register_region Christoph Hellwig
@ 2020-08-27  7:25   ` Hannes Reinecke
  0 siblings, 0 replies; 40+ messages in thread
From: Hannes Reinecke @ 2020-08-27  7:25 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On 8/26/20 8:24 AM, Christoph Hellwig wrote:
> There is no need to ever register the fake gendisk used for ide-tape.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ide/ide-probe.c | 32 --------------------------------
>  drivers/ide/ide-tape.c  |  2 --
>  include/linux/ide.h     |  3 ---
>  3 files changed, 37 deletions(-)
> 
IDE-tape. Shudder.
Anything to get rid of them.

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

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/19] char_dev: replace cdev_map with an xarray
  2020-08-26  8:19   ` Greg Kroah-Hartman
@ 2020-08-27  8:53     ` Christoph Hellwig
  2020-08-27  9:18       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-27  8:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Christoph Hellwig, Jens Axboe, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On Wed, Aug 26, 2020 at 10:19:05AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Aug 26, 2020 at 08:24:28AM +0200, Christoph Hellwig wrote:
> > None of the complicated overlapping regions bits of the kobj_map are
> > required for the character device lookup, so just a trivial xarray
> > instead.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Really?  This is ok to use and just as fast?  If so, wonderful, it would
> be great to clean up kobj_map users.

Xarrays provide pretty efficient as long as we have a unsigned long
or smaller index (check, dev_t is small) and the indices are reasonable
clustered (check, minors for the same major).  Memory usage will go down
vs the probes, and lookup speed up.

> > +	mutex_lock(&chrdevs_lock);
> > +	for (i = 0; i < count; i++) {
> > +		error = xa_insert(&cdev_map, dev + i, p, GFP_KERNEL);
> > +		if (error)
> > +			goto out_unwind;
> > +	}
> > +	mutex_unlock(&chrdevs_lock);
> >  
> >  	kobject_get(p->kobj.parent);
> 
> Can't you drop this kobject_get() too?

I'll have to drop it or add back the put on the delete side.  And
I think the latter is safer for now, because..

> 
> And also the "struct kobj" in struct cdev can be gone as well, as the
> kobj_map was the only "real" user of this structure.  I know some
> drivers liked to touch that field as well, but it never actually did
> anything for them, so it was pointless, but it will take some 'make
> allmodconfig' builds to flush them out.

I looked at it, but it does get registered and shows up in sysfs.
I don't really dare to touch this for now, as it can have huge
implications.  Better done in a separate series (if we can actually do
it at all).

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/19] char_dev: replace cdev_map with an xarray
  2020-08-27  7:25   ` Hannes Reinecke
@ 2020-08-27  8:55     ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-27  8:55 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Christoph Hellwig, Jens Axboe, Greg Kroah-Hartman,
	Rafael J. Wysocki, Denis Efremov, David S. Miller, Song Liu,
	Al Viro, linux-block, linux-kernel, linux-ide, linux-raid,
	linux-scsi, linux-m68k

On Thu, Aug 27, 2020 at 09:25:07AM +0200, Hannes Reinecke wrote:
> Do you really need the mutex?
> Wouldn't xa_store_range() be better and avoid the mutex?

We need the mutex as we need to grab the kobject reference under it.

xa_store_range is only available with a separate config option, and
has really strange calling conventions.  So I'd rather not pull it in
here, especially as most cdev_add callers are for a single minor only
anyway.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/19] char_dev: replace cdev_map with an xarray
  2020-08-27  8:53     ` Christoph Hellwig
@ 2020-08-27  9:18       ` Greg Kroah-Hartman
  2020-08-27  9:39         ` Christoph Hellwig
  0 siblings, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-27  9:18 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Rafael J. Wysocki, Denis Efremov, David S. Miller,
	Song Liu, Al Viro, linux-block, linux-kernel, linux-ide,
	linux-raid, linux-scsi, linux-m68k

On Thu, Aug 27, 2020 at 10:53:53AM +0200, Christoph Hellwig wrote:
> On Wed, Aug 26, 2020 at 10:19:05AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Aug 26, 2020 at 08:24:28AM +0200, Christoph Hellwig wrote:
> > > None of the complicated overlapping regions bits of the kobj_map are
> > > required for the character device lookup, so just a trivial xarray
> > > instead.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Really?  This is ok to use and just as fast?  If so, wonderful, it would
> > be great to clean up kobj_map users.
> 
> Xarrays provide pretty efficient as long as we have a unsigned long
> or smaller index (check, dev_t is small) and the indices are reasonable
> clustered (check, minors for the same major).  Memory usage will go down
> vs the probes, and lookup speed up.

Ok, great!

xarrays weren't around when this code was written (back in the 2.5
days).

> > > +	mutex_lock(&chrdevs_lock);
> > > +	for (i = 0; i < count; i++) {
> > > +		error = xa_insert(&cdev_map, dev + i, p, GFP_KERNEL);
> > > +		if (error)
> > > +			goto out_unwind;
> > > +	}
> > > +	mutex_unlock(&chrdevs_lock);
> > >  
> > >  	kobject_get(p->kobj.parent);
> > 
> > Can't you drop this kobject_get() too?
> 
> I'll have to drop it or add back the put on the delete side.  And
> I think the latter is safer for now, because..
> 
> > 
> > And also the "struct kobj" in struct cdev can be gone as well, as the
> > kobj_map was the only "real" user of this structure.  I know some
> > drivers liked to touch that field as well, but it never actually did
> > anything for them, so it was pointless, but it will take some 'make
> > allmodconfig' builds to flush them out.
> 
> I looked at it, but it does get registered and shows up in sysfs.

It does?  Where does that happen?  I see a bunch of kobject_init()
calls, but nothing that registers it in sysfs that I can see.

Note, this is not the kobject that shows up in /sys/dev/char/ as a
symlink, that comes from the driver core logic and is independent of the
cdev code.

The kobject does handle the structure lifetime rules, but that should be
able to be replaced with a simple kref instead.

> I don't really dare to touch this for now, as it can have huge
> implications.  Better done in a separate series (if we can actually do
> it at all).

Fair enough, I will be willing to tackle that once this gets merged, so
this is fine as-is.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/19] char_dev: replace cdev_map with an xarray
  2020-08-27  9:18       ` Greg Kroah-Hartman
@ 2020-08-27  9:39         ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-27  9:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Christoph Hellwig, Jens Axboe, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On Thu, Aug 27, 2020 at 11:18:59AM +0200, Greg Kroah-Hartman wrote:
> > I looked at it, but it does get registered and shows up in sysfs.
> 
> It does?  Where does that happen?  I see a bunch of kobject_init()
> calls, but nothing that registers it in sysfs that I can see.

Hmm, true.

> 
> Note, this is not the kobject that shows up in /sys/dev/char/ as a
> symlink, that comes from the driver core logic and is independent of the
> cdev code.
> 
> The kobject does handle the structure lifetime rules, but that should be
> able to be replaced with a simple kref instead.

Yeah.  I'll let you handle this stuff, as you obviously know the area
better than I do.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-26 14:51       ` Joe Perches
@ 2020-08-27 23:21         ` Finn Thain
  2020-08-28  0:01           ` Joe Perches
  0 siblings, 1 reply; 40+ messages in thread
From: Finn Thain @ 2020-08-27 23:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: John Paul Adrian Glaubitz, Christoph Hellwig, Jens Axboe,
	Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k


> @@ -109,34 +111,28 @@ static void get_z2ram(void)
...
>  	}
> -
> -	return;
>  }
>  

It would be good to have a semantic patch for that change.

>  
> -		if (z2ram_map[z2ram_size] == 0) {
> +		if (z2ram_map[z2ram_size] == 0)
>  			break;
> -		}
>  

And that one.

Reason being, a semantic patch only has to be debugged once, whereas 
manual churn has to be done correctly over and over again.

TBH, this kind of transformation can happen when code is displayed.
It doesn't have to involve git at all. Otherwise, why have 5 billion 
transistors? You'd be better off with an Amiga.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-27 23:21         ` Finn Thain
@ 2020-08-28  0:01           ` Joe Perches
  2020-08-28  0:57             ` Finn Thain
  0 siblings, 1 reply; 40+ messages in thread
From: Joe Perches @ 2020-08-28  0:01 UTC (permalink / raw)
  To: Finn Thain
  Cc: John Paul Adrian Glaubitz, Christoph Hellwig, Jens Axboe,
	Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On Fri, 2020-08-28 at 09:21 +1000, Finn Thain wrote:
> > @@ -109,34 +111,28 @@ static void get_z2ram(void)
> ...
> >  	}
> > -
> > -	return;
> >  }
> >  
> 
> It would be good to have a semantic patch for that change.
> 
> >  
> > -		if (z2ram_map[z2ram_size] == 0) {
> > +		if (z2ram_map[z2ram_size] == 0)
> >  			break;
> > -		}
> >  
> 
> And that one.
> 
> Reason being, a semantic patch only has to be debugged once, whereas 
> manual churn has to be done correctly over and over again.

checkpatch already does this.



^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-28  0:01           ` Joe Perches
@ 2020-08-28  0:57             ` Finn Thain
  2020-08-28  1:11               ` Joe Perches
  2020-08-28  4:25               ` Christoph Hellwig
  0 siblings, 2 replies; 40+ messages in thread
From: Finn Thain @ 2020-08-28  0:57 UTC (permalink / raw)
  To: Joe Perches
  Cc: John Paul Adrian Glaubitz, Christoph Hellwig, Jens Axboe,
	Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On Thu, 27 Aug 2020, Joe Perches wrote:

> 
> checkpatch already does this.
> 

Did you use checkpatch to generate this patch?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-28  0:57             ` Finn Thain
@ 2020-08-28  1:11               ` Joe Perches
  2020-08-28  4:25               ` Christoph Hellwig
  1 sibling, 0 replies; 40+ messages in thread
From: Joe Perches @ 2020-08-28  1:11 UTC (permalink / raw)
  To: Finn Thain
  Cc: John Paul Adrian Glaubitz, Christoph Hellwig, Jens Axboe,
	Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On Fri, 2020-08-28 at 10:57 +1000, Finn Thain wrote:
> On Thu, 27 Aug 2020, Joe Perches wrote:
> 
> > checkpatch already does this.
> > 
> 
> Did you use checkpatch to generate this patch?

Nope.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/19] z2ram: reindent
  2020-08-28  0:57             ` Finn Thain
  2020-08-28  1:11               ` Joe Perches
@ 2020-08-28  4:25               ` Christoph Hellwig
  1 sibling, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-28  4:25 UTC (permalink / raw)
  To: Finn Thain
  Cc: Joe Perches, John Paul Adrian Glaubitz, Christoph Hellwig,
	Jens Axboe, Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

On Fri, Aug 28, 2020 at 10:57:46AM +1000, Finn Thain wrote:
> On Thu, 27 Aug 2020, Joe Perches wrote:
> 
> > 
> > checkpatch already does this.
> > 
> 
> Did you use checkpatch to generate this patch?

I used scripts/Lindent.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 12/19] md: use __register_blkdev to allocate devices on demand
  2020-08-26  6:24 ` [PATCH 12/19] md: " Christoph Hellwig
@ 2020-08-28  7:08   ` Song Liu
  0 siblings, 0 replies; 40+ messages in thread
From: Song Liu @ 2020-08-28  7:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Al Viro, linux-block, open list, linux-ide,
	linux-raid, linux-scsi, linux-m68k

On Tue, Aug 25, 2020 at 11:53 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Use the simpler mechanism attached to major_name to allocate a brd device
> when a currently unregistered minor is accessed.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Song Liu <song@kernel.org>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 03/19] block: cleanup del_gendisk a bit
  2020-09-03  8:01 ` [PATCH 03/19] block: cleanup del_gendisk a bit Christoph Hellwig
@ 2020-09-04 10:18   ` Hannes Reinecke
  0 siblings, 0 replies; 40+ messages in thread
From: Hannes Reinecke @ 2020-09-04 10:18 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, Finn Thain, Michael Schmitz,
	linux-block, linux-kernel, linux-ide, linux-raid, linux-scsi,
	linux-m68k

On 9/3/20 10:01 AM, Christoph Hellwig wrote:
> Merge three hidden gendisk checks into one.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/genhd.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@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

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 03/19] block: cleanup del_gendisk a bit
  2020-09-03  8:01 simplify gendisk lookup and remove struct block_device aliases v3 Christoph Hellwig
@ 2020-09-03  8:01 ` Christoph Hellwig
  2020-09-04 10:18   ` Hannes Reinecke
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2020-09-03  8:01 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, Finn Thain, Michael Schmitz,
	linux-block, linux-kernel, linux-ide, linux-raid, linux-scsi,
	linux-m68k

Merge three hidden gendisk checks into one.

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

diff --git a/block/genhd.c b/block/genhd.c
index 44f69f4b2c5aa6..ec9b64207d9c2e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -948,6 +948,9 @@ void del_gendisk(struct gendisk *disk)
 
 	might_sleep();
 
+	if (WARN_ON_ONCE(!disk->queue))
+		return;
+
 	blk_integrity_del(disk);
 	disk_del_events(disk);
 
@@ -970,20 +973,18 @@ void del_gendisk(struct gendisk *disk)
 	disk->flags &= ~GENHD_FL_UP;
 	up_write(&disk->lookup_sem);
 
-	if (!(disk->flags & GENHD_FL_HIDDEN))
+	if (!(disk->flags & GENHD_FL_HIDDEN)) {
 		sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
-	if (disk->queue) {
+
 		/*
 		 * Unregister bdi before releasing device numbers (as they can
 		 * get reused and we'd get clashes in sysfs).
 		 */
-		if (!(disk->flags & GENHD_FL_HIDDEN))
-			bdi_unregister(disk->queue->backing_dev_info);
-		blk_unregister_queue(disk);
-	} else {
-		WARN_ON(1);
+		bdi_unregister(disk->queue->backing_dev_info);
 	}
 
+	blk_unregister_queue(disk);
+	
 	if (!(disk->flags & GENHD_FL_HIDDEN))
 		blk_unregister_region(disk_devt(disk), disk->minors);
 	/*
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/19] block: cleanup del_gendisk a bit
  2020-08-30  6:24 simplify gendisk lookup and remove struct block_device aliases v2 Christoph Hellwig
@ 2020-08-30  6:24 ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2020-08-30  6:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Denis Efremov,
	David S. Miller, Song Liu, Al Viro, linux-block, linux-kernel,
	linux-ide, linux-raid, linux-scsi, linux-m68k

Merge three hidden gendisk checks into one.

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

diff --git a/block/genhd.c b/block/genhd.c
index cb9a51be35b053..df6485223a2c3d 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -973,22 +973,22 @@ void del_gendisk(struct gendisk *disk)
 	disk->flags &= ~GENHD_FL_UP;
 	up_write(&disk->lookup_sem);
 
-	if (!(disk->flags & GENHD_FL_HIDDEN))
+	WARN_ON(!disk->queue);
+
+	if (!(disk->flags & GENHD_FL_HIDDEN)) {
 		sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
-	if (disk->queue) {
+
 		/*
 		 * Unregister bdi before releasing device numbers (as they can
 		 * get reused and we'd get clashes in sysfs).
 		 */
-		if (!(disk->flags & GENHD_FL_HIDDEN))
-			bdi_unregister(disk->queue->backing_dev_info);
+		bdi_unregister(disk->queue->backing_dev_info);
 		blk_unregister_queue(disk);
+		blk_unregister_region(disk_devt(disk), disk->minors);
 	} else {
-		WARN_ON(1);
+		blk_unregister_queue(disk);
 	}
 
-	if (!(disk->flags & GENHD_FL_HIDDEN))
-		blk_unregister_region(disk_devt(disk), disk->minors);
 	/*
 	 * Remove gendisk pointer from idr so that it cannot be looked up
 	 * while RCU period before freeing gendisk is running to prevent
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2020-09-04 10:18 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-26  6:24 simplify gendisk lookup and remove struct block_device aliases Christoph Hellwig
2020-08-26  6:24 ` [PATCH 01/19] char_dev: replace cdev_map with an xarray Christoph Hellwig
2020-08-26  8:19   ` Greg Kroah-Hartman
2020-08-27  8:53     ` Christoph Hellwig
2020-08-27  9:18       ` Greg Kroah-Hartman
2020-08-27  9:39         ` Christoph Hellwig
2020-08-27  7:25   ` Hannes Reinecke
2020-08-27  8:55     ` Christoph Hellwig
2020-08-26  6:24 ` [PATCH 02/19] block: merge drivers/base/map.c into block/genhd.c Christoph Hellwig
2020-08-26  8:19   ` Greg Kroah-Hartman
2020-08-26  6:24 ` [PATCH 03/19] block: cleanup del_gendisk a bit Christoph Hellwig
2020-08-26  6:24 ` [PATCH 04/19] block: split block_class_lock Christoph Hellwig
2020-08-26  6:24 ` [PATCH 05/19] block: rework requesting modules for unclaimed devices Christoph Hellwig
2020-08-26  6:24 ` [PATCH 06/19] block: add an optional probe callback to major_names Christoph Hellwig
2020-08-26  6:24 ` [PATCH 07/19] ide: remove ide_{,un}register_region Christoph Hellwig
2020-08-27  7:25   ` Hannes Reinecke
2020-08-26  6:24 ` [PATCH 08/19] swim: don't call blk_register_region Christoph Hellwig
2020-08-26  6:24 ` [PATCH 09/19] sd: use __register_blkdev to avoid a modprobe for an unregistered dev_t Christoph Hellwig
2020-08-26  6:24 ` [PATCH 10/19] brd: use __register_blkdev to allocate devices on demand Christoph Hellwig
2020-08-26  6:24 ` [PATCH 11/19] loop: " Christoph Hellwig
2020-08-26  6:24 ` [PATCH 12/19] md: " Christoph Hellwig
2020-08-28  7:08   ` Song Liu
2020-08-26  6:24 ` [PATCH 13/19] ide: switch to __register_blkdev for command set probing Christoph Hellwig
2020-08-26  6:24 ` [PATCH 14/19] floppy: use a separate gendisk for each media format Christoph Hellwig
2020-08-26  6:24 ` [PATCH 15/19] amiflop: use separate gendisks for Amiga vs MS-DOS mode Christoph Hellwig
2020-08-26  6:24 ` [PATCH 16/19] ataflop: use a separate gendisk for each media format Christoph Hellwig
2020-08-26  6:24 ` [PATCH 17/19] z2ram: reindent Christoph Hellwig
2020-08-26  9:12   ` Joe Perches
2020-08-26  9:49     ` John Paul Adrian Glaubitz
2020-08-26 14:51       ` Joe Perches
2020-08-27 23:21         ` Finn Thain
2020-08-28  0:01           ` Joe Perches
2020-08-28  0:57             ` Finn Thain
2020-08-28  1:11               ` Joe Perches
2020-08-28  4:25               ` Christoph Hellwig
2020-08-26  6:24 ` [PATCH 18/19] z2ram: use separate gendisk for the different modes Christoph Hellwig
2020-08-26  6:24 ` [PATCH 19/19] block: switch gendisk lookup to a simple xarray Christoph Hellwig
2020-08-30  6:24 simplify gendisk lookup and remove struct block_device aliases v2 Christoph Hellwig
2020-08-30  6:24 ` [PATCH 03/19] block: cleanup del_gendisk a bit Christoph Hellwig
2020-09-03  8:01 simplify gendisk lookup and remove struct block_device aliases v3 Christoph Hellwig
2020-09-03  8:01 ` [PATCH 03/19] block: cleanup del_gendisk a bit Christoph Hellwig
2020-09-04 10:18   ` Hannes Reinecke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).