lkml.org 
[lkml]   [2010]   [Feb]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 03/14] blktrans: Hotplug fixes
    Date
    * Add locking where it was missing.
    * Don't do a get_mtd_device in blktrans_open because it would lead to a deadlock
    instead do that in add_mtd_blktrans_dev.

    * Only free the mtd_blktrans_dev structure when the last user exits.

    * Flush request queue on device removal.

    * Track users, and call tr->release in del_mtd_blktrans_dev
    Due to that ->open and release aren't called more that once.

    Now it is safe to call del_mtd_blktrans_dev while the device is still in use.

    Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
    ---
    drivers/mtd/ftl.c | 1 -
    drivers/mtd/inftlcore.c | 1 -
    drivers/mtd/mtd_blkdevs.c | 183 ++++++++++++++++++++++++++++++------------
    drivers/mtd/mtdblock.c | 1 -
    drivers/mtd/mtdblock_ro.c | 1 -
    drivers/mtd/nftlcore.c | 1 -
    drivers/mtd/rfd_ftl.c | 1 -
    drivers/mtd/ssfdc.c | 1 -
    include/linux/mtd/blktrans.h | 3 +
    9 files changed, 134 insertions(+), 59 deletions(-)

    diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
    index e56d6b4..62da9eb 100644
    --- a/drivers/mtd/ftl.c
    +++ b/drivers/mtd/ftl.c
    @@ -1082,7 +1082,6 @@ static void ftl_remove_dev(struct mtd_blktrans_dev *dev)
    {
    del_mtd_blktrans_dev(dev);
    ftl_freepart((partition_t *)dev);
    - kfree(dev);
    }

    static struct mtd_blktrans_ops ftl_tr = {
    diff --git a/drivers/mtd/inftlcore.c b/drivers/mtd/inftlcore.c
    index 8aca552..015a7fe 100755
    --- a/drivers/mtd/inftlcore.c
    +++ b/drivers/mtd/inftlcore.c
    @@ -139,7 +139,6 @@ static void inftl_remove_dev(struct mtd_blktrans_dev *dev)

    kfree(inftl->PUtable);
    kfree(inftl->VUtable);
    - kfree(inftl);
    }

    /*
    diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
    index bbdb26d..1bc9c76 100644
    --- a/drivers/mtd/mtd_blkdevs.c
    +++ b/drivers/mtd/mtd_blkdevs.c
    @@ -24,6 +24,39 @@
    #include "mtdcore.h"

    static LIST_HEAD(blktrans_majors);
    +static DEFINE_MUTEX(blktrans_ref_mutex);
    +
    +void blktrans_dev_release(struct kref *kref)
    +{
    + struct mtd_blktrans_dev *dev =
    + container_of(kref, struct mtd_blktrans_dev, ref);
    +
    + dev->disk->private_data = NULL;
    + put_disk(dev->disk);
    + kfree(dev);
    +}
    +
    +static struct mtd_blktrans_dev *blktrans_dev_get(struct gendisk *disk)
    +{
    + struct mtd_blktrans_dev *dev;
    +
    + mutex_lock(&blktrans_ref_mutex);
    + dev = disk->private_data;
    +
    + if (!dev)
    + goto unlock;
    + kref_get(&dev->ref);
    +unlock:
    + mutex_unlock(&blktrans_ref_mutex);
    + return dev;
    +}
    +
    +void blktrans_dev_put(struct mtd_blktrans_dev *dev)
    +{
    + mutex_lock(&blktrans_ref_mutex);
    + kref_put(&dev->ref, blktrans_dev_release);
    + mutex_unlock(&blktrans_ref_mutex);
    +}


    static int do_blktrans_request(struct mtd_blktrans_ops *tr,
    @@ -111,81 +144,102 @@ static int mtd_blktrans_thread(void *arg)

    static void mtd_blktrans_request(struct request_queue *rq)
    {
    - struct mtd_blktrans_dev *dev = rq->queuedata;
    - wake_up_process(dev->thread);
    -}
    + struct mtd_blktrans_dev *dev;
    + struct request *req = NULL;
    +
    + dev = rq->queuedata;

    + if (!dev)
    + while ((req = blk_fetch_request(rq)) != NULL)
    + __blk_end_request_all(req, -ENODEV);
    + else
    + wake_up_process(dev->thread);
    +}

    static int blktrans_open(struct block_device *bdev, fmode_t mode)
    {
    - struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data;
    - struct mtd_blktrans_ops *tr = dev->tr;
    - int ret = -ENODEV;
    -
    - if (!get_mtd_device(NULL, dev->mtd->index))
    - goto out;
    -
    - if (!try_module_get(tr->owner))
    - goto out_tr;
    -
    - /* FIXME: Locking. A hot pluggable device can go away
    - (del_mtd_device can be called for it) without its module
    - being unloaded. */
    - dev->mtd->usecount++;
    -
    - ret = 0;
    - if (tr->open && (ret = tr->open(dev))) {
    - dev->mtd->usecount--;
    - put_mtd_device(dev->mtd);
    - out_tr:
    - module_put(tr->owner);
    - }
    - out:
    + struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk);
    + int ret = -ENXIO;
    +
    + if (!dev)
    + return ret;
    +
    + mutex_lock(&dev->lock);
    +
    + if (!dev->mtd)
    + goto unlock;
    +
    + ret = !dev->open++ && dev->tr->open ? dev->tr->open(dev) : 0;
    +unlock:
    + mutex_unlock(&dev->lock);
    + blktrans_dev_put(dev);
    return ret;
    }

    static int blktrans_release(struct gendisk *disk, fmode_t mode)
    {
    - struct mtd_blktrans_dev *dev = disk->private_data;
    - struct mtd_blktrans_ops *tr = dev->tr;
    - int ret = 0;
    + struct mtd_blktrans_dev *dev = blktrans_dev_get(disk);
    + int ret = -ENXIO;
    +
    + if (!dev)
    + return ret;

    - if (tr->release)
    - ret = tr->release(dev);
    + mutex_lock(&dev->lock);

    - if (!ret) {
    - dev->mtd->usecount--;
    - put_mtd_device(dev->mtd);
    - module_put(tr->owner);
    - }
    + if (!dev->mtd)
    + goto unlock;

    + ret = !--dev->open && dev->tr->release ? dev->tr->release(dev) : 0;
    +unlock:
    + mutex_unlock(&dev->lock);
    + blktrans_dev_put(dev);
    return ret;
    }

    static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
    {
    - struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data;
    + struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk);
    + int ret = -ENXIO;
    +
    + if (!dev)
    + return ret;
    +
    + mutex_lock(&dev->lock);
    +
    + if (!dev->mtd)
    + goto unlock;

    - if (dev->tr->getgeo)
    - return dev->tr->getgeo(dev, geo);
    - return -ENOTTY;
    + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
    +unlock:
    + mutex_unlock(&dev->lock);
    + blktrans_dev_put(dev);
    + return ret;
    }

    static int blktrans_ioctl(struct block_device *bdev, fmode_t mode,
    unsigned int cmd, unsigned long arg)
    {
    - struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data;
    - struct mtd_blktrans_ops *tr = dev->tr;
    + struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk);
    + int ret = -ENXIO;
    +
    + if (!dev)
    + return ret;
    +
    + mutex_lock(&dev->lock);
    +
    + if (!dev->mtd)
    + goto unlock;

    switch (cmd) {
    case BLKFLSBUF:
    - if (tr->flush)
    - return tr->flush(dev);
    - /* The core code did the work, we had nothing to do. */
    - return 0;
    + ret = dev->tr->flush ? dev->tr->flush(dev) : 0;
    default:
    - return -ENOTTY;
    + ret = -ENOTTY;
    }
    +unlock:
    + mutex_unlock(&dev->lock);
    + blktrans_dev_put(dev);
    + return ret;
    }

    static const struct block_device_operations mtd_blktrans_ops = {
    @@ -239,10 +293,10 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
    list_add_tail(&new->list, &tr->devs);
    added:
    mutex_init(&new->lock);
    + kref_init(&new->ref);
    if (!tr->writesect)
    new->readonly = 1;

    -
    /* Create gendisk */
    ret = -ENOMEM;
    gd = alloc_disk(1 << tr->part_bits);
    @@ -271,7 +325,6 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)

    set_capacity(gd, (new->size * tr->blksize) >> 9);

    -
    /* Create the request queue */
    spin_lock_init(&new->queue_lock);
    new->rq = blk_init_queue(mtd_blktrans_request, &new->queue_lock);
    @@ -288,6 +341,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)

    gd->queue = new->rq;

    + __get_mtd_device(new->mtd);
    + __module_get(tr->owner);
    +
    /* Create processing thread */
    /* TODO: workqueue ? */
    new->thread = kthread_run(mtd_blktrans_thread, new,
    @@ -305,6 +361,8 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)

    return 0;
    error4:
    + module_put(tr->owner);
    + __put_mtd_device(new->mtd);
    blk_cleanup_queue(new->rq);
    error3:
    put_disk(new->disk);
    @@ -317,6 +375,8 @@ error1:

    int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
    {
    + unsigned long flags;
    +
    if (mutex_trylock(&mtd_table_mutex)) {
    mutex_unlock(&mtd_table_mutex);
    BUG();
    @@ -324,13 +384,34 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)

    list_del(&old->list);

    - /* stop new requests to arrive */
    + /* Stop new requests to arrive */
    del_gendisk(old->disk);

    /* Stop the thread */
    kthread_stop(old->thread);

    + /* Kill current requests */
    + spin_lock_irqsave(&old->queue_lock, flags);
    + old->rq->queuedata = NULL;
    + blk_start_queue(old->rq);
    + spin_unlock_irqrestore(&old->queue_lock, flags);
    blk_cleanup_queue(old->rq);
    +
    + /* Ask trans driver for release to the mtd device */
    + mutex_lock(&old->lock);
    + if (old->open && old->tr->release) {
    + old->tr->release(old);
    + old->open = 0;
    + }
    +
    + __put_mtd_device(old->mtd);
    + module_put(old->tr->owner);
    +
    + /* At that point, we don't touch the mtd anymore */
    + old->mtd = NULL;
    +
    + mutex_unlock(&old->lock);
    + blktrans_dev_put(old);
    return 0;
    }

    @@ -393,7 +474,6 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
    }

    mutex_unlock(&mtd_table_mutex);
    -
    return 0;
    }

    @@ -403,7 +483,6 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)

    mutex_lock(&mtd_table_mutex);

    -
    /* Remove it from the list of active majors */
    list_del(&tr->list);

    diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
    index 9f41b1a..d8322cc 100644
    --- a/drivers/mtd/mtdblock.c
    +++ b/drivers/mtd/mtdblock.c
    @@ -368,7 +368,6 @@ static void mtdblock_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
    static void mtdblock_remove_dev(struct mtd_blktrans_dev *dev)
    {
    del_mtd_blktrans_dev(dev);
    - kfree(dev);
    }

    static struct mtd_blktrans_ops mtdblock_tr = {
    diff --git a/drivers/mtd/mtdblock_ro.c b/drivers/mtd/mtdblock_ro.c
    index 852165f..54ff288 100644
    --- a/drivers/mtd/mtdblock_ro.c
    +++ b/drivers/mtd/mtdblock_ro.c
    @@ -49,7 +49,6 @@ static void mtdblock_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
    static void mtdblock_remove_dev(struct mtd_blktrans_dev *dev)
    {
    del_mtd_blktrans_dev(dev);
    - kfree(dev);
    }

    static struct mtd_blktrans_ops mtdblock_tr = {
    diff --git a/drivers/mtd/nftlcore.c b/drivers/mtd/nftlcore.c
    index 1002e18..a4578bf 100644
    --- a/drivers/mtd/nftlcore.c
    +++ b/drivers/mtd/nftlcore.c
    @@ -126,7 +126,6 @@ static void nftl_remove_dev(struct mtd_blktrans_dev *dev)
    del_mtd_blktrans_dev(dev);
    kfree(nftl->ReplUnitTable);
    kfree(nftl->EUNtable);
    - kfree(nftl);
    }

    /*
    diff --git a/drivers/mtd/rfd_ftl.c b/drivers/mtd/rfd_ftl.c
    index d2aa9c4..63b83c0 100644
    --- a/drivers/mtd/rfd_ftl.c
    +++ b/drivers/mtd/rfd_ftl.c
    @@ -817,7 +817,6 @@ static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev)
    vfree(part->sector_map);
    kfree(part->header_cache);
    kfree(part->blocks);
    - kfree(part);
    }

    static struct mtd_blktrans_ops rfd_ftl_tr = {
    diff --git a/drivers/mtd/ssfdc.c b/drivers/mtd/ssfdc.c
    index 3f67e00..81c4ecd 100644
    --- a/drivers/mtd/ssfdc.c
    +++ b/drivers/mtd/ssfdc.c
    @@ -375,7 +375,6 @@ static void ssfdcr_remove_dev(struct mtd_blktrans_dev *dev)

    del_mtd_blktrans_dev(dev);
    kfree(ssfdc->logic_block_map);
    - kfree(ssfdc);
    }

    static int ssfdcr_readsect(struct mtd_blktrans_dev *dev,
    diff --git a/include/linux/mtd/blktrans.h b/include/linux/mtd/blktrans.h
    index a4b3928..d89b8fb 100644
    --- a/include/linux/mtd/blktrans.h
    +++ b/include/linux/mtd/blktrans.h
    @@ -9,6 +9,7 @@
    #define __MTD_TRANS_H__

    #include <linux/mutex.h>
    +#include <linux/kref.h>

    struct hd_geometry;
    struct mtd_info;
    @@ -24,6 +25,8 @@ struct mtd_blktrans_dev {
    int devnum;
    unsigned long size;
    int readonly;
    + int open;
    + struct kref ref;
    struct gendisk *disk;
    struct task_struct *thread;
    struct request_queue *rq;
    --
    1.6.3.3


    \
     
     \ /
      Last update: 2010-02-13 14:05    [W:0.038 / U:31.188 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site