[PATCH 1/4] void unregister_blkdev - do WARN_ON failure

[PATCH 1/4] void unregister_blkdev - do WARN_ON failure

[Akinobu Mita]

When unregister_blkdev() has failed, something wrong happened.
This patch adds WARN_ON to notify such badness.

Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

---
 block/genhd.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: 2.6-mm/block/genhd.c
===================================================================
--- 2.6-mm.orig/block/genhd.c
+++ 2.6-mm/block/genhd.c
@@ -120,9 +120,10 @@ int unregister_blkdev(unsigned int major
 	for (n = &major_names[index]; *n; n = &(*n)->next)
 		if ((*n)->major == major)
 			break;
-	if (!*n || strcmp((*n)->name, name))
+	if (!*n || strcmp((*n)->name, name)) {
+		WARN_ON(1);
 		ret = -EINVAL;
-	else {
+	} else {
 		p = *n;
 		*n = p->next;
 	}

[PATCH 2/4] void unregister_blkdev - delete redundant messages

[Akinobu Mita]

No need to warn unregister_blkdev() failure by the callers.
(The previous patch makes unregister_blkdev() print error message in
error case)

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

---
 drivers/block/acsi.c         |    4 +---
 drivers/block/loop.c         |    3 +--
 drivers/block/z2ram.c        |    4 +---
 drivers/cdrom/cdu31a.c       |    4 +---
 drivers/cdrom/mcdx.c         |   11 ++---------
 drivers/cdrom/sbpcd.c        |    5 +----
 drivers/cdrom/sjcd.c         |    9 ++++-----
 drivers/cdrom/sonycd535.c    |    6 ++----
 drivers/md/dm.c              |    4 +---
 drivers/s390/block/dcssblk.c |    7 +------
 drivers/sbus/char/jsflash.c  |    3 +--
 11 files changed, 16 insertions(+), 44 deletions(-)

Index: 2.6-mm/drivers/block/acsi.c
===================================================================
--- 2.6-mm.orig/drivers/block/acsi.c
+++ 2.6-mm/drivers/block/acsi.c
@@ -1775,9 +1775,7 @@ void cleanup_module(void)
 	del_timer( &acsi_timer );
 	blk_cleanup_queue(acsi_queue);
 	atari_stram_free( acsi_buffer );
-
-	if (unregister_blkdev( ACSI_MAJOR, "ad" ) != 0)
-		printk( KERN_ERR "acsi: cleanup_module failed\n");
+	unregister_blkdev(ACSI_MAJOR, "ad");
 
 	for (i = 0; i < NDevices; i++) {
 		del_gendisk(acsi_gendisk[i]);
Index: 2.6-mm/drivers/block/z2ram.c
===================================================================
--- 2.6-mm.orig/drivers/block/z2ram.c
+++ 2.6-mm/drivers/block/z2ram.c
@@ -371,9 +371,7 @@ static void __exit z2_exit(void)
 {
     int i, j;
     blk_unregister_region(MKDEV(Z2RAM_MAJOR, 0), 256);
-    if ( unregister_blkdev( Z2RAM_MAJOR, DEVICE_NAME ) != 0 )
-	printk( KERN_ERR DEVICE_NAME ": unregister of device failed\n");
-
+    unregister_blkdev(Z2RAM_MAJOR, DEVICE_NAME);
     del_gendisk(z2ram_gendisk);
     put_disk(z2ram_gendisk);
     blk_cleanup_queue(z2_queue);
Index: 2.6-mm/drivers/s390/block/dcssblk.c
===================================================================
--- 2.6-mm.orig/drivers/s390/block/dcssblk.c
+++ 2.6-mm/drivers/s390/block/dcssblk.c
@@ -747,14 +747,9 @@ dcssblk_check_params(void)
 static void __exit
 dcssblk_exit(void)
 {
-	int rc;
-
 	PRINT_DEBUG("DCSSBLOCK EXIT...\n");
 	s390_root_dev_unregister(dcssblk_root_dev);
-	rc = unregister_blkdev(dcssblk_major, DCSSBLK_NAME);
-	if (rc) {
-		PRINT_ERR("unregister_blkdev() failed!\n");
-	}
+	unregister_blkdev(dcssblk_major, DCSSBLK_NAME);
 	PRINT_DEBUG("...finished!\n");
 }
 
Index: 2.6-mm/drivers/sbus/char/jsflash.c
===================================================================
--- 2.6-mm.orig/drivers/sbus/char/jsflash.c
+++ 2.6-mm/drivers/sbus/char/jsflash.c
@@ -619,8 +619,7 @@ static void __exit jsflash_cleanup_modul
 	jsf0.busy = 0;
 
 	misc_deregister(&jsf_dev);
-	if (unregister_blkdev(JSFD_MAJOR, "jsfd") != 0)
-		printk("jsfd: cleanup_module failed\n");
+	unregister_blkdev(JSFD_MAJOR, "jsfd");
 	blk_cleanup_queue(jsf_queue);
 }
 
Index: 2.6-mm/drivers/cdrom/mcdx.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/mcdx.c
+++ 2.6-mm/drivers/cdrom/mcdx.c
@@ -1050,13 +1050,7 @@ static void __exit mcdx_exit(void)
 		kfree(stuffp);
 	}
 
-	if (unregister_blkdev(MAJOR_NR, "mcdx") != 0) {
-		xwarn("cleanup() unregister_blkdev() failed\n");
-	}
-#if !MCDX_QUIET
-	else
-	xinfo("cleanup() succeeded\n");
-#endif
+	unregister_blkdev(MAJOR_NR, "mcdx");
 	blk_cleanup_queue(mcdx_queue);
 }
 
@@ -1240,8 +1234,7 @@ static int __init mcdx_init_drive(int dr
 		release_region(stuffp->wreg_data, MCDX_IO_SIZE);
 		kfree(stuffp);
 		put_disk(disk);
-		if (unregister_blkdev(MAJOR_NR, "mcdx") != 0)
-			xwarn("cleanup() unregister_blkdev() failed\n");
+		unregister_blkdev(MAJOR_NR, "mcdx");
 		blk_cleanup_queue(mcdx_queue);
 		return 2;
 	}
Index: 2.6-mm/drivers/cdrom/sjcd.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/sjcd.c
+++ 2.6-mm/drivers/cdrom/sjcd.c
@@ -1792,9 +1792,9 @@ out2:
 out1:
 	blk_cleanup_queue(sjcd_queue);
 out0:
-	if ((unregister_blkdev(MAJOR_NR, "sjcd") == -EINVAL))
-		printk("SJCD: cannot unregister device.\n");
-	return (-EIO);
+	unregister_blkdev(MAJOR_NR, "sjcd");
+
+	return -EIO;
 }
 
 static void __exit sjcd_exit(void)
@@ -1803,8 +1803,7 @@ static void __exit sjcd_exit(void)
 	put_disk(sjcd_disk);
 	release_region(sjcd_base, 4);
 	blk_cleanup_queue(sjcd_queue);
-	if ((unregister_blkdev(MAJOR_NR, "sjcd") == -EINVAL))
-		printk("SJCD: cannot unregister device.\n");
+	unregister_blkdev(MAJOR_NR, "sjcd");
 	printk(KERN_INFO "SJCD: module: removed.\n");
 }
 
Index: 2.6-mm/drivers/md/dm.c
===================================================================
--- 2.6-mm.orig/drivers/md/dm.c
+++ 2.6-mm/drivers/md/dm.c
@@ -165,9 +165,7 @@ static void local_exit(void)
 {
 	kmem_cache_destroy(_tio_cache);
 	kmem_cache_destroy(_io_cache);
-
-	if (unregister_blkdev(_major, _name) < 0)
-		DMERR("unregister_blkdev failed");
+	unregister_blkdev(_major, _name);
 
 	_major = 0;
 
Index: 2.6-mm/drivers/block/loop.c
===================================================================
--- 2.6-mm.orig/drivers/block/loop.c
+++ 2.6-mm/drivers/block/loop.c
@@ -1536,8 +1536,7 @@ static void __exit loop_exit(void)
 		loop_del_one(lo);
 
 	blk_unregister_region(MKDEV(LOOP_MAJOR, 0), range);
-	if (unregister_blkdev(LOOP_MAJOR, "loop"))
-		printk(KERN_WARNING "loop: cannot unregister blkdev\n");
+	unregister_blkdev(LOOP_MAJOR, "loop");
 }
 
 module_init(loop_init);
Index: 2.6-mm/drivers/cdrom/cdu31a.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/cdu31a.c
+++ 2.6-mm/drivers/cdrom/cdu31a.c
@@ -3210,9 +3210,7 @@ errout0:
 	printk(KERN_ERR PFX "Unable to register with Uniform cdrom driver\n");
 	put_disk(disk);
 errout1:
-	if (unregister_blkdev(MAJOR_NR, "cdu31a")) {
-		printk(KERN_WARNING PFX "Can't unregister block device\n");
-	}
+	unregister_blkdev(MAJOR_NR, "cdu31a");
 errout2:
 	release_region(cdu31a_port, 4);
 errout3:
Index: 2.6-mm/drivers/cdrom/sbpcd.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/sbpcd.c
+++ 2.6-mm/drivers/cdrom/sbpcd.c
@@ -5830,10 +5830,7 @@ int __init sbpcd_init(void)
 		if (!p->sbp_buf) {
 			msg(DBG_INF,"data buffer (%d frames) not available.\n",
 				buffers);
-			if ((unregister_blkdev(MAJOR_NR, major_name) == -EINVAL))
-			{
-				printk("Can't unregister %s\n", major_name);
-			}
+			unregister_blkdev(MAJOR_NR, major_name);
 			release_region(CDo_command,4);
 			blk_cleanup_queue(sbpcd_queue);
 			return -EIO;
Index: 2.6-mm/drivers/cdrom/sonycd535.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/sonycd535.c
+++ 2.6-mm/drivers/cdrom/sonycd535.c
@@ -1675,10 +1675,8 @@ sony535_exit(void)
 	del_gendisk(cdu_disk);
 	put_disk(cdu_disk);
 	blk_cleanup_queue(sonycd535_queue);
-	if (unregister_blkdev(MAJOR_NR, CDU535_HANDLE) == -EINVAL)
-		printk("Uh oh, couldn't unregister " CDU535_HANDLE "\n");
-	else
-		printk(KERN_INFO CDU535_HANDLE " module released\n");
+	unregister_blkdev(MAJOR_NR, CDU535_HANDLE);
+	printk(KERN_INFO CDU535_HANDLE " module released\n");
 }
 
 module_init(sony535_init);

[PATCH 2/4 -mm] void unregister_blkdev - delete redundant message

[Akinobu Mita]

No need to warn unregister_blkdev() failure by caller.
(The previous patch makes unregister_blkdev() print error message in
error case)

Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

---
 drivers/block/xsysace.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Index: 2.6-mm/drivers/block/xsysace.c
===================================================================
--- 2.6-mm.orig/drivers/block/xsysace.c
+++ 2.6-mm/drivers/block/xsysace.c
@@ -1157,9 +1157,7 @@ static void __exit ace_exit(void)
 {
 	pr_debug("Unregistering Xilinx SystemACE driver\n");
 	driver_unregister(&ace_driver);
-	if (unregister_blkdev(ace_major, "xsysace"))
-		printk(KERN_WARNING "systemace unregister_blkdev(%i) failed\n",
-		       ace_major);
+	unregister_blkdev(ace_major, "xsysace");
 }
 
 module_init(ace_init);

[PATCH 3/4] void unregister_blkdev - ignore the return value

[Akinobu Mita]

Some cdrom drivers stop destruct operations in module_exit() when
unregister_blkdev() failure happens.

But it can't help to stop unloading module. So it will not be good
error handling. Furthermore any other block drivers don't have such
handling and there is no special reason that only those cdrom drivers
have to do.

This patch removes the return value checks for unregister_blkdev().

This change will not hide the bugs. Because unregister_blkdev() prints
error message on failures by the previous patch.

Cc: Eberhard Moenkeberg <emoenke@gwdg.de>
Cc: Oliver Raupach <raupach@nwfs1.rz.fh-hannover.de>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

---
 drivers/cdrom/aztcd.c  |    5 +----
 drivers/cdrom/cdu31a.c |    6 +-----
 drivers/cdrom/cm206.c  |    5 +----
 drivers/cdrom/gscd.c   |    5 +----
 drivers/cdrom/optcd.c  |    5 +----
 drivers/cdrom/sbpcd.c  |    8 ++------
 6 files changed, 7 insertions(+), 27 deletions(-)

Index: 2.6-mm/drivers/cdrom/optcd.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/optcd.c
+++ 2.6-mm/drivers/cdrom/optcd.c
@@ -2089,10 +2089,7 @@ static void __exit optcd_exit(void)
 {
 	del_gendisk(optcd_disk);
 	put_disk(optcd_disk);
-	if (unregister_blkdev(MAJOR_NR, "optcd") == -EINVAL) {
-		printk(KERN_ERR "optcd: what's that: can't unregister\n");
-		return;
-	}
+	unregister_blkdev(MAJOR_NR, "optcd");
 	blk_cleanup_queue(opt_queue);
 	release_region(optcd_port, 4);
 	printk(KERN_INFO "optcd: module released.\n");
Index: 2.6-mm/drivers/cdrom/sbpcd.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/sbpcd.c
+++ 2.6-mm/drivers/cdrom/sbpcd.c
@@ -5885,12 +5885,8 @@ int __init sbpcd_init(void)
 static void sbpcd_exit(void)
 {
 	int j;
-	
-	if ((unregister_blkdev(MAJOR_NR, major_name) == -EINVAL))
-	{
-		msg(DBG_INF, "What's that: can't unregister %s.\n", major_name);
-		return;
-	}
+
+	unregister_blkdev(MAJOR_NR, major_name);
 	release_region(CDo_command,4);
 	blk_cleanup_queue(sbpcd_queue);
 	for (j=0;j<NR_SBPCD;j++)
Index: 2.6-mm/drivers/cdrom/cdu31a.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/cdu31a.c
+++ 2.6-mm/drivers/cdrom/cdu31a.c
@@ -3227,11 +3227,7 @@ static void __exit cdu31a_exit(void)
 				"cdrom driver\n");
 		return;
 	}
-	if ((unregister_blkdev(MAJOR_NR, "cdu31a") == -EINVAL)) {
-		printk(KERN_WARNING PFX "Can't unregister\n");
-		return;
-	}
-
+	unregister_blkdev(MAJOR_NR, "cdu31a");
 	blk_cleanup_queue(cdu31a_queue);
 
 	if (cdu31a_irq > 0)
Index: 2.6-mm/drivers/cdrom/gscd.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/gscd.c
+++ 2.6-mm/drivers/cdrom/gscd.c
@@ -882,10 +882,7 @@ static void __exit gscd_exit(void)
 
 	del_gendisk(gscd_disk);
 	put_disk(gscd_disk);
-	if ((unregister_blkdev(MAJOR_NR, "gscd") == -EINVAL)) {
-		printk("What's that: can't unregister GoldStar-module\n");
-		return;
-	}
+	unregister_blkdev(MAJOR_NR, "gscd");
 	blk_cleanup_queue(gscd_queue);
 	release_region(gscd_port, GSCD_IO_EXTENT);
 	printk(KERN_INFO "GoldStar-module released.\n");
Index: 2.6-mm/drivers/cdrom/aztcd.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/aztcd.c
+++ 2.6-mm/drivers/cdrom/aztcd.c
@@ -1941,10 +1941,7 @@ static void __exit aztcd_exit(void)
 {
 	del_gendisk(azt_disk);
 	put_disk(azt_disk);
-	if ((unregister_blkdev(MAJOR_NR, "aztcd") == -EINVAL)) {
-		printk("What's that: can't unregister aztcd\n");
-		return;
-	}
+	unregister_blkdev(MAJOR_NR, "aztcd");
 	blk_cleanup_queue(azt_queue);
 	if ((azt_port == 0x1f0) || (azt_port == 0x170)) {
 		SWITCH_IDE_MASTER;
Index: 2.6-mm/drivers/cdrom/cm206.c
===================================================================
--- 2.6-mm.orig/drivers/cdrom/cm206.c
+++ 2.6-mm/drivers/cdrom/cm206.c
@@ -1548,10 +1548,7 @@ static void __exit cm206_exit(void)
 		printk("Can't unregister cdrom cm206\n");
 		return;
 	}
-	if (unregister_blkdev(MAJOR_NR, "cm206")) {
-		printk("Can't unregister major cm206\n");
-		return;
-	}
+	unregister_blkdev(MAJOR_NR, "cm206");
 	blk_cleanup_queue(cm206_queue);
 	free_irq(cm206_irq, NULL);
 	kfree(cd);

[PATCH 4/4] void unregister_blkdev - make void

[Akinobu Mita]

Put WARN_ON and fixed all callers of unregister_blkdev().
Now we can make unregister_blkdev return void.

Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

---
 block/genhd.c      |    7 +------
 include/linux/fs.h |    2 +-
 2 files changed, 2 insertions(+), 7 deletions(-)

Index: 2.6-mm/block/genhd.c
===================================================================
--- 2.6-mm.orig/block/genhd.c
+++ 2.6-mm/block/genhd.c
@@ -108,13 +108,11 @@ out:
 
 EXPORT_SYMBOL(register_blkdev);
 
-/* todo: make void - error printk here */
-int unregister_blkdev(unsigned int major, const char *name)
+void unregister_blkdev(unsigned int major, const char *name)
 {
 	struct blk_major_name **n;
 	struct blk_major_name *p = NULL;
 	int index = major_to_index(major);
-	int ret = 0;
 
 	mutex_lock(&block_subsys_lock);
 	for (n = &major_names[index]; *n; n = &(*n)->next)
@@ -122,15 +120,12 @@ int unregister_blkdev(unsigned int major
 			break;
 	if (!*n || strcmp((*n)->name, name)) {
 		WARN_ON(1);
-		ret = -EINVAL;
 	} else {
 		p = *n;
 		*n = p->next;
 	}
 	mutex_unlock(&block_subsys_lock);
 	kfree(p);
-
-	return ret;
 }
 
 EXPORT_SYMBOL(unregister_blkdev);
Index: 2.6-mm/include/linux/fs.h
===================================================================
--- 2.6-mm.orig/include/linux/fs.h
+++ 2.6-mm/include/linux/fs.h
@@ -1553,7 +1553,7 @@ extern void putname(const char *name);
 
 #ifdef CONFIG_BLOCK
 extern int register_blkdev(unsigned int, const char *);
-extern int unregister_blkdev(unsigned int, const char *);
+extern void unregister_blkdev(unsigned int, const char *);
 extern struct block_device *bdget(dev_t);
 extern void bd_set_size(struct block_device *, loff_t size);
 extern void bd_forget(struct inode *inode);

Re: [PATCH 2/4] void unregister_blkdev - delete redundant messages

[Grant Likely]

On 6/29/07, Akinobu Mita <akinobu.mita@gmail.com> wrote:
> No need to warn unregister_blkdev() failure by the callers.
> (The previous patch makes unregister_blkdev() print error message in
> error case)

Seeing as none of the callers check the error code; why does the
function return anything at all?

g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH 2/4 -mm] void unregister_blkdev - delete redundant message

[Grant Likely]

On 6/29/07, Akinobu Mita <akinobu.mita@gmail.com> wrote:
> No need to warn unregister_blkdev() failure by caller.
> (The previous patch makes unregister_blkdev() print error message in
> error case)
>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>

>
> ---
>  drivers/block/xsysace.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> Index: 2.6-mm/drivers/block/xsysace.c
> ===================================================================
> --- 2.6-mm.orig/drivers/block/xsysace.c
> +++ 2.6-mm/drivers/block/xsysace.c
> @@ -1157,9 +1157,7 @@ static void __exit ace_exit(void)
>  {
>         pr_debug("Unregistering Xilinx SystemACE driver\n");
>         driver_unregister(&ace_driver);
> -       if (unregister_blkdev(ace_major, "xsysace"))
> -               printk(KERN_WARNING "systemace unregister_blkdev(%i) failed\n",
> -                      ace_major);
> +       unregister_blkdev(ace_major, "xsysace");
>  }
>
>  module_init(ace_init);
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH 2/4] void unregister_blkdev - delete redundant messages

[Akinobu Mita]

2007/6/30, Grant Likely <grant.likely@secretlab.ca>:
> On 6/29/07, Akinobu Mita <akinobu.mita@gmail.com> wrote:
> > No need to warn unregister_blkdev() failure by the callers.
> > (The previous patch makes unregister_blkdev() print error message in
> > error case)
>
> Seeing as none of the callers check the error code; why does the
> function return anything at all?

Right. This series of patches is trying to make it return void.

http://lkml.org/lkml/2007/6/29/320

Re: [PATCH 2/4] void unregister_blkdev - delete redundant messages

[Grant Likely]

On 6/30/07, Akinobu Mita <akinobu.mita@gmail.com> wrote:
> > Seeing as none of the callers check the error code; why does the
> > function return anything at all?
>
> Right. This series of patches is trying to make it return void.
>
> http://lkml.org/lkml/2007/6/29/320

Heh, cool.  I should do more research before I reply blindly.  :-)

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195