Skip to contentSkip to conversation

blktap: make max # of tap devices a module parameter

PATCH In review Laszlo Ersek 3 participants 11 messages Feb 22, 2011 – Feb 24, 2011 Xen hypervisor interface

lore
Share & tools
lore

Laszlo Ersek

author
#0 lore raw

PATCH 1/1 blktap: make max # of tap devices a module parameter

Hi,

should anybody still use the blktap(1) driver in linux-2.6.18-xen, the following patch intends to make the maximum number of tapdevs configurable at module insertion time. The number is clamped to [256 .. NR_EVENT_CHANNELS]. I removed the definition of MAX_DEV_NAME because it didn't seem to be used at all.

Thanks for considering,
Laszlo Ersek

drivers/xen/blktap/blktap.c +43 -12
diff -r 2994d2997f9d drivers/xen/blktap/blktap.c
@@ -55,8 +55,7 @@
 #include <linux/delay.h>
 #include <asm/tlbflush.h>
 
-#define MAX_TAP_DEV 256     /*the maximum number of tapdisk ring devices    */
-#define MAX_DEV_NAME 100    /*the max tapdisk ring device name e.g. blktap0 */
+#define DEF_MAX_TAP_DEV 256 /* default maximum of tapdisk ring devices */
 
 /*
  * The maximum number of requests that can be outstanding at any time

@@ -120,7 +119,8 @@
 	struct vm_foreign_map foreign_map;    /*Mapping page */
 } tap_blkif_t;
 
-static struct tap_blkif *tapfds[MAX_TAP_DEV];
+/* Points to the first of "max_tap_dev" objects. */
+static struct tap_blkif **tapfds;
 static int blktap_next_minor;
 
 /* Run-time switchable: /sys/module/blktap/parameters/ */

@@ -129,6 +129,12 @@
 module_param(log_stats, int, 0644);
 module_param(debug_lvl, int, 0644);
 
+/* Settable only at initialization time. */
+static int max_tap_dev = DEF_MAX_TAP_DEV;
+module_param(max_tap_dev, int, 0444);
+MODULE_PARM_DESC(max_tap_dev, "maximum number of tapdisk devices, defaults to "
+		 __stringify(DEF_MAX_TAP_DEV));
+
 /*
  * Each outstanding request that we've passed to the lower device layers has a 
  * 'pending_req' allocated to it. Each buffer_head that completes decrements 

@@ -470,7 +476,7 @@
 	 * available.  This is done while we are still under
 	 * the protection of the pending_free_lock.
 	 */
-	if (blktap_next_minor < MAX_TAP_DEV)
+	if (blktap_next_minor < max_tap_dev)
 		minor = blktap_next_minor++;
 found:
 	spin_unlock_irq(&pending_free_lock);

@@ -538,7 +544,7 @@
 	 * if the userland tools set things up wrong, this could be negative;
 	 * just don't try to signal in this case
 	 */
-	if (idx < 0 || idx >= MAX_TAP_DEV)
+	if (idx < 0 || idx >= max_tap_dev)
 		return;
 
 	info = tapfds[idx];

@@ -567,7 +573,7 @@
 	/* ctrl device, treat differently */
 	if (!idx)
 		return 0;
-	if (idx < 0 || idx >= MAX_TAP_DEV) {
+	if (idx < 0 || idx >= max_tap_dev) {
 		WPRINTK("No device /dev/xen/blktap%d\n", idx);
 		return -ENODEV;
 	}

@@ -836,7 +842,7 @@
 		unsigned long dev = arg;
 		unsigned long flags;
 
-		if (info || dev >= MAX_TAP_DEV)
+		if (info || dev >= max_tap_dev)
 			return -EINVAL;
 
 		info = tapfds[dev];

@@ -854,7 +860,7 @@
 		if (!info) {
 			unsigned long dev = arg;
 
-			if (dev >= MAX_TAP_DEV)
+			if (dev >= max_tap_dev)
 				return -EINVAL;
 
 			info = tapfds[dev];

@@ -895,7 +901,7 @@
 {
 	tap_blkif_t *info;
 
-	if (idx < 0 || idx >= MAX_TAP_DEV)
+	if (idx < 0 || idx >= max_tap_dev)
 		return;
 
 	info = tapfds[idx];

@@ -1045,7 +1051,7 @@
 	struct mm_struct *mm;
 	
 
-	if ((tapidx < 0) || (tapidx >= MAX_TAP_DEV)
+	if ((tapidx < 0) || (tapidx >= max_tap_dev)
 	    || !(info = tapfds[tapidx])) {
 		WPRINTK("fast_flush: Couldn't get info!\n");
 		return;

@@ -1298,7 +1304,7 @@
 	rmb(); /* Ensure we see queued requests up to 'rp'. */
 
 	/*Check blkif has corresponding UE ring*/
-	if (blkif->dev_num < 0 || blkif->dev_num >= MAX_TAP_DEV) {
+	if (blkif->dev_num < 0 || blkif->dev_num >= max_tap_dev) {
 		/*oops*/
 		if (print_dbug) {
 			WPRINTK("Corresponding UE " 

@@ -1413,7 +1419,7 @@
 	struct mm_struct *mm;
 	struct vm_area_struct *vma = NULL;
 
-	if (blkif->dev_num < 0 || blkif->dev_num >= MAX_TAP_DEV)
+	if (blkif->dev_num < 0 || blkif->dev_num >= max_tap_dev)
 		goto fail_response;
 
 	info = tapfds[blkif->dev_num];

@@ -1696,6 +1702,21 @@
 		notify_remote_via_irq(blkif->irq);
 }
 
+static int alloc_tapfds(void)
+{
+	int new = clamp_t(int, max_tap_dev, DEF_MAX_TAP_DEV,
+			  NR_EVENT_CHANNELS);
+
+	if (new != max_tap_dev) {
+		WPRINTK("clamping max_tap_dev from %d to %d\n", max_tap_dev,
+			new);
+		max_tap_dev = new;
+	}
+
+	tapfds = kmalloc(max_tap_dev * sizeof(*tapfds), GFP_KERNEL);
+	return tapfds ? 0 : -ENOMEM;
+}
+
 static int __init blkif_init(void)
 {
 	int i, ret;

@@ -1704,6 +1725,16 @@
 	if (!is_running_on_xen())
 		return -ENODEV;
 
+	ret = alloc_tapfds();
+	if (ret != 0) {
+		WPRINTK("failed to allocate tapfds\n");
+		return ret;
+	}
+	/* Not released on error paths below, because the failure of
+	 * register_chrdev() leaks whatever was allocated by the req_increase()
+	 * calls anyway.
+	 */
+
 	INIT_LIST_HEAD(&pending_free);
         for(i = 0; i < 2; i++) {
 		ret = req_increase();

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 11 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek

Hi,

should anybody still use the blktap(1) driver in linux-2.6.18-xen, the following patch intends to make the maximum number of tapdevs configurable at module insertion time. The number is clamped to [256 .. NR_EVENT_CHANNELS]. I removed the definition of MAX_DEV_NAME because it didn't seem to be used at all.

Thanks for considering,
Laszlo Ersek

Without replacing the call to register_chrdev() with one to __register_chrdev() (available only with 2.6.32 and newer) I can't see how you would get beyond 256 devices with the changes you propose.

Jan

author
#2 lore raw
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek

Hi,

should anybody still use the blktap(1) driver in linux-2.6.18-xen, the following patch intends to make the maximum number of tapdevs configurable at module insertion time. The number is clamped to [256 .. NR_EVENT_CHANNELS]. I removed the definition of MAX_DEV_NAME because it didn't seem to be used at all.

Quoted 7 lines
Quoted 2 lines

Thanks for considering,
Laszlo Ersek

Without replacing the call to register_chrdev() with one to __register_chrdev() (available only with 2.6.32 and newer) I can't see how you would get beyond 256 devices with the changes you propose.

Oops, sorry; I naively assumed that minor device numbers were already covered by an earlier change.

I figure register_chrdev() could be reimplemented in blktap, based on lower-level char_dev.c (and kobject) primitives, but I'm not sure if the original goal is worth that ugliness. In any case, should I bother posting a version like that eventually, or would it have no chance of being accepted?

Thanks & sorry for the noise.
lacos

#3 lore raw
Quoted 18 lines · Laszlo Ersek
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek

Hi,

should anybody still use the blktap(1) driver in linux-2.6.18-xen, the following patch intends to make the maximum number of tapdevs configurable at module insertion time. The number is clamped to [256 .. NR_EVENT_CHANNELS]. I removed the definition of MAX_DEV_NAME because it didn't seem to be used at all.

Quoted 7 lines
Quoted 2 lines

Thanks for considering,
Laszlo Ersek

Without replacing the call to register_chrdev() with one to __register_chrdev() (available only with 2.6.32 and newer) I can't see how you would get beyond 256 devices with the changes you propose.

Quoted 8 lines

Oops, sorry; I naively assumed that minor device numbers were already covered by an earlier change.

I figure register_chrdev() could be reimplemented in blktap, based on lower-level char_dev.c (and kobject) primitives, but I'm not sure if the original goal is worth that ugliness. In any case, should I bother posting a version like that eventually, or would it have no chance of being accepted?

I'm pretty sure minors > 256 date way before 2.6.32. Here's the module init fragment from blktap2, replacing the register_chrdev() call:

int __init
blktap_ring_init(void)

{
	dev_t dev = 0;
	int err;

	cdev_init(&blktap_ring_cdev, &blktap_ring_file_operations);
	blktap_ring_cdev.owner = THIS_MODULE;

	err = alloc_chrdev_region(&dev, 0, MAX_BLKTAP_DEVICE, "blktap2");
	if (err < 0) {
		BTERR("error registering ring devices: %d\n", err);
		return err;
	}

	err = cdev_add(&blktap_ring_cdev, dev, MAX_BLKTAP_DEVICE);
	if (err) {
		BTERR("error adding ring device: %d\n", err);
		unregister_chrdev_region(dev, MAX_BLKTAP_DEVICE);
		return err;
	}

	blktap_ring_major = MAJOR(dev);
	BTINFO("blktap ring major: %d\n", blktap_ring_major);

	return 0;
}

void
blktap_ring_exit(void)

{
	if (!blktap_ring_major)
		return;

	cdev_del(&blktap_ring_cdev);
	unregister_chrdev_region(MKDEV(blktap_ring_major, 0),
				 MAX_BLKTAP_DEVICE);

	blktap_ring_major = 0;
}

Happy hacking.

Daniel

author
#4 lore raw
Quoted 4 lines · Daniel Stodden

I'm pretty sure minors> 256 date way before 2.6.32. Here's the module init fragment from blktap2, replacing the register_chrdev() call:

[...]

That's about what I was thinking of in [0]. It also sets the cdev's owner manually. However, register_chrdev() also does this:

   225		kobject_set_name(&cdev->kobj, "%s", name);
   226		for (s = strchr(kobject_name(&cdev->kobj),'/'); s; s = strchr(s, '/'))
   227			*s = '!';

I reckon we can ignore the 's,/,!,g' replacement, since the name is fixed "blktap" (or "blktap2"). But the kobject name doesn't appear to be set in the first place. Is that no problem? If not, I'd just omit it from blktap as well.

Thank you!
lacos

[0] https://bugzilla.redhat.com/show_bug.cgi?id=452650#c21

#5 lore raw
Quoted 9 lines · Laszlo Ersek
Quoted 4 lines · Daniel Stodden

I'm pretty sure minors> 256 date way before 2.6.32. Here's the module init fragment from blktap2, replacing the register_chrdev() call:

[...]

That's about what I was thinking of in [0]. It also sets the cdev's owner manually. However, register_chrdev() also does this:

Quoted 7 lines
   225		kobject_set_name(&cdev->kobj, "%s", name);
   226		for (s = strchr(kobject_name(&cdev->kobj),'/'); s; s = strchr(s, '/'))
   227			*s = '!';

I reckon we can ignore the 's,/,!,g' replacement, since the name is fixed "blktap" (or "blktap2"). But the kobject name doesn't appear to be set in the first place.

Is that no problem? If not, I'd just omit it from blktap as well.

I remember I browsed a whole lot of code back then when I wrote that. All gone now :)

After looking again, I think the way this works was that the kobj map holds the blktap2 class device, nowhere a cdev->kobj, after device_register (blktap2/sysfs.c) actually placed a node for that minor. Before then, only the range is allocated, but won't show up in sysfs. So I guess cdev->kobj is a somewhat anonymous refcount bag only.

# l /sys/dev/char/* | grep blktap
lrwxrwxrwx 1 root root 0 Feb 22 10:24 /sys/dev/char/10:61 -> ../../class/misc/blktap-control
lrwxrwxrwx 1 root root 0 Feb 22 10:24 /sys/dev/char/254:0 -> ../../class/blktap2/blktap0
lrwxrwxrwx 1 root root 0 Feb 22 10:24 /sys/dev/char/254:1 -> ../../class/blktap2/blktap1
lrwxrwxrwx 1 root root 0 Feb 22 10:24 /sys/dev/char/254:2 -> ../../class/blktap2/blktap2

Compare this to the number of tty mappings you'll probably find on your box in the same directory.

Daniel

#10 lore raw
In reply to Laszlo Ersek
Quoted 19 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek

Hi,

should anybody still use the blktap(1) driver in linux-2.6.18-xen, the following patch intends to make the maximum number of tapdevs configurable at module insertion time. The number is clamped to [256 .. NR_EVENT_CHANNELS]. I removed the definition of MAX_DEV_NAME because it didn't seem to be used at all.

Quoted 7 lines
Quoted 2 lines

Thanks for considering,
Laszlo Ersek

Without replacing the call to register_chrdev() with one to __register_chrdev() (available only with 2.6.32 and newer) I can't see how you would get beyond 256 devices with the changes you propose.

Quoted 8 lines

Oops, sorry; I naively assumed that minor device numbers were already covered by an earlier change.

I figure register_chrdev() could be reimplemented in blktap, based on lower-level char_dev.c (and kobject) primitives, but I'm not sure if the original goal is worth that ugliness. In any case, should I bother posting a version like that eventually, or would it have no chance of being accepted?

I too had thought about doing this in the past, but decided there#s not much point in it.

Jan

#6 lore raw
In reply to Daniel Stodden
Quoted 23 lines
Quoted 1 line
Quoted 1 line · Daniel Stodden
Quoted 21 lines · Laszlo Ersek
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 1 line
Quoted 1 line · Laszlo Ersek

Hi,

should anybody still use the blktap(1) driver in linux-2.6.18-xen, the following patch intends to make the maximum number of tapdevs configurable at module insertion time. The number is clamped to [256 .. NR_EVENT_CHANNELS]. I removed the definition of MAX_DEV_NAME because it didn't seem to be used at all.

Quoted 7 lines
Quoted 2 lines

Thanks for considering,
Laszlo Ersek

Without replacing the call to register_chrdev() with one to __register_chrdev() (available only with 2.6.32 and newer) I can't see how you would get beyond 256 devices with the changes you propose.

Oops, sorry; I naively assumed that minor device numbers were already covered by an earlier change.

Quoted 8 lines
Quoted 5 lines

I figure register_chrdev() could be reimplemented in blktap, based on lower-level char_dev.c (and kobject) primitives, but I'm not sure if the original goal is worth that ugliness. In any case, should I bother posting a version like that eventually, or would it have no chance of being accepted?

I'm pretty sure minors > 256 date way before 2.6.32. Here's the module init fragment from blktap2, replacing the register_chrdev() call:

Sure, just that you have do more things "manually".

Quoted 19 lines

int __init
blktap_ring_init(void)

{
	dev_t dev = 0;
	int err;

	cdev_init(&blktap_ring_cdev, &blktap_ring_file_operations);
	blktap_ring_cdev.owner = THIS_MODULE;

	err = alloc_chrdev_region(&dev, 0, MAX_BLKTAP_DEVICE, "blktap2");
	if (err < 0) {
		BTERR("error registering ring devices: %d\n", err);
		return err;
	}

	err = cdev_add(&blktap_ring_cdev, dev, MAX_BLKTAP_DEVICE);
	if (err) {
		BTERR("error adding ring device: %d\n", err);
		unregister_chrdev_region(dev, MAX_BLKTAP_DEVICE);
Quoted 8 lines
		return err;
	}

	blktap_ring_major = MAJOR(dev);
	BTINFO("blktap ring major: %d\n", blktap_ring_major);

	return 0;
}

Any reason why in .32 and newer you still don't use __register_chrdev() (and __unregister_chrdev())? And even if this still needs to be this way, I note that unregister_chrdev() calls __unregister_chrdev_region() before cdev_del(), while blktap_ring_exit() does it the other way around?

Jan

#7 lore raw
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jan Beulich

Any reason why in .32 and newer you still don't use __register_chrdev() (and __unregister_chrdev())? And even if this still needs to be this way, I note that unregister_chrdev() calls __unregister_chrdev_region() before cdev_del(), while blktap_ring_exit() does it the other way around?

I think this could be cleaned up like this:

drivers/xen/blktap/ring.c +5 -17
@@ -8,7 +8,6 @@
 #include "blktap.h"
 
 int blktap_ring_major;
-static struct cdev blktap_ring_cdev;
 
  /* 
   * BLKTAP - immediately before the mmap area,

@@ -511,26 +510,16 @@ blktap_ring_debug(struct blktap *tap, ch
 int __init
 blktap_ring_init(void)
 {
-	dev_t dev = 0;
 	int err;
 
-	cdev_init(&blktap_ring_cdev, &blktap_ring_file_operations);
-	blktap_ring_cdev.owner = THIS_MODULE;
-
-	err = alloc_chrdev_region(&dev, 0, MAX_BLKTAP_DEVICE, "blktap2");
+	err = __register_chrdev(0, 0, MAX_BLKTAP_DEVICE, "blktap2",
+				&blktap_ring_file_operations);
 	if (err < 0) {
 		BTERR("error registering ring devices: %d\n", err);
 		return err;
 	}
 
-	err = cdev_add(&blktap_ring_cdev, dev, MAX_BLKTAP_DEVICE);
-	if (err) {
-		BTERR("error adding ring device: %d\n", err);
-		unregister_chrdev_region(dev, MAX_BLKTAP_DEVICE);
-		return err;
-	}
-
-	blktap_ring_major = MAJOR(dev);
+	blktap_ring_major = err;
 	BTINFO("blktap ring major: %d\n", blktap_ring_major);
 
 	return 0;

@@ -542,9 +531,8 @@ blktap_ring_exit(void)
 	if (!blktap_ring_major)
 		return;
 
-	cdev_del(&blktap_ring_cdev);
-	unregister_chrdev_region(MKDEV(blktap_ring_major, 0),
-				 MAX_BLKTAP_DEVICE);
+	__unregister_chrdev(blktap_ring_major, 0, MAX_BLKTAP_DEVICE,
+			    "blktap2");
 
 	blktap_ring_major = 0;
 }

And probably converting MAX_BLKTAP_DEVICE into a configure option would also be reasonable.

In any case, if I wanted to formally submit patches to clean up this and some other things in the pv-ops variant, what (preferably non-topic) branch should those be against? If I'm not mistaken, xen/next-2.6.38 for example doesn't even have a blktap - or did it get moved out of drivers/xen/? And what would be the most up-to-date non-experimental branch to pull blktap bits from?

Jan

#8 lore raw
Quoted 6 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jan Beulich

Any reason why in .32 and newer you still don't use __register_chrdev() (and __unregister_chrdev())? And even if this still needs to be this way, I note that unregister_chrdev() calls __unregister_chrdev_region() before cdev_del(), while blktap_ring_exit() does it the other way around?

I wrote this for .27 iirc and haven't looked into char_dev since then. I'm always for prettification.

Quoted 55 lines

I think this could be cleaned up like this:

drivers/xen/blktap/ring.c +5 -17
@@ -8,7 +8,6 @@
 #include "blktap.h"
 
 int blktap_ring_major;
-static struct cdev blktap_ring_cdev;
 
  /* 
   * BLKTAP - immediately before the mmap area,

@@ -511,26 +510,16 @@ blktap_ring_debug(struct blktap *tap, ch
 int __init
 blktap_ring_init(void)
 {
-	dev_t dev = 0;
 	int err;
 
-	cdev_init(&blktap_ring_cdev, &blktap_ring_file_operations);
-	blktap_ring_cdev.owner = THIS_MODULE;
-
-	err = alloc_chrdev_region(&dev, 0, MAX_BLKTAP_DEVICE, "blktap2");
+	err = __register_chrdev(0, 0, MAX_BLKTAP_DEVICE, "blktap2",
+				&blktap_ring_file_operations);
 	if (err < 0) {
 		BTERR("error registering ring devices: %d\n", err);
 		return err;
 	}
 
-	err = cdev_add(&blktap_ring_cdev, dev, MAX_BLKTAP_DEVICE);
-	if (err) {
-		BTERR("error adding ring device: %d\n", err);
-		unregister_chrdev_region(dev, MAX_BLKTAP_DEVICE);
-		return err;
-	}
-
-	blktap_ring_major = MAJOR(dev);
+	blktap_ring_major = err;
 	BTINFO("blktap ring major: %d\n", blktap_ring_major);
 
 	return 0;

@@ -542,9 +531,8 @@ blktap_ring_exit(void)
 	if (!blktap_ring_major)
 		return;
 
-	cdev_del(&blktap_ring_cdev);
-	unregister_chrdev_region(MKDEV(blktap_ring_major, 0),
-				 MAX_BLKTAP_DEVICE);
+	__unregister_chrdev(blktap_ring_major, 0, MAX_BLKTAP_DEVICE,
+			    "blktap2");
 
 	blktap_ring_major = 0;
 }
Quoted 2 lines

And probably converting MAX_BLKTAP_DEVICE into a configure option would also be reasonable.

If you could confirm that __register_chrdev doesn't grow O(n) in space anywhere (I guess it doesn't), then I don't think that people should be required to spend much thought on it.

It just to matter because the the *blktaps[minor] vector was statically allocated. Nowadays it grows during ALLOC_TAP, base-2.

We can set it to either something insanely large, provided toolstacks don't shoot themselves by running tap-ctl allocate in a loop.

Or keep it large enough so few would ever have to care (I thought 1024 would be just that), and add a sysfs node to override that limit.

Quoted 5 lines

In any case, if I wanted to formally submit patches to clean up this and some other things in the pv-ops variant, what (preferably non-topic) branch should those be against? If I'm not mistaken, xen/next-2.6.38 for example doesn't even have a blktap - or did it get moved out of drivers/xen/?

I've got a patch for .38 mostly ready. It's going to move into drivers/block/blktap.

Plus and some thoughts on logical block size, barrier support and some early trim stuff.

Quoted 2 lines

And what would be the most
up-to-date non-experimental branch to pull blktap bits from?

That'd be my tree on xenbits after I pushed some more stuff up there. Which apparently is gone. Anyone knowing what's going on there? Did I miss some organizational change or is it just broken?

Used to be
http://xenbits.xensource.com/gitweb/?p=people/dstodden/linux.git/.git;a=summary

Daniel

#9 lore raw
In reply to Jan Beulich
Quoted 20 lines · Jan Beulich

/*
* BLKTAP - immediately before the mmap area,

drivers/xen/blktap/ring.c +2 -5
@@ -511,26 +510,16 @@ blktap_ring_debug(struct blktap *tap, ch
 int __init
 blktap_ring_init(void)
 {
-	dev_t dev = 0;
 	int err;
 
-	cdev_init(&blktap_ring_cdev, &blktap_ring_file_operations);
-	blktap_ring_cdev.owner = THIS_MODULE;
-
-	err = alloc_chrdev_region(&dev, 0, MAX_BLKTAP_DEVICE, "blktap2");
+	err = __register_chrdev(0, 0, MAX_BLKTAP_DEVICE, "blktap2",
+				&blktap_ring_file_operations);
 	if (err < 0) {
 		BTERR("error registering ring devices: %d\n", err);
 		return err;
 	}

drivers/xen/blktap/ring.c blktap_ring_debug(struct blktap *tap, ch · line 523

Quoted 10 lines
drivers/xen/blktap/ring.c +1 -8
-	err = cdev_add(&blktap_ring_cdev, dev, MAX_BLKTAP_DEVICE);
-	if (err) {
-		BTERR("error adding ring device: %d\n", err);
-		unregister_chrdev_region(dev, MAX_BLKTAP_DEVICE);
-		return err;
-	}
-
-	blktap_ring_major = MAJOR(dev);
+	blktap_ring_major = err;
 	BTINFO("blktap ring major: %d\n", blktap_ring_major);

While you are at it: Feel free to drop init message(s?), too. Blktap is not boot critical, and there are plenty alternative places to check presence.

Cheers,
Daniel