diff mbox

[RFC] MMC: Request for comments attempt at dealing with removeable suspend/resume.

Message ID 1301949069-13283-1-git-send-email-andreiw@motorola.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrei Warkentin April 4, 2011, 8:31 p.m. UTC
Is there any value to doing something like this in order to be able to suspend/resume
with a (manually, or rootfs) mounted filesystem on mmcblk?

Thoughts?

Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |   76 +++++++++++++++++++++++++++++++++++++++++----
 drivers/mmc/core/core.c  |    3 +-
 2 files changed, 70 insertions(+), 9 deletions(-)

Comments

Chuanxiao.Dong May 17, 2011, 10:15 a.m. UTC | #1
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Andrei Warkentin
> Sent: Tuesday, April 05, 2011 4:31 AM
> To: linux-mmc@vger.kernel.org
> Cc: Andrei Warkentin
> Subject: [RFC] MMC: Request for comments attempt at dealing with removeable
> suspend/resume.
> 
> Is there any value to doing something like this in order to be able to
> suspend/resume
> with a (manually, or rootfs) mounted filesystem on mmcblk?
> 
> Thoughts?
Hi Andrei,
I also encountered the same issue with you when system is suspending. And the modification in core.c actually can fix my issue.
For the mounted root file system, I think it makes sense for such medias to use MMC_UNSAFE_RESUME. Is there some use case need to remove root file system media?

> 
> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
> ---
>  drivers/mmc/card/block.c |   76
> +++++++++++++++++++++++++++++++++++++++++----
>  drivers/mmc/core/core.c  |    3 +-
>  2 files changed, 70 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index ee8f7a9..19eb5b6 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -53,6 +53,9 @@ MODULE_ALIAS("mmc:block");
>       ((card)->ext_csd.rel_sectors)))
> 
>  static DEFINE_MUTEX(block_mutex);
> +static DEFINE_MUTEX(orphan_mutex);
> +
> +struct list_head orphans = LIST_HEAD_INIT(orphans);
> 
>  /*
>   * The defaults come from config options but can be overriden by module
> @@ -77,6 +80,7 @@ struct mmc_blk_data {
>  	struct gendisk	*disk;
>  	struct mmc_queue queue;
>  	struct list_head part;
> +	struct list_head orphan;
> 
>  	unsigned int	usage;
>  	unsigned int	read_only;
> @@ -88,6 +92,7 @@ struct mmc_blk_data {
>  	 * track of the current selected device partition.
>  	 */
>  	unsigned int	part_curr;
> +	u32		raw_cid[4];
>  	struct device_attribute force_ro;
>  };
> 
> @@ -126,10 +131,12 @@ static void mmc_blk_put(struct mmc_blk_data *md)
>  	mutex_lock(&open_lock);
>  	md->usage--;
>  	if (md->usage == 0) {
> -		int devidx = mmc_get_devidx(md->disk);
> -		blk_cleanup_queue(md->queue.queue);
> +		mutex_lock(&orphan_mutex);
> +		list_del(&md->orphan);
> +		mutex_unlock(&orphan_mutex);
> 
> -		__clear_bit(devidx, dev_use);
> +		blk_cleanup_queue(md->queue.queue);
> +		__clear_bit(mmc_get_devidx(md->disk), dev_use);
> 
>  		put_disk(md->disk);
>  		kfree(md);
> @@ -718,6 +725,49 @@ static inline int mmc_blk_readonly(struct mmc_card
> *card)
>  	       !(card->csd.cmdclass & CCC_BLOCK_WRITE);
>  }
> 
> +static inline struct mmc_blk_data *mmc_lookup_orphan(struct mmc_card *card,
> +						     struct device *parent,
> +						     unsigned int part_type,
> +						     sector_t size)
> +{
> +	int ret;
> +	struct list_head *pos, *q;
> +	struct mmc_blk_data *md;
> +	bool found = false;
> +
> +	mutex_lock(&orphan_mutex);
> +	list_for_each_safe(pos, q, &orphans) {
> +		md = list_entry(pos, struct mmc_blk_data, orphan);
> +		if (!memcmp(md->raw_cid, card->raw_cid, sizeof(md->raw_cid)) &&
> +		    md->part_type == part_type) {
> +			list_del(pos);
> +			found = true;
> +			mmc_blk_get(md->disk);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&orphan_mutex);
> +
> +	if (!found)
> +		return NULL;
> +
> +	ret = mmc_init_queue(&md->queue, card, &md->lock);
> +	if (ret)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&md->part);
> +	md->disk->driverfs_dev = parent;
> +	md->queue.issue_fn = mmc_blk_issue_rq;
> +	md->queue.data = md;
> +	md->disk->queue = md->queue.queue;
> +	if (REL_WRITES_SUPPORTED(card))
> +		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
> +	blk_queue_logical_block_size(md->queue.queue, 512);
> +	set_capacity(md->disk, size);
> +	printk("set cap to %x\n", (unsigned int) get_capacity(md->disk));
> +	return md;
> +}
> +
>  static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>  					      struct device *parent,
>  					      sector_t size,
> @@ -752,7 +802,9 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct
> mmc_card *card,
> 
>  	spin_lock_init(&md->lock);
>  	INIT_LIST_HEAD(&md->part);
> +	INIT_LIST_HEAD(&md->orphan);
>  	md->usage = 1;
> +	memcpy(md->raw_cid, card->raw_cid, sizeof(card->raw_cid));
> 
>  	ret = mmc_init_queue(&md->queue, card, &md->lock);
>  	if (ret)
> @@ -822,7 +874,9 @@ static struct mmc_blk_data *mmc_blk_alloc(struct
> mmc_card *card)
>  		size = card->csd.capacity << (card->csd.read_blkbits - 9);
>  	}
> 
> -	md = mmc_blk_alloc_req(card, &card->dev, size, false, NULL);
> +	md = mmc_lookup_orphan(card, &card->dev, 0, size);
> +	if (!md)
> +		md = mmc_blk_alloc_req(card, &card->dev, size, false, NULL);
>  	return md;
>  }
> 
> @@ -836,8 +890,10 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
>  	char cap_str[10];
>  	struct mmc_blk_data *part_md;
> 
> -	part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
> -				    subname);
> +	part_md = mmc_lookup_orphan(card, disk_to_dev(md->disk), part_type, size);
> +	if (!part_md)
> +		part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size,
> +					    default_ro, subname);
>  	if (IS_ERR(part_md))
>  		return PTR_ERR(part_md);
>  	part_md->part_type = part_type;
> @@ -906,6 +962,10 @@ static void mmc_blk_remove_req(struct mmc_blk_data
> *md)
> 
>  		/* Then flush out any already in there */
>  		mmc_cleanup_queue(&md->queue);
> +
> +		mutex_lock(&orphan_mutex);
> +		list_add(&md->orphan, &orphans);
> +		mutex_unlock(&orphan_mutex);
>  		mmc_blk_put(md);
>  	}
>  }
> @@ -933,8 +993,10 @@ static int mmc_add_disk(struct mmc_blk_data *md)
>  	md->force_ro.attr.name = "force_ro";
>  	md->force_ro.attr.mode = S_IRUGO | S_IWUSR;
>  	ret = device_create_file(disk_to_dev(md->disk), &md->force_ro);
> -	if (ret)
> +	if (ret) {
>  		del_gendisk(md->disk);
> +		return ret;
> +	}
> 
>  	return ret;
>  }
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 85ef72c..87c4af7 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1823,11 +1823,10 @@ int mmc_pm_notify(struct notifier_block
> *notify_block,
>  		if (!host->bus_ops || host->bus_ops->suspend)
>  			break;
> 
> -		mmc_claim_host(host);
> -
>  		if (host->bus_ops->remove)
>  			host->bus_ops->remove(host);
> 
> +		mmc_claim_host(host);
>  		mmc_detach_bus(host);
>  		mmc_release_host(host);
>  		host->pm_flags = 0;
> --
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrei Warkentin May 18, 2011, 8:06 a.m. UTC | #2
On Tue, May 17, 2011 at 5:15 AM, Dong, Chuanxiao
<chuanxiao.dong@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org
>> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Andrei Warkentin
>> Sent: Tuesday, April 05, 2011 4:31 AM
>> To: linux-mmc@vger.kernel.org
>> Cc: Andrei Warkentin
>> Subject: [RFC] MMC: Request for comments attempt at dealing with removeable
>> suspend/resume.
>>
>> Is there any value to doing something like this in order to be able to
>> suspend/resume
>> with a (manually, or rootfs) mounted filesystem on mmcblk?
>>
>> Thoughts?
> Hi Andrei,
> I also encountered the same issue with you when system is suspending. And the modification in core.c actually can fix my issue.
> For the mounted root file system, I think it makes sense for such medias to use MMC_UNSAFE_RESUME. Is there some use case need to remove root file system media?
>

Except that if the filesystem (non-root) fails to unmount or it's a
root filesystem on a device on a host which isn't MMC_UNSAFE_RESUME,
then you will re-enumerate incorrectly on resume. On resume, fs will
be on a now "dead" device (say mmc0), while kernel will re-enumerate
card as mmc1 now since mmc0 never tore down.

Basically even if you're smart enough to avoid switching cards on
suspend/resume, this can still get to you and you will wind up with
either just a hang (if root fs on card), or weird re-enumeration and
countless I/O errors to dead device.

What above patch does is that it uses a two step process to clean up
MDs. MDs are put on an orphan list when the backing device goes away.
If they can be safely cleaned up - great, and they go away completely
when the refcount drops. Otherwise, they'll be "reconnected" to if the
*same* backing device reappears again.

A
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index ee8f7a9..19eb5b6 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -53,6 +53,9 @@  MODULE_ALIAS("mmc:block");
      ((card)->ext_csd.rel_sectors)))
 
 static DEFINE_MUTEX(block_mutex);
+static DEFINE_MUTEX(orphan_mutex);
+
+struct list_head orphans = LIST_HEAD_INIT(orphans);
 
 /*
  * The defaults come from config options but can be overriden by module
@@ -77,6 +80,7 @@  struct mmc_blk_data {
 	struct gendisk	*disk;
 	struct mmc_queue queue;
 	struct list_head part;
+	struct list_head orphan;
 
 	unsigned int	usage;
 	unsigned int	read_only;
@@ -88,6 +92,7 @@  struct mmc_blk_data {
 	 * track of the current selected device partition.
 	 */
 	unsigned int	part_curr;
+	u32		raw_cid[4];
 	struct device_attribute force_ro;
 };
 
@@ -126,10 +131,12 @@  static void mmc_blk_put(struct mmc_blk_data *md)
 	mutex_lock(&open_lock);
 	md->usage--;
 	if (md->usage == 0) {
-		int devidx = mmc_get_devidx(md->disk);
-		blk_cleanup_queue(md->queue.queue);
+		mutex_lock(&orphan_mutex);
+		list_del(&md->orphan);
+		mutex_unlock(&orphan_mutex);
 
-		__clear_bit(devidx, dev_use);
+		blk_cleanup_queue(md->queue.queue);
+		__clear_bit(mmc_get_devidx(md->disk), dev_use);
 
 		put_disk(md->disk);
 		kfree(md);
@@ -718,6 +725,49 @@  static inline int mmc_blk_readonly(struct mmc_card *card)
 	       !(card->csd.cmdclass & CCC_BLOCK_WRITE);
 }
 
+static inline struct mmc_blk_data *mmc_lookup_orphan(struct mmc_card *card,
+						     struct device *parent,
+						     unsigned int part_type,
+						     sector_t size)
+{
+	int ret;
+	struct list_head *pos, *q;
+	struct mmc_blk_data *md;
+	bool found = false;
+
+	mutex_lock(&orphan_mutex);
+	list_for_each_safe(pos, q, &orphans) {
+		md = list_entry(pos, struct mmc_blk_data, orphan);
+		if (!memcmp(md->raw_cid, card->raw_cid, sizeof(md->raw_cid)) &&
+		    md->part_type == part_type) {
+			list_del(pos);
+			found = true;
+			mmc_blk_get(md->disk);
+			break;
+		}
+	}
+	mutex_unlock(&orphan_mutex);
+
+	if (!found)
+		return NULL;
+
+	ret = mmc_init_queue(&md->queue, card, &md->lock);
+	if (ret)
+		return NULL;
+
+	INIT_LIST_HEAD(&md->part);
+	md->disk->driverfs_dev = parent;
+	md->queue.issue_fn = mmc_blk_issue_rq;
+	md->queue.data = md;
+	md->disk->queue = md->queue.queue;
+	if (REL_WRITES_SUPPORTED(card))
+		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
+	blk_queue_logical_block_size(md->queue.queue, 512);
+	set_capacity(md->disk, size);
+	printk("set cap to %x\n", (unsigned int) get_capacity(md->disk));
+	return md;
+}
+
 static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 					      struct device *parent,
 					      sector_t size,
@@ -752,7 +802,9 @@  static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	spin_lock_init(&md->lock);
 	INIT_LIST_HEAD(&md->part);
+	INIT_LIST_HEAD(&md->orphan);
 	md->usage = 1;
+	memcpy(md->raw_cid, card->raw_cid, sizeof(card->raw_cid));
 
 	ret = mmc_init_queue(&md->queue, card, &md->lock);
 	if (ret)
@@ -822,7 +874,9 @@  static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
 		size = card->csd.capacity << (card->csd.read_blkbits - 9);
 	}
 
-	md = mmc_blk_alloc_req(card, &card->dev, size, false, NULL);
+	md = mmc_lookup_orphan(card, &card->dev, 0, size);
+	if (!md)
+		md = mmc_blk_alloc_req(card, &card->dev, size, false, NULL);
 	return md;
 }
 
@@ -836,8 +890,10 @@  static int mmc_blk_alloc_part(struct mmc_card *card,
 	char cap_str[10];
 	struct mmc_blk_data *part_md;
 
-	part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
-				    subname);
+	part_md = mmc_lookup_orphan(card, disk_to_dev(md->disk), part_type, size);
+	if (!part_md)
+		part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size,
+					    default_ro, subname);
 	if (IS_ERR(part_md))
 		return PTR_ERR(part_md);
 	part_md->part_type = part_type;
@@ -906,6 +962,10 @@  static void mmc_blk_remove_req(struct mmc_blk_data *md)
 
 		/* Then flush out any already in there */
 		mmc_cleanup_queue(&md->queue);
+
+		mutex_lock(&orphan_mutex);
+		list_add(&md->orphan, &orphans);
+		mutex_unlock(&orphan_mutex);
 		mmc_blk_put(md);
 	}
 }
@@ -933,8 +993,10 @@  static int mmc_add_disk(struct mmc_blk_data *md)
 	md->force_ro.attr.name = "force_ro";
 	md->force_ro.attr.mode = S_IRUGO | S_IWUSR;
 	ret = device_create_file(disk_to_dev(md->disk), &md->force_ro);
-	if (ret)
+	if (ret) {
 		del_gendisk(md->disk);
+		return ret;
+	}
 
 	return ret;
 }
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 85ef72c..87c4af7 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1823,11 +1823,10 @@  int mmc_pm_notify(struct notifier_block *notify_block,
 		if (!host->bus_ops || host->bus_ops->suspend)
 			break;
 
-		mmc_claim_host(host);
-
 		if (host->bus_ops->remove)
 			host->bus_ops->remove(host);
 
+		mmc_claim_host(host);
 		mmc_detach_bus(host);
 		mmc_release_host(host);
 		host->pm_flags = 0;