All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] block: Add new generic block device naming interface
@ 2013-04-25 20:22 Stephen M. Cameron
  2013-04-25 20:40 ` Tejun Heo
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen M. Cameron @ 2013-04-25 20:22 UTC (permalink / raw)
  To: axboe, neilb, hch, jmoyer, hare, tj, shaohua.li, vgoyal
  Cc: stephenmcameron, linux-kernel, lsorense

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

Currently, when adding a new block device driver, if grub support is needed,
then grub needs to be modified to know about this new driver.

See this thread:
https://lists.gnu.org/archive/html/grub-devel/2013-03/msg00049.html

If block drivers could (optionally) share a common device namespace then
grub could be taught about this common namespace once, and any new block
device drivers could take advantage of this namespace and grub would
automatically work with them.

The idea is that instead of each new block driver manually setting the
gendisk's disk_name field (though legacy drivers could still do that) they
would call blk_get_new_disk_name(struct gendisk *disk), and the block layer
hands out a name.  When removing a device, blk_put_disk_name() releases the
device name.

The scheme implemented in this patch is to name the devices /dev/bdn, with n
being an integer starting at zero that counts up, so you'd get device names
like /dev/bd0, /dev/bd1, /dev/bd2, etc.

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 block/genhd.c         |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/genhd.h |    2 ++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 3c001fb..d4f1588 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -568,6 +568,58 @@ exit:
 	disk_part_iter_exit(&piter);
 }
 
+static DEFINE_SPINLOCK(bd_index_lock);
+static DEFINE_IDA(bd_index_ida);
+char *blk_get_disk_name(struct gendisk *disk)
+{
+	int index, error;
+	char name[DISK_NAME_LEN + 1];
+
+	do {
+		if (!ida_pre_get(&bd_index_ida, GFP_KERNEL))
+			goto out;
+
+		spin_lock(&bd_index_lock);
+		error = ida_get_new(&bd_index_ida, &index);
+		spin_unlock(&bd_index_lock);
+	} while (error == -EAGAIN);
+
+	if (error) {
+		pr_warn("blk_get_disk_name: memory exhausted\n");
+		goto out;
+	}
+
+	snprintf(name, DISK_NAME_LEN, "bd%d", index);
+	if (strlen(name) > DISK_NAME_LEN - 1) {
+		pr_warn("block disk (bd) name length exceeded.\n");
+		goto out_free_index;
+	}
+	strncpy(disk->disk_name, name, DISK_NAME_LEN - 1);
+	return disk->disk_name;
+
+out_free_index:
+	spin_lock(&bd_index_lock);
+	ida_remove(&bd_index_ida, index);
+	spin_unlock(&bd_index_lock);
+out:
+	return NULL;
+}
+EXPORT_SYMBOL(blk_get_disk_name);
+
+int blk_put_disk_name(char *name)
+{
+	int index;
+
+	if (sscanf(name, "bd%d", &index) != 1)
+		return -EINVAL;
+
+	spin_lock(&bd_index_lock);
+	ida_remove(&bd_index_ida, index);
+	spin_unlock(&bd_index_lock);
+	return 0;
+}
+EXPORT_SYMBOL(blk_put_disk_name);
+
 /**
  * add_disk - add partitioning information to kernel list
  * @disk: per-device partitioning information
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 9f3c275..fddb41d 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -602,6 +602,8 @@ extern int blk_alloc_devt(struct hd_struct *part, dev_t *devt);
 extern void blk_free_devt(dev_t devt);
 extern dev_t blk_lookup_devt(const char *name, int partno);
 extern char *disk_name (struct gendisk *hd, int partno, char *buf);
+extern char *blk_get_disk_name(struct gendisk *disk);
+extern int blk_put_disk_name(char *name);
 
 extern int disk_expand_part_tbl(struct gendisk *disk, int target);
 extern int rescan_partitions(struct gendisk *disk, struct block_device *bdev);


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [RFC PATCH] block: Add new generic block device naming interface
@ 2013-03-28 16:18 Stephen M. Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen M. Cameron @ 2013-03-28 16:18 UTC (permalink / raw)
  To: axboe; +Cc: Sumant.Patro, chayan.biswas, lsorense, linux-kernel,
	stephenmcameron

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

Currently, when adding a new block device driver, if grub support is needed,
then grub needs to be modified to know about this new driver.

See this thread:
https://lists.gnu.org/archive/html/grub-devel/2013-03/msg00049.html

If block drivers could (optionally) share a common device namespace then
grub could be taught about this common namespace once, and any new block
device drivers could take advantage of this namespace and grub would
automatically work with them.

The idea is that instead of each new block driver manually setting the
gendisk's disk_name field (though legacy drivers could still do that) they
would call blk_get_new_disk_name(struct gendisk *disk), and the block layer
hands out a name.  When removing a device, blk_put_disk_name() releases the
device name.

The scheme implemented in this patch is to name the devices /dev/bdn, with n
being an integer starting at zero that counts up, so you'd get device names
like /dev/bd0, /dev/bd1, /dev/bd2, etc.

Maybe there is a good reason why such an obvious idea is not already
implemented, but if so, I don't know what that reason is.

-- steve
---
 block/genhd.c         |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/genhd.h |    2 ++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 3c001fb..d4f1588 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -568,6 +568,58 @@ exit:
 	disk_part_iter_exit(&piter);
 }
 
+static DEFINE_SPINLOCK(bd_index_lock);
+static DEFINE_IDA(bd_index_ida);
+char *blk_get_disk_name(struct gendisk *disk)
+{
+	int index, error;
+	char name[DISK_NAME_LEN + 1];
+
+	do {
+		if (!ida_pre_get(&bd_index_ida, GFP_KERNEL))
+			goto out;
+
+		spin_lock(&bd_index_lock);
+		error = ida_get_new(&bd_index_ida, &index);
+		spin_unlock(&bd_index_lock);
+	} while (error == -EAGAIN);
+
+	if (error) {
+		pr_warn("blk_get_disk_name: memory exhausted\n");
+		goto out;
+	}
+
+	snprintf(name, DISK_NAME_LEN, "bd%d", index);
+	if (strlen(name) > DISK_NAME_LEN - 1) {
+		pr_warn("block disk (bd) name length exceeded.\n");
+		goto out_free_index;
+	}
+	strncpy(disk->disk_name, name, DISK_NAME_LEN - 1);
+	return disk->disk_name;
+
+out_free_index:
+	spin_lock(&bd_index_lock);
+	ida_remove(&bd_index_ida, index);
+	spin_unlock(&bd_index_lock);
+out:
+	return NULL;
+}
+EXPORT_SYMBOL(blk_get_disk_name);
+
+int blk_put_disk_name(char *name)
+{
+	int index;
+
+	if (sscanf(name, "bd%d", &index) != 1)
+		return -EINVAL;
+
+	spin_lock(&bd_index_lock);
+	ida_remove(&bd_index_ida, index);
+	spin_unlock(&bd_index_lock);
+	return 0;
+}
+EXPORT_SYMBOL(blk_put_disk_name);
+
 /**
  * add_disk - add partitioning information to kernel list
  * @disk: per-device partitioning information
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 9f3c275..fddb41d 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -602,6 +602,8 @@ extern int blk_alloc_devt(struct hd_struct *part, dev_t *devt);
 extern void blk_free_devt(dev_t devt);
 extern dev_t blk_lookup_devt(const char *name, int partno);
 extern char *disk_name (struct gendisk *hd, int partno, char *buf);
+extern char *blk_get_disk_name(struct gendisk *disk);
+extern int blk_put_disk_name(char *name);
 
 extern int disk_expand_part_tbl(struct gendisk *disk, int target);
 extern int rescan_partitions(struct gendisk *disk, struct block_device *bdev);


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [RFC PATCH] block: Add new generic block device naming interface
@ 2013-03-11 19:00 Stephen M. Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen M. Cameron @ 2013-03-11 19:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: axboe, Chayan.Biswas, lsorense, Sumant.Patro, stephenmcameron

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

This is really the merest sketch of an idea, I'm not asking that
this patch be taken in its current form, just asking for feedback
on the idea, and if someone else better acquainted with the block
layer wants to take this idea and implement it in a better, proper
way, please, have at it.

Upon requesting advice from the grub developers about best practices
for naming device nodes for new block device drivers[*] to cause the
least amount of pain in grub, I was understandably greeted with the
advice to make it a SCSI driver or have a damn good reason why not.

See this thread:
https://lists.gnu.org/archive/html/grub-devel/2013-03/msg00049.html

Well, I did have what I think is a good reason, but this got me to
thinking that adding a new block device driver really shouldn't be causing
the grub guys any extra work, yet, it seems that it does.  This is because
there is no standardized naming system for block drivers as there is for
SCSI drivers -- the block device drivers kind of choose their own device
names, as does the sd driver -- but all the scsi hba drivers share the sd
driver -- so all the scsi block devices share the same device namespace,
and grub only has to know about this shared namespace.

So I cooked up a similar system for the block drivers, copied from sd
pretty much.  The idea is that instead of each new block driver manually
setting gendisk's disk_name field (though legacy drivers could still do
that) they call blk_assign_new_disk_name(struct gendisk *disk), and the
block layer hands out a name, so that all the new block drivers share
a namespace, and grub only has to learn about this one new naming
scheme, and any new block drivers can leverage that for booting support
automatically.

The scheme implemented in this patch is to name the devices /dev/bda,
/dev/bdb, /dev/bdc, etc.  in the exact same manner as the sd driver names
its devices /dev/sda, etc. (I literally copied the code from the sd
driver and tweaked it ever so slightly.)  This of course may not be the
best solution (as that ancient scheme came from what, SunOS?) but it
was the quickest to get up and running for a proof of concept.

It does occur to me that this seems like such an obvious idea that perhaps
there is a good reason why it's not done already, but if so, I don't know
what that reason is.

[*] the new driver I have in mind which triggered this question is here:
    https://github.com/HPSmartStorage/scsi-over-pcie

-- steve
---
 block/genhd.c         |   86 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/genhd.h |    1 +
 2 files changed, 87 insertions(+), 0 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 3c001fb..9d735f6 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -569,6 +569,92 @@ exit:
 }
 
 /**
+ *	bd_format_disk_name - format disk name
+ *	@prefix: name prefix - ie. "bd" for SCSI disks
+ *	@index: index of the disk to format name for
+ *	@buf: output buffer
+ *	@buflen: length of the output buffer
+ *
+ *	block disk names starts at bda.  The 26th device is bdz and the
+ *	27th is bdaa.  The last one for two lettered suffix is bdzz
+ *	which is followed by bdaaa.
+ *
+ *	This is basically 26 base counting with one extra 'nil' entry
+ *	at the beginning from the second digit on and can be
+ *	determined using similar method as 26 base conversion with the
+ *	index shifted -1 after each digit is computed.
+ *
+ *	CONTEXT:
+ *	Don't care.
+ *
+ *	RETURNS:
+ *	0 on success, -errno on failure.
+ *
+ *	bd_format_disk_name is obviously cribbed from sd_format_disk_name
+ *
+ */
+static int bd_format_disk_name(char *prefix, int index, char *buf, int buflen)
+{
+	const int base = 'z' - 'a' + 1;
+	char *begin = buf + strlen(prefix);
+	char *end = buf + buflen;
+	char *p;
+	int unit;
+
+	p = end - 1;
+	*p = '\0';
+	unit = base;
+	do {
+		if (p == begin)
+			return -EINVAL;
+		*--p = 'a' + (index % unit);
+		index = (index / unit) - 1;
+	} while (index >= 0);
+
+	memmove(begin, p, end - p);
+	memcpy(buf, prefix, strlen(prefix));
+
+	return 0;
+}
+
+DEFINE_SPINLOCK(bd_index_lock);
+DEFINE_IDA(bd_index_ida);
+char *blk_assign_new_disk_name(struct gendisk *disk)
+{
+	int index, error;
+
+	do {
+		if (!ida_pre_get(&bd_index_ida, GFP_KERNEL))
+			goto out;
+
+		spin_lock(&bd_index_lock);
+		error = ida_get_new(&bd_index_ida, &index);
+		spin_unlock(&bd_index_lock);
+	} while (error == -EAGAIN);
+
+	if (error) {
+		pr_warn("blk_assign_new_disk_name: memory exhausted\n");
+		goto out;
+	}
+
+	error = bd_format_disk_name("bd", index, disk->disk_name,
+					DISK_NAME_LEN);
+	if (error) {
+		pr_warn("block disk (bd) name length exceeded.\n");
+		goto out_free_index;
+	}
+	return disk->disk_name;
+
+out_free_index:
+	spin_lock(&bd_index_lock);
+	ida_remove(&bd_index_ida, index);
+	spin_unlock(&bd_index_lock);
+out:
+	return NULL;
+}
+EXPORT_SYMBOL(blk_assign_new_disk_name);
+
+/**
  * add_disk - add partitioning information to kernel list
  * @disk: per-device partitioning information
  *
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 9f3c275..cef2169 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -602,6 +602,7 @@ extern int blk_alloc_devt(struct hd_struct *part, dev_t *devt);
 extern void blk_free_devt(dev_t devt);
 extern dev_t blk_lookup_devt(const char *name, int partno);
 extern char *disk_name (struct gendisk *hd, int partno, char *buf);
+extern char *blk_assign_new_disk_name(struct gendisk *hd);
 
 extern int disk_expand_part_tbl(struct gendisk *disk, int target);
 extern int rescan_partitions(struct gendisk *disk, struct block_device *bdev);


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-04-29 16:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-25 20:22 [RFC PATCH] block: Add new generic block device naming interface Stephen M. Cameron
2013-04-25 20:40 ` Tejun Heo
2013-04-25 21:07   ` scameron
2013-04-25 21:14     ` Tejun Heo
2013-04-25 22:12       ` scameron
2013-04-26 19:03         ` Tejun Heo
2013-04-29 14:56           ` Hannes Reinecke
2013-04-29 15:17             ` Vivek Goyal
2013-04-29 16:06             ` Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2013-03-28 16:18 Stephen M. Cameron
2013-03-11 19:00 Stephen M. Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.