dm: consolidate target deregistration error handling

Change dm_unregister_target to return void and use BUG() for error reporting. dm_unregister_target can only fail because of programming bug in the target driver. It can't fail because of user's behavior or disk errors. This patch changes unregister_target to return void and use BUG if someone tries to unregister non-registered target or unregister target that is in use. This patch removes code duplication (testing of error codes in all dm targets) and reports bugs in just one place, in dm_unregister_target. In some target drivers, these return codes were ignored, which could lead to a situation where bugs could be missed. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Alasdair G Kergon <agk@redhat.com>

dm: consolidate target deregistration error handling
Change dm_unregister_target to return void and use BUG() for error reporting. dm_unregister_target can only fail because of programming bug in the target driver. It can't fail because of user's behavior or disk errors. This patch changes unregister_target to return void and use BUG if someone tries to unregister non-registered target or unregister target that is in use. This patch removes code duplication (testing of error codes in all dm targets) and reports bugs in just one place, in dm_unregister_target. In some target drivers, these return codes were ignored, which could lead to a situation where bugs could be missed. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Alasdair G Kergon <agk@redhat.com>
Mikulas Patocka · Alasdair G Kergon
1 parent d460c65a6a
Showing 10 changed files with 20 additions and 50 deletions Side-by-side Diff
drivers/md/dm-crypt.c
drivers/md/dm-delay.c
drivers/md/dm-linear.c
drivers/md/dm-mpath.c
drivers/md/dm-raid1.c
drivers/md/dm-snap.c
drivers/md/dm-stripe.c
drivers/md/dm-target.c
drivers/md/dm-zero.c
include/linux/device-mapper.h
@@ -1322,11 +1322,7 @@
  
 static void __exit dm_crypt_exit(void)
 {
-	int r = dm_unregister_target(&crypt_target);
-
-	if (r < 0)
-		DMERR("unregister failed %d", r);
-
+	dm_unregister_target(&crypt_target);
 	kmem_cache_destroy(_crypt_io_pool);
 }
  
@@ -364,11 +364,7 @@
  
 static void __exit dm_delay_exit(void)
 {
-	int r = dm_unregister_target(&delay_target);
-
-	if (r < 0)
-		DMERR("unregister failed %d", r);
-
+	dm_unregister_target(&delay_target);
 	kmem_cache_destroy(delayed_cache);
 	destroy_workqueue(kdelayd_wq);
 }
@@ -156,9 +156,6 @@
  
 void dm_linear_exit(void)
 {
-	int r = dm_unregister_target(&linear_target);
-
-	if (r < 0)
-		DMERR("unregister failed %d", r);
+	dm_unregister_target(&linear_target);
 }
@@ -1495,14 +1495,10 @@
  
 static void __exit dm_multipath_exit(void)
 {
-	int r;
-
 	destroy_workqueue(kmpath_handlerd);
 	destroy_workqueue(kmultipathd);
  
-	r = dm_unregister_target(&multipath_target);
-	if (r < 0)
-		DMERR("target unregister failed %d", r);
+	dm_unregister_target(&multipath_target);
 	kmem_cache_destroy(_mpio_cache);
 }
  
@@ -1300,11 +1300,7 @@
  
 static void __exit dm_mirror_exit(void)
 {
-	int r;
-
-	r = dm_unregister_target(&mirror_target);
-	if (r < 0)
-		DMERR("unregister failed %d", r);
+	dm_unregister_target(&mirror_target);
 }
  
 /* Module hooks */
@@ -1470,17 +1470,10 @@
  
 static void __exit dm_snapshot_exit(void)
 {
-	int r;
-
 	destroy_workqueue(ksnapd);
  
-	r = dm_unregister_target(&snapshot_target);
-	if (r)
-		DMERR("snapshot unregister failed %d", r);
-
-	r = dm_unregister_target(&origin_target);
-	if (r)
-		DMERR("origin unregister failed %d", r);
+	dm_unregister_target(&snapshot_target);
+	dm_unregister_target(&origin_target);
  
 	exit_origin_hash();
 	kmem_cache_destroy(pending_cache);
@@ -337,9 +337,7 @@
  
 void dm_stripe_exit(void)
 {
-	if (dm_unregister_target(&stripe_target))
-		DMWARN("target unregistration failed");
-
+	dm_unregister_target(&stripe_target);
 	destroy_workqueue(kstriped);
  
 	return;
@@ -130,26 +130,26 @@
 	return rv;
 }
  
-int dm_unregister_target(struct target_type *t)
+void dm_unregister_target(struct target_type *t)
 {
 	struct tt_internal *ti;
  
 	down_write(&_lock);
 	if (!(ti = __find_target_type(t->name))) {
-		up_write(&_lock);
-		return -EINVAL;
+		DMCRIT("Unregistering unrecognised target: %s", t->name);
+		BUG();
 	}
  
 	if (ti->use) {
-		up_write(&_lock);
-		return -ETXTBSY;
+		DMCRIT("Attempt to unregister target still in use: %s",
+		       t->name);
+		BUG();
 	}
  
 	list_del(&ti->list);
 	kfree(ti);
  
 	up_write(&_lock);
-	return 0;
 }
  
 /*
@@ -187,8 +187,7 @@
  
 void dm_target_exit(void)
 {
-	if (dm_unregister_target(&error_target))
-		DMWARN("error target unregistration failed");
+	dm_unregister_target(&error_target);
 }
  
 EXPORT_SYMBOL(dm_register_target);
@@ -69,10 +69,7 @@
  
 static void __exit dm_zero_exit(void)
 {
-	int r = dm_unregister_target(&zero_target);
-
-	if (r < 0)
-		DMERR("unregister failed %d", r);
+	dm_unregister_target(&zero_target);
 }
  
 module_init(dm_zero_init)
@@ -157,9 +157,8 @@
 };
  
 int dm_register_target(struct target_type *t);
-int dm_unregister_target(struct target_type *t);
+void dm_unregister_target(struct target_type *t);
  
-
 /*-----------------------------------------------------------------
  * Functions for creating and manipulating mapped devices.
  * Drop the reference with dm_put when you finish with the object.
@@ -275,6 +274,9 @@
  * Macros.
  *---------------------------------------------------------------*/
 #define DM_NAME "device-mapper"
+
+#define DMCRIT(f, arg...) \
+	printk(KERN_CRIT DM_NAME ": " DM_MSG_PREFIX ": " f "\n", ## arg)
  
 #define DMERR(f, arg...) \
 	printk(KERN_ERR DM_NAME ": " DM_MSG_PREFIX ": " f "\n", ## arg)
...	...	@@ -1322,11 +1322,7 @@
1322	1322
1323	1323	static void __exit dm_crypt_exit(void)
1324	1324	{
1325		- int r = dm_unregister_target(&crypt_target);
1326		-
1327		- if (r < 0)
1328		- DMERR("unregister failed %d", r);
1329		-
	1325	+ dm_unregister_target(&crypt_target);
1330	1326	kmem_cache_destroy(_crypt_io_pool);
1331	1327	}
1332	1328
...	...	@@ -364,11 +364,7 @@
364	364
365	365	static void __exit dm_delay_exit(void)
366	366	{
367		- int r = dm_unregister_target(&delay_target);
368		-
369		- if (r < 0)
370		- DMERR("unregister failed %d", r);
371		-
	367	+ dm_unregister_target(&delay_target);
372	368	kmem_cache_destroy(delayed_cache);
373	369	destroy_workqueue(kdelayd_wq);
374	370	}
...	...	@@ -156,9 +156,6 @@
156	156
157	157	void dm_linear_exit(void)
158	158	{
159		- int r = dm_unregister_target(&linear_target);
160		-
161		- if (r < 0)
162		- DMERR("unregister failed %d", r);
	159	+ dm_unregister_target(&linear_target);
163	160	}
...	...	@@ -1495,14 +1495,10 @@
1495	1495
1496	1496	static void __exit dm_multipath_exit(void)
1497	1497	{
1498		- int r;
1499		-
1500	1498	destroy_workqueue(kmpath_handlerd);
1501	1499	destroy_workqueue(kmultipathd);
1502	1500
1503		- r = dm_unregister_target(&multipath_target);
1504		- if (r < 0)
1505		- DMERR("target unregister failed %d", r);
	1501	+ dm_unregister_target(&multipath_target);
1506	1502	kmem_cache_destroy(_mpio_cache);
1507	1503	}
1508	1504
...	...	@@ -1300,11 +1300,7 @@
1300	1300
1301	1301	static void __exit dm_mirror_exit(void)
1302	1302	{
1303		- int r;
1304		-
1305		- r = dm_unregister_target(&mirror_target);
1306		- if (r < 0)
1307		- DMERR("unregister failed %d", r);
	1303	+ dm_unregister_target(&mirror_target);
1308	1304	}
1309	1305
1310	1306	/* Module hooks */
...	...	@@ -1470,17 +1470,10 @@
1470	1470
1471	1471	static void __exit dm_snapshot_exit(void)
1472	1472	{
1473		- int r;
1474		-
1475	1473	destroy_workqueue(ksnapd);
1476	1474
1477		- r = dm_unregister_target(&snapshot_target);
1478		- if (r)
1479		- DMERR("snapshot unregister failed %d", r);
1480		-
1481		- r = dm_unregister_target(&origin_target);
1482		- if (r)
1483		- DMERR("origin unregister failed %d", r);
	1475	+ dm_unregister_target(&snapshot_target);
	1476	+ dm_unregister_target(&origin_target);
1484	1477
1485	1478	exit_origin_hash();
1486	1479	kmem_cache_destroy(pending_cache);
...	...	@@ -337,9 +337,7 @@
337	337
338	338	void dm_stripe_exit(void)
339	339	{
340		- if (dm_unregister_target(&stripe_target))
341		- DMWARN("target unregistration failed");
342		-
	340	+ dm_unregister_target(&stripe_target);
343	341	destroy_workqueue(kstriped);
344	342
345	343	return;
...	...	@@ -130,26 +130,26 @@
130	130	return rv;
131	131	}
132	132
133		-int dm_unregister_target(struct target_type *t)
	133	+void dm_unregister_target(struct target_type *t)
134	134	{
135	135	struct tt_internal *ti;
136	136
137	137	down_write(&_lock);
138	138	if (!(ti = __find_target_type(t->name))) {
139		- up_write(&_lock);
140		- return -EINVAL;
	139	+ DMCRIT("Unregistering unrecognised target: %s", t->name);
	140	+ BUG();
141	141	}
142	142
143	143	if (ti->use) {
144		- up_write(&_lock);
145		- return -ETXTBSY;
	144	+ DMCRIT("Attempt to unregister target still in use: %s",
	145	+ t->name);
	146	+ BUG();
146	147	}
147	148
148	149	list_del(&ti->list);
149	150	kfree(ti);
150	151
151	152	up_write(&_lock);
152		- return 0;
153	153	}
154	154
155	155	/*
...	...	@@ -187,8 +187,7 @@
187	187
188	188	void dm_target_exit(void)
189	189	{
190		- if (dm_unregister_target(&error_target))
191		- DMWARN("error target unregistration failed");
	190	+ dm_unregister_target(&error_target);
192	191	}
193	192
194	193	EXPORT_SYMBOL(dm_register_target);
...	...	@@ -69,10 +69,7 @@
69	69
70	70	static void __exit dm_zero_exit(void)
71	71	{
72		- int r = dm_unregister_target(&zero_target);
73		-
74		- if (r < 0)
75		- DMERR("unregister failed %d", r);
	72	+ dm_unregister_target(&zero_target);
76	73	}
77	74
78	75	module_init(dm_zero_init)
...	...	@@ -157,9 +157,8 @@
157	157	};
158	158
159	159	int dm_register_target(struct target_type *t);
160		-int dm_unregister_target(struct target_type *t);
	160	+void dm_unregister_target(struct target_type *t);
161	161
162		-
163	162	/*-----------------------------------------------------------------
164	163	* Functions for creating and manipulating mapped devices.
165	164	* Drop the reference with dm_put when you finish with the object.
...	...	@@ -275,6 +274,9 @@
275	274	* Macros.
276	275	---------------------------------------------------------------/
277	276	#define DM_NAME "device-mapper"
	277	+
	278	+#define DMCRIT(f, arg...) \
	279	+ printk(KERN_CRIT DM_NAME ": " DM_MSG_PREFIX ": " f "\n", ## arg)
278	280
279	281	#define DMERR(f, arg...) \
280	282	printk(KERN_ERR DM_NAME ": " DM_MSG_PREFIX ": " f "\n", ## arg)