xen/blkfront: fixes for 'xm block-detach ... --force'

Prevent prematurely freeing 'struct blkfront_info' instances (when the
xenbus data structures are gone, but the Linux ones are still needed).

Prevent adding a disk with the same (major, minor) [and hence the same
name and sysfs entries, which leads to oopses] when the previous
instance wasn't fully de-allocated yet.

This still doesn't address all issues resulting from forced detach:
I/O submitted after the detach still blocks forever, likely preventing
subsequent un-mounting from completing. It's not clear to me (not
knowing much about the block layer) how this can be avoided.

This also doesn't address issues with duplicate device creation caused
by re-using the hdXX and sdXX name spaces - this would require
synchronization with the respective native code.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff --git a/drivers/xen/blkfront/blkfront.c b/drivers/xen/blkfront/blkfront.c
index 78b8855a183df20d8c5478bddbfbc4be585f68ce..678a76090b5373410e1aaccb4cd76f986749a39f 100644 (file)
--- a/drivers/xen/blkfront/blkfront.c
+++ b/drivers/xen/blkfront/blkfront.c
@@ -418,7 +418,10 @@ static int blkfront_remove(struct xenbus_device *dev)
 
        blkif_free(info, 0);
 
-       kfree(info);
+       if(info->users == 0)
+               kfree(info);
+       else
+               info->is_ready = -1;
 
        return 0;
 }
@@ -480,6 +483,9 @@ static void blkif_restart_queue_callback(void *arg)
 int blkif_open(struct inode *inode, struct file *filep)
 {
        struct blkfront_info *info = inode->i_bdev->bd_disk->private_data;
+
+       if(info->is_ready < 0)
+               return -ENODEV;
        info->users++;
        return 0;
 }
@@ -496,7 +502,10 @@ int blkif_release(struct inode *inode, struct file *filep)
                struct xenbus_device * dev = info->xbdev;
                enum xenbus_state state = xenbus_read_driver_state(dev->otherend);
 
-               if (state == XenbusStateClosing && info->is_ready)
+               if(info->is_ready < 0) {
+                       blkfront_closing(dev);
+                       kfree(info);
+               } else if (state == XenbusStateClosing && info->is_ready)
                        blkfront_closing(dev);
        }
        return 0;
@@ -887,7 +896,7 @@ int blkfront_is_ready(struct xenbus_device *dev)
 {
        struct blkfront_info *info = dev->dev.driver_data;
 
-       return info->is_ready;
+       return info->is_ready > 0;
 }
 
 

diff --git a/drivers/xen/blkfront/block.h b/drivers/xen/blkfront/block.h
index 3168653ddf3509ac59ab1719a7ba91aefc42d31a..a50be77e46db08145ad57a8d1f4220eeb8cda39c 100644 (file)
--- a/drivers/xen/blkfront/block.h
+++ b/drivers/xen/blkfront/block.h
@@ -78,6 +78,7 @@ struct xlbd_major_info
        int index;
        int usage;
        struct xlbd_type_info *type;
+       struct xlbd_minor_state *minors;
 };
 
 struct blk_shadow {

diff --git a/drivers/xen/blkfront/vbd.c b/drivers/xen/blkfront/vbd.c
index 4c5a93df8a3cad8469cbf52907d948b0695d77c3..f4ecf15d7ee0d64e41482316f66526f86f86f586 100644 (file)
--- a/drivers/xen/blkfront/vbd.c
+++ b/drivers/xen/blkfront/vbd.c
@@ -48,6 +48,12 @@
 #define VDEV_IS_EXTENDED(dev) ((dev)&(EXTENDED))
 #define BLKIF_MINOR_EXT(dev) ((dev)&(~EXTENDED))
 
+struct xlbd_minor_state {
+       unsigned int nr;
+       unsigned long *bitmap;
+       spinlock_t lock;
+};
+
 /*
  * For convenience we distinguish between ide, scsi and 'other' (i.e.,
  * potentially combinations of the two) in the naming scheme and in a few other
@@ -97,6 +103,8 @@ static struct xlbd_major_info *major_info[NUM_IDE_MAJORS + NUM_SCSI_MAJORS +
 #define XLBD_MAJOR_SCSI_RANGE  XLBD_MAJOR_SCSI_START ... XLBD_MAJOR_VBD_START - 1
 #define XLBD_MAJOR_VBD_RANGE   XLBD_MAJOR_VBD_START ... XLBD_MAJOR_VBD_START + NUM_VBD_MAJORS - 1
 
+#define XLBD_MAJOR_VBD_ALT(idx) ((idx) ^ XLBD_MAJOR_VBD_START ^ (XLBD_MAJOR_VBD_START + 1))
+
 static struct block_device_operations xlvbd_block_fops =
 {
        .owner = THIS_MODULE,
@@ -114,6 +122,7 @@ static struct xlbd_major_info *
 xlbd_alloc_major_info(int major, int minor, int index)
 {
        struct xlbd_major_info *ptr;
+       struct xlbd_minor_state *minors;
        int do_register;
 
        ptr = kzalloc(sizeof(struct xlbd_major_info), GFP_KERNEL);
@@ -121,6 +130,22 @@ xlbd_alloc_major_info(int major, int minor, int index)
                return NULL;
 
        ptr->major = major;
+       minors = kmalloc(sizeof(*minors), GFP_KERNEL);
+       if (minors == NULL) {
+               kfree(ptr);
+               return NULL;
+       }
+
+       minors->bitmap = kzalloc(BITS_TO_LONGS(256) * sizeof(*minors->bitmap),
+                                GFP_KERNEL);
+       if (minors->bitmap == NULL) {
+               kfree(minors);
+               kfree(ptr);
+               return NULL;
+       }
+
+       spin_lock_init(&minors->lock);
+       minors->nr = 256;
        do_register = 1;
 
        switch (index) {
@@ -143,13 +168,19 @@ xlbd_alloc_major_info(int major, int minor, int index)
                 * if someone already registered block major 202,
                 * don't try to register it again
                 */
-               if (major_info[XLBD_MAJOR_VBD_START] != NULL)
+               if (major_info[XLBD_MAJOR_VBD_ALT(index)] != NULL) {
+                       kfree(minors->bitmap);
+                       kfree(minors);
+                       minors = major_info[XLBD_MAJOR_VBD_ALT(index)]->minors;
                        do_register = 0;
+               }
                break;
        }
 
        if (do_register) {
                if (register_blkdev(ptr->major, ptr->type->devname)) {
+                       kfree(minors->bitmap);
+                       kfree(minors);
                        kfree(ptr);
                        return NULL;
                }
@@ -157,6 +188,7 @@ xlbd_alloc_major_info(int major, int minor, int index)
                printk("xen-vbd: registered block device major %i\n", ptr->major);
        }
 
+       ptr->minors = minors;
        major_info[index] = ptr;
        return ptr;
 }
@@ -208,6 +240,61 @@ xlbd_put_major_info(struct xlbd_major_info *mi)
        /* XXX: release major if 0 */
 }
 
+static int
+xlbd_reserve_minors(struct xlbd_major_info *mi, unsigned int minor,
+                   unsigned int nr_minors)
+{
+       struct xlbd_minor_state *ms = mi->minors;
+       unsigned int end = minor + nr_minors;
+       int rc;
+
+       if (end > ms->nr) {
+               unsigned long *bitmap, *old;
+
+               bitmap = kzalloc(BITS_TO_LONGS(end) * sizeof(*bitmap),
+                                GFP_KERNEL);
+               if (bitmap == NULL)
+                       return -ENOMEM;
+
+               spin_lock(&ms->lock);
+               if (end > ms->nr) {
+                       old = ms->bitmap;
+                       memcpy(bitmap, ms->bitmap,
+                              BITS_TO_LONGS(ms->nr) * sizeof(*bitmap));
+                       ms->bitmap = bitmap;
+                       ms->nr = BITS_TO_LONGS(end) * BITS_PER_LONG;
+               } else
+                       old = bitmap;
+               spin_unlock(&ms->lock);
+               kfree(old);
+       }
+
+       spin_lock(&ms->lock);
+       if (find_next_bit(ms->bitmap, end, minor) >= end) {
+               for (; minor < end; ++minor)
+                       __set_bit(minor, ms->bitmap);
+               rc = 0;
+       } else
+               rc = -EBUSY;
+       spin_unlock(&ms->lock);
+
+       return rc;
+}
+
+static void
+xlbd_release_minors(struct xlbd_major_info *mi, unsigned int minor,
+                   unsigned int nr_minors)
+{
+       struct xlbd_minor_state *ms = mi->minors;
+       unsigned int end = minor + nr_minors;
+
+       BUG_ON(end > ms->nr);
+       spin_lock(&ms->lock);
+       for (; minor < end; ++minor)
+               __clear_bit(minor, ms->bitmap);
+       spin_unlock(&ms->lock);
+}
+
 static int
 xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size)
 {
@@ -263,9 +350,14 @@ xlvbd_alloc_gendisk(int major, int minor, blkif_sector_t capacity, int vdevice,
        if ((minor & ((1 << mi->type->partn_shift) - 1)) == 0)
                nr_minors = 1 << mi->type->partn_shift;
 
+       err = xlbd_reserve_minors(mi, minor, nr_minors);
+       if (err)
+               goto out;
+       err = -ENODEV;
+
        gd = alloc_disk(nr_minors);
        if (gd == NULL)
-               goto out;
+               goto release;
 
        offset =  mi->index * mi->type->disks_per_major +
                        (minor >> mi->type->partn_shift);
@@ -304,7 +396,7 @@ xlvbd_alloc_gendisk(int major, int minor, blkif_sector_t capacity, int vdevice,
 
        if (xlvbd_init_blk_queue(gd, sector_size)) {
                del_gendisk(gd);
-               goto out;
+               goto release;
        }
 
        info->rq = gd->queue;
@@ -324,6 +416,8 @@ xlvbd_alloc_gendisk(int major, int minor, blkif_sector_t capacity, int vdevice,
 
        return 0;
 
+ release:
+       xlbd_release_minors(mi, minor, nr_minors);
  out:
        if (mi)
                xlbd_put_major_info(mi);
@@ -369,14 +463,19 @@ xlvbd_add(blkif_sector_t capacity, int vdevice, u16 vdisk_info,
 void
 xlvbd_del(struct blkfront_info *info)
 {
+       unsigned int minor, nr_minors;
+
        if (info->mi == NULL)
                return;
 
        BUG_ON(info->gd == NULL);
+       minor = info->gd->first_minor;
+       nr_minors = info->gd->minors;
        del_gendisk(info->gd);
        put_disk(info->gd);
        info->gd = NULL;
 
+       xlbd_release_minors(info->mi, minor, nr_minors);
        xlbd_put_major_info(info->mi);
        info->mi = NULL;