0/5: Device-Mapper bug-fixes and cleanups
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]
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 ; } }
PATCH 1/1 2/5: dm.c: free cloned bio on error path
In reply to Kevin CorryIn __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); } }
PATCH 1/1 5/5: dm-table.c: proper usage of dm_vcalloc
In reply to Kevin Corrydm-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 ;
PATCH 1/1 4/5: dm: add static and __init qualifiers
In reply to Kevin CorryDM: 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. */
PATCH 1/1 3/5: dm-ioctl: replace dm_[add|remove]_wait_queue() with dm_wait_event()
In reply to Kevin CorryReplace 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.