From patchwork Wed Feb 17 21:49:13 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxim Levitsky X-Patchwork-Id: 45676 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 59CB6B7CFA for ; Thu, 18 Feb 2010 08:51:39 +1100 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1NhrmH-0005SH-Rn; Wed, 17 Feb 2010 21:49:49 +0000 Received: from mail-fx0-f209.google.com ([209.85.220.209]) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1Nhrm7-0005Mt-W5 for linux-mtd@lists.infradead.org; Wed, 17 Feb 2010 21:49:47 +0000 Received: by fxm1 with SMTP id 1so8377331fxm.4 for ; Wed, 17 Feb 2010 13:49:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:from:to:cc:subject:date :message-id:x-mailer:in-reply-to:references; bh=QQYM6dFbZZoLBiI8VA+Km/ZGOs2O/9hX5n0nhI7OkCc=; b=NIr6DEfXDvJz1RQoIZ5Qr5R+mCGmkywfQ6mHEgKqsCcyE4vWnSSyGZ6kKVttOMC5RD 9mpuAkh9avixEX/VJ3XVRJowbREKNk6SuEBcOXWClLGgmd39OQb6/14YpmA1FN3aGTNV jcKf3B8vh7SJqHDzVR/VRK+za8XFW++C7jASM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; b=DEgqwVBd5s1KouXWQwlbN31UKnXg6NAqcLkHS79gaLqYlaqBAyS/+Qwe2BvxJ3kVdO ZnBQLTStCpccBm004SSGhiqRTQ8FTGVImVT7d3dG1ZXv6jgAJEIjUbxw/3e0U3O4qRJU fw6jkZ1MFMKnmVzug7vn/h5CiutkW6g/ZJN9Q= Received: by 10.87.38.5 with SMTP id q5mr3494401fgj.45.1266443379051; Wed, 17 Feb 2010 13:49:39 -0800 (PST) Received: from localhost.localdomain ([77.126.218.27]) by mx.google.com with ESMTPS id d4sm3096749fga.8.2010.02.17.13.49.36 (version=SSLv3 cipher=RC4-MD5); Wed, 17 Feb 2010 13:49:38 -0800 (PST) From: Maxim Levitsky To: David Woodhouse Subject: [PATCH 03/14] blktrans: Hotplug fixes Date: Wed, 17 Feb 2010 23:49:13 +0200 Message-Id: <1266443364-4538-4-git-send-email-maximlevitsky@gmail.com> X-Mailer: git-send-email 1.6.3.3 In-Reply-To: <1266443364-4538-1-git-send-email-maximlevitsky@gmail.com> References: <1266443364-4538-1-git-send-email-maximlevitsky@gmail.com> X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20100217_164940_245434_AE2AB736 X-CRM114-Status: GOOD ( 26.24 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.2.5 on bombadil.infradead.org summary: Content analysis details: (0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- _SUMMARY_ Cc: Maxim Levitsky , Alex Dubov , Artem Bityutskiy , joern , Vitaly Wool , linux-kernel , "stanley.miao" , linux-mtd , Thomas Gleixner X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org * 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 --- 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 +#include 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;