Skip to contentSkip to conversation

0/5: Device-Mapper bug-fixes and cleanups

PATCH New Kevin Corry 1 participant 6 messages May 26, 2004 Device mapper

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 0/5: Device-Mapper bug-fixes and cleanups

Revision 1:
dm-ioctl.c: Fix an OB1 error when calculating an output buffer size, that could cause a missing null termininator in the 'list devices' ioctl results. [Steffan Paletta]

Revision 2:
In __map_bio(), if the target returns an error while mapping the I/O, the cloned bio needs to be freed.

Revision 3:
Replace dm_[add|remove]_wait_queue() with dm_wait_event().

Some testing of DM multipath has turned up a problem with the DEVICE_WAIT command. In the tests, while performing a DEVICE_WAIT on a multipath device, the command sometimes returns immediately, even though the event-number is correct and no path-failure has occurred to trigger an event. The problem was tracked down to the call to schedule() in dev_wait(), which would return even though it was not woken up by a DM table event.

This patch moves the responsibility for waiting from the ioctl interface into the core driver, and uses wait_event_interruptible() instead of relying on wait-queues and schedule().

Revision 4:
DM: Add static and __init qualifiers. [Dave Olien]

Revision 5:
dm-table.c: Proper usage of dm_vcalloc. [Dave Olien]

#1 lore raw

PATCH 1/1 1/5: dm-ioctl.c: fix off-by-one error

dm-ioctl.c: Fix an OB1 error when calculating an output buffer size, that could cause a missing null termininator in the 'list devices' ioctl results. [Steffan Paletta]

drivers/md/dm-ioctl.c +1 -1
@@ -377,7 +377,7 @@
 	for (i = 0; i < NUM_BUCKETS; i++) {
 		list_for_each_entry (hc, _name_buckets + i, name_list) {
 			needed += sizeof(struct dm_name_list);
-			needed += strlen(hc->name);
+			needed += strlen(hc->name) + 1;
 			needed += ALIGN_MASK;
 		}
 	}
#2 lore raw

PATCH 1/1 2/5: dm.c: free cloned bio on error path

In reply to Kevin Corry

In __map_bio(), if the target returns an error while mapping the I/O, the cloned bio needs to be freed.

drivers/md/dm.c +1 -0
@@ -369,6 +369,7 @@
 		struct dm_io *io = tio->io;
 		free_tio(tio->io->md, tio);
 		dec_pending(io, -EIO);
+		bio_put(clone);
 	}
 }
 
#3 lore raw

PATCH 1/1 5/5: dm-table.c: proper usage of dm_vcalloc

In reply to Kevin Corry

dm-table.c: Proper usage of dm_vcalloc. [Dave Olien]

drivers/md/dm-table.c +2 -2
@@ -181,8 +181,8 @@
 	/*
 	 * Allocate both the target array and offset array at once.
 	 */
-	n_highs = (sector_t *) dm_vcalloc(sizeof(struct dm_target) +
-					  sizeof(sector_t), num);
+	n_highs = (sector_t *) dm_vcalloc(num, sizeof(struct dm_target) +
+					  sizeof(sector_t));
 	if (!n_highs)
 		return -ENOMEM;
 
#4 lore raw

PATCH 1/1 4/5: dm: add static and __init qualifiers

In reply to Kevin Corry

DM: Add static and __init qualifiers. [Dave Olien]

drivers/md/dm-ioctl.c +7 -7
@@ -46,7 +46,7 @@
 static struct list_head _name_buckets[NUM_BUCKETS];
 static struct list_head _uuid_buckets[NUM_BUCKETS];
 
-void dm_hash_remove_all(void);
+static void dm_hash_remove_all(void);
 
 /*
  * Guards access to both hash tables.
@@ -61,7 +61,7 @@
 		INIT_LIST_HEAD(buckets + i);
 }
 
-int dm_hash_init(void)
+static int dm_hash_init(void)
 {
 	init_buckets(_name_buckets);
 	init_buckets(_uuid_buckets);
@@ -69,7 +69,7 @@
 	return 0;
 }
 
-void dm_hash_exit(void)
+static void dm_hash_exit(void)
 {
 	dm_hash_remove_all();
 	devfs_remove(DM_DIR);
@@ -195,7 +195,7 @@
  * The kdev_t and uuid of a device can never change once it is
  * initially inserted.
  */
-int dm_hash_insert(const char *name, const char *uuid, struct mapped_device *md)
+static int dm_hash_insert(const char *name, const char *uuid, struct mapped_device *md)
 {
 	struct hash_cell *cell;
 
@@ -234,7 +234,7 @@
 	return -EBUSY;
 }
 
-void __hash_remove(struct hash_cell *hc)
+static void __hash_remove(struct hash_cell *hc)
 {
 	/* remove from the dev hash */
 	list_del(&hc->uuid_list);
@@ -246,7 +246,7 @@
 	free_cell(hc);
 }
 
-void dm_hash_remove_all(void)
+static void dm_hash_remove_all(void)
 {
 	int i;
 	struct hash_cell *hc;
@@ -262,7 +262,7 @@
 	up_write(&_hash_lock);
 }
 
-int dm_hash_rename(const char *old, const char *new)
+static int dm_hash_rename(const char *old, const char *new)
 {
 	char *new_name, *old_name;
 	struct hash_cell *hc;
drivers/md/dm-target.c +2 -1
@@ -7,6 +7,7 @@
 #include "dm.h"
 
 #include <linux/module.h>
+#include <linux/init.h>
 #include <linux/kmod.h>
 #include <linux/bio.h>
 #include <linux/slab.h>
@@ -181,7 +182,7 @@
 	.map  = io_err_map,
 };
 
-int dm_target_init(void)
+int __init dm_target_init(void)
 {
 	return dm_register_target(&error_target);
 }
drivers/md/dm.c +4 -2
@@ -93,7 +93,7 @@
 static kmem_cache_t *_io_cache;
 static kmem_cache_t *_tio_cache;
 
-static __init int local_init(void)
+static int __init local_init(void)
 {
 	int r;
 
@@ -664,6 +664,8 @@
 	return r;
 }
 
+static struct block_device_operations dm_blk_dops;
+
 /*
  * Allocate and initialise a blank device with a given minor.
  */
@@ -1087,7 +1089,7 @@
 
 EXPORT_SYMBOL(dm_get_mapinfo);
 
-struct block_device_operations dm_blk_dops = {
+static struct block_device_operations dm_blk_dops = {
 	.open = dm_blk_open,
 	.release = dm_blk_close,
 	.owner = THIS_MODULE
drivers/md/dm.h +0 -2
@@ -31,8 +31,6 @@
 
 #define SECTOR_SHIFT 9
 
-extern struct block_device_operations dm_blk_dops;
-
 /*
  * List of devices that a metadevice uses and should open/close.
  */
#5 lore raw

PATCH 1/1 3/5: dm-ioctl: replace dm_[add|remove]_wait_queue() with dm_wait_event()

In reply to Kevin Corry

Replace dm_[add|remove]_wait_queue() with dm_wait_event().

Some testing of DM multipath has turned up a problem with the DEVICE_WAIT command. In the tests, while performing a DEVICE_WAIT on a multipath device, the command sometimes returns immediately, even though the event-number is correct and no path-failure has occurred to trigger an event. The problem was tracked down to the call to schedule() in dev_wait(), which would return even though it was not woken up by a DM table event.

This patch moves the responsibility for waiting from the ioctl interface into the core driver, and uses wait_event_interruptible() instead of relying on wait-queues and schedule().

drivers/md/dm-ioctl.c +3 -6
@@ -850,7 +850,6 @@
 	int r;
 	struct mapped_device *md;
 	struct dm_table *table;
-	DECLARE_WAITQUEUE(wq, current);
 
 	md = find_device(param);
 	if (!md)
@@ -859,12 +858,10 @@
 	/*
 	 * Wait for a notification event
 	 */
-	set_current_state(TASK_INTERRUPTIBLE);
-	if (!dm_add_wait_queue(md, &wq, param->event_nr)) {
-		schedule();
-		dm_remove_wait_queue(md, &wq);
+	if (dm_wait_event(md, param->event_nr)) {
+		r = -ERESTARTSYS;
+		goto out;
 	}
- 	set_current_state(TASK_RUNNING);
 
 	/*
 	 * The userland program is going to want to know what
drivers/md/dm.c +7 -30
@@ -80,7 +80,7 @@
 	/*
 	 * Event handling.
 	 */
-	uint32_t event_nr;
+	atomic_t event_nr;
 	wait_queue_head_t eventq;
 
 	/*
@@ -685,6 +685,7 @@
 	init_rwsem(&md->lock);
 	rwlock_init(&md->map_lock);
 	atomic_set(&md->holders, 1);
+	atomic_set(&md->event_nr, 0);
 
 	md->queue = blk_alloc_queue(GFP_KERNEL);
 	if (!md->queue)
@@ -754,10 +755,8 @@
 {
 	struct mapped_device *md = (struct mapped_device *) context;
 
-	down_write(&md->lock);
-	md->event_nr++;
+	atomic_inc(&md->event_nr);;
 	wake_up(&md->eventq);
-	up_write(&md->lock);
 }
 
 static void __set_size(struct gendisk *disk, sector_t size)
@@ -1055,35 +1054,13 @@
  *---------------------------------------------------------------*/
 uint32_t dm_get_event_nr(struct mapped_device *md)
 {
-	uint32_t r;
-
-	down_read(&md->lock);
-	r = md->event_nr;
-	up_read(&md->lock);
-
-	return r;
-}
-
-int dm_add_wait_queue(struct mapped_device *md, wait_queue_t *wq,
-		      uint32_t event_nr)
-{
-	down_write(&md->lock);
-	if (event_nr != md->event_nr) {
-		up_write(&md->lock);
-		return 1;
-	}
-
-	add_wait_queue(&md->eventq, wq);
-	up_write(&md->lock);
-
-	return 0;
+	return atomic_read(&md->event_nr);
 }
 
-void dm_remove_wait_queue(struct mapped_device *md, wait_queue_t *wq)
+int dm_wait_event(struct mapped_device *md, int event_nr)
 {
-	down_write(&md->lock);
-	remove_wait_queue(&md->eventq, wq);
-	up_write(&md->lock);
+	return wait_event_interruptible(md->eventq,
+			(event_nr != atomic_read(&md->event_nr)));
 }
 
 /*
drivers/md/dm.h +1 -3
@@ -81,9 +81,7 @@
  * Event functions.
  */
 uint32_t dm_get_event_nr(struct mapped_device *md);
-int dm_add_wait_queue(struct mapped_device *md, wait_queue_t *wq,
-		      uint32_t event_nr);
-void dm_remove_wait_queue(struct mapped_device *md, wait_queue_t *wq);
+int dm_wait_event(struct mapped_device *md, int event_nr);
 
 /*
  * Info functions.