All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc
@ 2011-12-14 15:06 Ulf Hansson
  2011-12-14 15:06 ` [PATCH V2 1/2] mmc: block: Remove use of mmc_blk_set_blksize Ulf Hansson
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ulf Hansson @ 2011-12-14 15:06 UTC (permalink / raw)
  To: linux-mmc, Chris Ball; +Cc: Per Forlin, Ulf Hansson, Johan Rudholm, Lee Jones

Typically an sd/mmc card takes around 200 - 1100 ms to
initialize when the power to the card has been cut, which
is what happens during a suspend/resume sequence.

All device's resume time adds up to the total kernel resume
time. Some use cases requires the kernel to be resumed fast,
to be able to meet deadlines. One use case example is WLAN
SOFT_AP, but there are certainly more.

This patch serie deferres the resume for mmc/sd cards into
a delayed work to be able to respond quickly on the suspend
request. Earlier the blkdev were issuing a set_blksize as a
part of the blkdev resume, which right now is removed since
it is not needed. In future, if other block sizes than 512
bytes is used, this needs to be re-implemented in a different
manner.

A deferred resume feature has been discussed on the mmc-list
previously but did not end up in a final patch/solution.

Ulf Hansson (2):
  mmc: block: Remove use of mmc_blk_set_blksize
  mmc: Minimize resume-time by deferring resume

 drivers/mmc/card/block.c |   35 +++++++++--------------------------
 drivers/mmc/core/core.c  |   36 +++++++++++++++++++++++++++++++++++-
 drivers/mmc/core/core.h  |    1 +
 drivers/mmc/core/host.c  |    1 +
 include/linux/mmc/host.h |   16 ++++++++++++++++
 5 files changed, 62 insertions(+), 27 deletions(-)

-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V2 1/2] mmc: block: Remove use of mmc_blk_set_blksize
  2011-12-14 15:06 [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc Ulf Hansson
@ 2011-12-14 15:06 ` Ulf Hansson
  2011-12-14 15:06 ` [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume Ulf Hansson
  2012-01-03 15:24 ` [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc Ulf Hansson
  2 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2011-12-14 15:06 UTC (permalink / raw)
  To: linux-mmc, Chris Ball; +Cc: Per Forlin, Ulf Hansson, Johan Rudholm, Lee Jones

According to the specifications for SD and (e)MMC default
blocksize (named BLOCKLEN in Spec.) must always be 512
bytes. Since we hardcoded to always use 512 bytes, we do
not explicitly have to set it. Future improvements should
potentially make it possible to use a greater blocksize
than 512 bytes, but until then let's skip this.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Reviewed-by: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>
---

Changes in v2:
	- Rebased patch on latest version of mmc-next

---
 drivers/mmc/card/block.c |   27 +--------------------------
 1 files changed, 1 insertions(+), 26 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 0c959c9..4b62d1f 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1598,24 +1598,6 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
 	return ret;
 }
 
-static int
-mmc_blk_set_blksize(struct mmc_blk_data *md, struct mmc_card *card)
-{
-	int err;
-
-	mmc_claim_host(card->host);
-	err = mmc_set_blocklen(card, 512);
-	mmc_release_host(card->host);
-
-	if (err) {
-		pr_err("%s: unable to set block size to 512: %d\n",
-			md->disk->disk_name, err);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static void mmc_blk_remove_req(struct mmc_blk_data *md)
 {
 	struct mmc_card *card;
@@ -1742,7 +1724,6 @@ static const struct mmc_fixup blk_fixups[] =
 static int mmc_blk_probe(struct mmc_card *card)
 {
 	struct mmc_blk_data *md, *part_md;
-	int err;
 	char cap_str[10];
 
 	/*
@@ -1755,10 +1736,6 @@ static int mmc_blk_probe(struct mmc_card *card)
 	if (IS_ERR(md))
 		return PTR_ERR(md);
 
-	err = mmc_blk_set_blksize(md, card);
-	if (err)
-		goto out;
-
 	string_get_size((u64)get_capacity(md->disk) << 9, STRING_UNITS_2,
 			cap_str, sizeof(cap_str));
 	pr_info("%s: %s %s %s %s\n",
@@ -1783,7 +1760,7 @@ static int mmc_blk_probe(struct mmc_card *card)
  out:
 	mmc_blk_remove_parts(card, md);
 	mmc_blk_remove_req(md);
-	return err;
+	return 0;
 }
 
 static void mmc_blk_remove(struct mmc_card *card)
@@ -1819,8 +1796,6 @@ static int mmc_blk_resume(struct mmc_card *card)
 	struct mmc_blk_data *md = mmc_get_drvdata(card);
 
 	if (md) {
-		mmc_blk_set_blksize(md, card);
-
 		/*
 		 * Resume involves the card going into idle state,
 		 * so current partition is always the main one.
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume
  2011-12-14 15:06 [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc Ulf Hansson
  2011-12-14 15:06 ` [PATCH V2 1/2] mmc: block: Remove use of mmc_blk_set_blksize Ulf Hansson
@ 2011-12-14 15:06 ` Ulf Hansson
       [not found]   ` <CAMJBoFNU_+Ah8oxXy2Smr5L5oZ4wjZ1m8v6sp-kVcxgk4kYmNg@mail.gmail.com>
  2012-01-03 15:24 ` [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc Ulf Hansson
  2 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2011-12-14 15:06 UTC (permalink / raw)
  To: linux-mmc, Chris Ball; +Cc: Per Forlin, Ulf Hansson, Johan Rudholm, Lee Jones

Typically an sd/mmc card takes around 200 - 1100 ms to
initialize when the power to the card has been cut, which
is what happens during a suspend/resume sequence.

All device's resume time adds up to the total kernel resume
time. Some use cases requires the kernel to be resumed fast,
to be able to meet deadlines. One use case example is WLAN
SOFT_AP, but there are certainly more.

This patch schedules a delayed work to do a deferred resume
of the mmc host, if the bus holds a card of SD or MMC type.
The reason for not supporting SDIO and SDcombo cards at this
stage, is because the SDIO API is synchronus, which complicates
request locking mechanism when waiting for a deferred resume to
be completed.

While waiting for a deferred resume to be completed, detect works
are prevented from doing a new rescan. If a mmcblk request arrives,
the deferred resume will be synced immediately.

The deferred resume is scheduled 3000 ms after the resume request
arrived. The idea behind this timer value is to let the mmc host
being able to accept a new suspend request before it has been
deferred resumed and thus not increase the resume to suspend time
if not really needed.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---

Changes in v2:
	- Rebased patch on latest version of mmc-next

---
 drivers/mmc/card/block.c |    8 ++++++++
 drivers/mmc/core/core.c  |   36 +++++++++++++++++++++++++++++++++++-
 drivers/mmc/core/core.h  |    1 +
 drivers/mmc/core/host.c  |    1 +
 include/linux/mmc/host.h |   16 ++++++++++++++++
 5 files changed, 61 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 4b62d1f..a4d8ae4 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1364,6 +1364,14 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
 	struct mmc_blk_data *md = mq->data;
 	struct mmc_card *card = md->queue.card;
 
+	/*
+	 * We must make sure we have not claimed the host before
+	 * doing a flush to prevent deadlock, thus we check if
+	 * the host needs a resume first.
+	 */
+	if (mmc_host_needs_resume(card->host))
+		mmc_resume_host_sync(card->host);
+
 	if (req && !mq->mqrq_prev->req)
 		/* claim host only for the first request */
 		mmc_claim_host(card->host);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a2aa860..4706284 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2099,7 +2099,7 @@ void mmc_rescan(struct work_struct *work)
 		container_of(work, struct mmc_host, detect.work);
 	int i;
 
-	if (host->rescan_disable)
+	if (host->rescan_disable || mmc_host_needs_resume(host))
 		return;
 
 	mmc_bus_get(host);
@@ -2354,7 +2354,13 @@ int mmc_suspend_host(struct mmc_host *host)
 	if (host->caps & MMC_CAP_DISABLE)
 		cancel_delayed_work(&host->disable);
 	cancel_delayed_work(&host->detect);
+	cancel_delayed_work_sync(&host->resume);
 	mmc_flush_scheduled_work();
+
+	/* Skip suspend, if deferred resume were scheduled but not completed. */
+	if (mmc_host_needs_resume(host))
+		return 0;
+
 	err = mmc_cache_ctrl(host, 0);
 	if (err)
 		goto out;
@@ -2393,6 +2399,10 @@ int mmc_suspend_host(struct mmc_host *host)
 				mmc_release_host(host);
 				host->pm_flags = 0;
 				err = 0;
+			} else if (mmc_card_mmc(host->card) ||
+				   mmc_card_sd(host->card)) {
+				host->pm_state |= MMC_HOST_DEFERRED_RESUME |
+						  MMC_HOST_NEEDS_RESUME;
 			}
 		} else {
 			err = -EBUSY;
@@ -2417,6 +2427,12 @@ int mmc_resume_host(struct mmc_host *host)
 {
 	int err = 0;
 
+	if (mmc_host_deferred_resume(host)) {
+		mmc_schedule_delayed_work(&host->resume,
+					  msecs_to_jiffies(3000));
+		return 0;
+	}
+
 	mmc_bus_get(host);
 	if (host->bus_ops && !host->bus_dead) {
 		if (!mmc_card_keep_power(host)) {
@@ -2451,6 +2467,24 @@ int mmc_resume_host(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_resume_host);
 
+void mmc_resume_work(struct work_struct *work)
+{
+	struct mmc_host *host =
+		container_of(work, struct mmc_host, resume.work);
+
+	host->pm_state &= ~MMC_HOST_DEFERRED_RESUME;
+	mmc_resume_host(host);
+	host->pm_state &= ~MMC_HOST_NEEDS_RESUME;
+
+	mmc_detect_change(host, 0);
+}
+
+void mmc_resume_host_sync(struct mmc_host *host)
+{
+	flush_delayed_work_sync(&host->resume);
+}
+EXPORT_SYMBOL(mmc_resume_host_sync);
+
 /* Do the card removal on suspend if card is assumed removeable
  * Do that in pm notifier while userspace isn't yet frozen, so we will be able
    to sync the card.
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 3400924..c894a3f 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -59,6 +59,7 @@ static inline void mmc_delay(unsigned int ms)
 void mmc_rescan(struct work_struct *work);
 void mmc_start_host(struct mmc_host *host);
 void mmc_stop_host(struct mmc_host *host);
+void mmc_resume_work(struct work_struct *work);
 
 int _mmc_detect_card_removed(struct mmc_host *host);
 
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index c152ce0..d6f955a 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -331,6 +331,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 	init_waitqueue_head(&host->wq);
 	INIT_DELAYED_WORK(&host->detect, mmc_rescan);
 	INIT_DELAYED_WORK_DEFERRABLE(&host->disable, mmc_host_deeper_disable);
+	INIT_DELAYED_WORK(&host->resume, mmc_resume_work);
 #ifdef CONFIG_PM
 	host->pm_notify.notifier_call = mmc_pm_notify;
 #endif
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 9a03d03..90dd299 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -301,6 +301,11 @@ struct mmc_host {
 	struct delayed_work	detect;
 	int			detect_change;	/* card detect flag */
 
+	struct delayed_work	resume;		/* deferred resume work */
+	unsigned int		pm_state;	/* used for deferred resume */
+#define MMC_HOST_DEFERRED_RESUME	(1 << 0)
+#define MMC_HOST_NEEDS_RESUME		(1 << 1)
+
 	const struct mmc_bus_ops *bus_ops;	/* current bus driver */
 	unsigned int		bus_refs;	/* reference counter */
 
@@ -349,6 +354,7 @@ static inline void *mmc_priv(struct mmc_host *host)
 
 extern int mmc_suspend_host(struct mmc_host *);
 extern int mmc_resume_host(struct mmc_host *);
+extern void mmc_resume_host_sync(struct mmc_host *);
 
 extern int mmc_power_save_host(struct mmc_host *host);
 extern int mmc_power_restore_host(struct mmc_host *host);
@@ -428,4 +434,14 @@ static inline int mmc_boot_partition_access(struct mmc_host *host)
 	return !(host->caps2 & MMC_CAP2_BOOTPART_NOACC);
 }
 
+static inline int mmc_host_deferred_resume(struct mmc_host *host)
+{
+	return host->pm_state & MMC_HOST_DEFERRED_RESUME;
+}
+
+static inline int mmc_host_needs_resume(struct mmc_host *host)
+{
+	return host->pm_state & MMC_HOST_NEEDS_RESUME;
+}
+
 #endif /* LINUX_MMC_HOST_H */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume
       [not found]   ` <CAMJBoFNU_+Ah8oxXy2Smr5L5oZ4wjZ1m8v6sp-kVcxgk4kYmNg@mail.gmail.com>
@ 2011-12-14 15:29     ` Ulf Hansson
       [not found]       ` <CAMJBoFM9v9ugDL8gtNJ+AMCKmvOsUQHEmEf13VDXko41AkFfFA@mail.gmail.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2011-12-14 15:29 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones

Vitaly Wool wrote:
> Hi Ulf,
> 
> On Wed, Dec 14, 2011 at 4:06 PM, Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>> wrote:
> Typically an sd/mmc card takes around 200 - 1100 ms to
> initialize when the power to the card has been cut, which
> is what happens during a suspend/resume sequence.
> 
> I'm a bit pessimistic about this patch. What if we have a root filesystem on an SD card, or, what is a more common case, on an eMMC? How is it going to be handled?
> 

This is handled for sure. I have verified this case and I agree that 
this is likely a common case.

In principle, every mmc/sd requests handled in issue_rq (block.c), will 
unless the host is not already resumed, do a "sync" of the resume work.


> I see no trace of taking this into account in here so it's a NAK for now from my side.
> 
> Thanks,
>    Vitaly
> 

Br
Ulf Hansson

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume
       [not found]         ` <CAMJBoFPcBzezFDRXpL36zJZTttGgUFzDCUaFC69Pxg+gEWcAOw@mail.gmail.com>
@ 2011-12-14 16:43           ` Ulf Hansson
       [not found]             ` <CAMJBoFPSEtM1rQ7Rq8d+Y6Qgd_GKq2o4p=vk60wZdbJ_Mhk_Gw@mail.gmail.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2011-12-14 16:43 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones

Vitaly Wool wrote:
> On Wed, Dec 14, 2011 at 4:29 PM, Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>> wrote:
> 
> I'm a bit pessimistic about this patch. What if we have a root filesystem on an SD card, or, what is a more common case, on an eMMC? How is it going to be handled?
> 
> 
> This is handled for sure. I have verified this case and I agree that this is likely a common case.
> 
> In principle, every mmc/sd requests handled in issue_rq (block.c), will unless the host is not already resumed, do a "sync" of the resume work.
> 
> 
> So in fact instead of decreasing time-to-userspace for resume, you are rather going to increase it in this case.

Nope, not true. :-)

Somewhere you need to handle the resume. Earlier it was done immediately 
when getting the resume request, thus every host resume in the sequence 
is added to the total time for userspace to be resumed.

I did some measurement, having two eMMC connected (one of them with a 
root file system mounted) and one rather good SD-card with VFAT. The 
resume time for the kernel before these patches were around 600 ms. 
After my patches I had around 20 ms.

Moreover, I noticed very seldom than any mmc/sd request arrived within 
the time were the deferred resumed has not been done. Of course this 
will very much depend on what kind of userspace application that is 
running and if there is an ongoing file transfer that were frozen when 
doing suspend.

But, if this happens (deferred resume not done), the total resume time 
for that particular userspace application will anyway be heavily 
decreased since the other hosts resume time did not affect the resume 
time for this application.

  >
> IMHO, at the end of the day this thing should either
> - be capability-based (add MMC_CAP_...)

Why do someone not want this?

> - be PM_QOS based

Why do someone not want this?

> -be async (e. g. start card resume process in resume routine, set atomic, return success and have wait_event_interruptible_timeout in block_rq if this atomic is set).

Don't follow you. This is exactly what the patch is doing. Not just for 
SD, but also for (e)MMC. I don't see your issue.

> 
> Anyway, in fact this patch is only addressing SD card case as of now which can be covered by runtime PM.

Nope, both SD and (e)MMC. How can runtime PM solve a normal suspend? It 
has noting to do we each other I believe.

> 
> Thanks,
>    Vitaly
> 


Br
Ulf Hansson

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume
       [not found]             ` <CAMJBoFPSEtM1rQ7Rq8d+Y6Qgd_GKq2o4p=vk60wZdbJ_Mhk_Gw@mail.gmail.com>
@ 2011-12-15  9:22               ` Ulf Hansson
       [not found]                 ` <CAMJBoFN5tub+Lv5p2Ae3UQY48G2TMBBmdkdsA1TNDqf2stDGVg@mail.gmail.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2011-12-15  9:22 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones

Hi Vitaly,

Vitaly Wool wrote:
> 
> On Wed, Dec 14, 2011 at 5:43 PM, Ulf Hansson
> <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
> wrote:
> 
> So in fact instead of decreasing time-to-userspace for resume, you
> are rather going to increase it in this case.
> 
> Nope, not true. :-)
> 
> Somewhere you need to handle the resume. Earlier it was done
> immediately when getting the resume request, thus every host resume
> in the sequence is added to the total time for userspace to be
> resumed.
> 
> I did some measurement, having two eMMC connected (one of them with a
> root file system mounted) and one rather good SD-card with VFAT. The
> resume time for the kernel before these patches were around 600 ms.
> After my patches I had around 20 ms.
> 
> What do you call "resume time" in this case?

Total kernel resume time.

> 
> 
> Moreover, I noticed very seldom than any mmc/sd request arrived
> within the time were the deferred resumed has not been done. Of
> course this will very much depend on what kind of userspace
> application that is running and if there is an ongoing file transfer
> that were frozen when doing suspend.
> 
> ...or  the wakeup source was the userspace alarm etc. etc.
> 
> 
> But, if this happens (deferred resume not done), the total resume
> time for that particular userspace application will anyway be heavily
> decreased since the other hosts resume time did not affect the resume
> time for this application.
> 
> I take that by "other hosts" you mean SD card? :)

"Other hosts", are all those hosts holding an eMMC/MMC or an SD-card,
but not that host that there were a request for, before the deferred
resume has finalized.

> 
> 
> -be async (e. g. start card resume process in resume routine, set
> atomic, return success and have wait_event_interruptible_timeout in
> block_rq if this atomic is set).
> 
> Don't follow you. This is exactly what the patch is doing. Not just
> for SD, but also for (e)MMC. I don't see your issue.
> 
> The issue is  that in most cases when you have a root filesystem on
> the eMMC, block request will come in less than 3 seconds and this
> application will have to wait. You don't spawn resume immediately.
> 
> Also, how about race conditions resuming say SD and eMMC at the same
> time?

One host holds one card. One card has one blkdev thread/queue. Resume is
done for each host separate, no such thing as race can ever occur, I
believe.

> 
> 
> Anyway, in fact this patch is only addressing SD card case as of now
> which can be covered by runtime PM.
> 
> Nope, both SD and (e)MMC. How can runtime PM solve a normal suspend?
> It has noting to do we each other I believe.
> 
> SD card may not be resumed on system resume if it was runtime
> suspended before system suspend. That basically covers your case,
> doesn't it?

Don't quite follow you. Right now SDIO is the only type of card that
make use of runtime PM, which means mmc_power_save|restore_host could
potentially be executed for SDIO cards.

Do you mean that we should implement this for SD cards as well?

Anyway, I don't understand what this should prevent a resume from being
executed for SD/SDIO/(e)MMC at all?

Please elaborate.

> 
> ~Vitaly
> 

Please try to in-line comments, it will make it easier to follow the
discussion.


Br
Ulf Hansson


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume
       [not found]                 ` <CAMJBoFN5tub+Lv5p2Ae3UQY48G2TMBBmdkdsA1TNDqf2stDGVg@mail.gmail.com>
@ 2011-12-19 11:03                   ` Ulf Hansson
       [not found]                   ` <4EEF0541.6070306@stericsson.com>
  1 sibling, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2011-12-19 11:03 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones

Vitaly Wool wrote:
> Hi,
> 
> I did some measurement, having two eMMC connected (one of them with a
>  root file system mounted) and one rather good SD-card with VFAT. The
>  resume time for the kernel before these patches were around 600 ms. 
> After my patches I had around 20 ms.
> 
> What do you call "resume time" in this case?
> 
> Total kernel resume time.
> 
> This is an artificial and unfair definition. No one should care about
> that. You can think of a time-to-splashscreen or
> time-to-functional-userspace and both are not going to be improved
> much with your patch if we run off of a eMMC-based root.
> 

This is again not correct. You must not _always_ assume, just because 
you have a eMMC-based root, that requests arrives within 3 s after the 
kernel has been resumed, this is completely dependent on the active use 
cases. Moreover I think you are missing a very valuable thing about how 
the resume sequence is being changed with this patch. I will try to 
elaborate a bit more.

Before this patch:
_Every_ MMC and SD card were resumed sequentially. Thus _every_ resume 
for _every_ card adds up to the resume time _always_.

After this patch:
1. MMC and SD cards can be is resumed in parallel since the resume for 
those cards are deferred to be handled in a work instead.

2. Only those usecases which sends request on MMC and SD cards within ~3 
s after the kernel has been resumed, will notice a resume time for that 
particular card and of course only for the first request after a resume. 
Requests triggered after the ~3 s will be performed on a already resumed 
host.


> 
> 
> Moreover, I noticed very seldom than any mmc/sd request arrived 
> within the time were the deferred resumed has not been done. Of 
> course this will very much depend on what kind of userspace 
> application that is running and if there is an ongoing file transfer 
> that were frozen when doing suspend.
> 
> ...or  the wakeup source was the userspace alarm etc. etc.
> 
> 
> But, if this happens (deferred resume not done), the total resume 
> time for that particular userspace application will anyway be heavily
>  decreased since the other hosts resume time did not affect the
> resume time for this application.
> 
> I take that by "other hosts" you mean SD card? :)
> 
> "Other hosts", are all those hosts holding an eMMC/MMC or an SD-card,
>  but not that host that there were a request for, before the deferred
>  resume has finalized.
> 
> Ok, what if a rootfs application to be started first has to re-read
> say certificates from the SD card? And what if not doing that in time
> means QoS degradation?
> 
> No, you don't see all the _real_ use cases that you can break with
> your patch.
> 

I understand that potentially some userspace application might be 
affected. I did not see this a any critical problem which you definitely 
pointed out that it might be. But the reason to why I took this approach 
is simply because I think it is more a matter of in what context you are 
"waiting".

Earlier a userspace application could not even execute until all mmc and 
sd cards were resumed and thus waiting a long time for the kernel to be 
resumed. After this patch a userspace application will be able to 
execute much earlier but could in worst case have to wait for a SD or 
MMC card to be resumed when it need access to it.

I think this problem for these userspace applications, should be 
possible to solve within each application instead. Do you not think that 
should be possible?

If you feel that dot 2 (see above) is kind of what makes this patch more 
problematic for you to accept, what do you think about removing the 3 s 
resume delay and instead schedule the resume work immediately?

> 
> 
> 
> 
> -be async (e. g. start card resume process in resume routine, set 
> atomic, return success and have wait_event_interruptible_timeout in 
> block_rq if this atomic is set).
> 
> Don't follow you. This is exactly what the patch is doing. Not just 
> for SD, but also for (e)MMC. I don't see your issue.
> 
> No it doesn't. It defers the execution by arbitrary chosen time of 3
> seconds.
> 

Execution? Why?

> 
> 
> Do you mean that we should implement this for SD cards as well?
> 
> Anyway, I don't understand what this should prevent a resume from
> being executed for SD/SDIO/(e)MMC at all?
> 
> Please elaborate.
> 
> 
> I'm not going to, at least not in this thread. The method you propose
> is a hack and it can not be accepted at least because it doesn't give
> a damn about QoS. The whole idea of "let's unconditionally defer
> resume of something for arbitrary amount of time no matter what" is
> invalid.

As suggested above. We could easily remove the timer for the scheduled 
work. The idea as such will not change for this patch but userspace will 
be less affected.

It would have been good to really understand more about the idea you 
have of how to solve the long kernel resume time for mmc/sd/sdio with 
QoS. If you decide to send out a patch of rfc please keep me on the cc-list.

> 
> Thanks, Vitaly
> 

Br
Ulf Hansson

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume
       [not found]                   ` <4EEF0541.6070306@stericsson.com>
@ 2011-12-19 12:21                     ` Ulf Hansson
  0 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2011-12-19 12:21 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones

If you got this half written reply, please ignore it.

BR
Ulf Hansson

Ulf Hansson wrote:
> Vitaly Wool wrote:
>> Hi,
>>
>> I did some measurement, having two eMMC connected (one of them with a
>>  root file system mounted) and one rather good SD-card with VFAT. The
>>  resume time for the kernel before these patches were around 600 ms. 
>> After my patches I had around 20 ms.
>>
>> What do you call "resume time" in this case?
>>
>> Total kernel resume time.
>>
>> This is an artificial and unfair definition. No one should care about
>> that. You can think of a time-to-splashscreen or
>> time-to-functional-userspace and both are not going to be improved
>> much with your patch if we run off of a eMMC-based root.
>>
> 
> I think you are missing a very valuable thing about how the resume 
> sequence is being changed with this patch.
> 
> Before this patch:
> _Every_ MMC and SD card were resumed sequentially. Thus every resume for 
> every card adds up to the resume time _always_.
> 
> After this patch:
> 1. MMC and SD cards can be is resumed in parallel.
> 2. Only MMC and SD cards which will get requests within ~3 s after the 
> kernel has resumed will notice the resume time. Requests triggered later 
> will be performed on a already resumed host.done coming
> 
>   moreover, only if there arrives a request for before
> 
> only tho No resume is forst of all only done if there is a request
> 
> 
> So for some reas and that the resume time (and also the the time for use
> 
> 
>       in a sequence
> 
>>
>> Moreover, I noticed very seldom than any mmc/sd request arrived 
>> within the time were the deferred resumed has not been done. Of 
>> course this will very much depend on what kind of userspace 
>> application that is running and if there is an ongoing file transfer 
>> that were frozen when doing suspend.
>>
>> ...or  the wakeup source was the userspace alarm etc. etc.
>>
>>
>> But, if this happens (deferred resume not done), the total resume 
>> time for that particular userspace application will anyway be heavily
>>  decreased since the other hosts resume time did not affect the
>> resume time for this application.
>>
>> I take that by "other hosts" you mean SD card? :)
>>
>> "Other hosts", are all those hosts holding an eMMC/MMC or an SD-card,
>>  but not that host that there were a request for, before the deferred
>>  resume has finalized.
>>
>> Ok, what if a rootfs application to be started first has to re-read
>> say certificates from the SD card? And what if not doing that in time
>> means QoS degradation?
>>
>> No, you don't see all the _real_ use cases that you can break with
>> your patch.
>>
>>
>>
>>
>>
>> -be async (e. g. start card resume process in resume routine, set 
>> atomic, return success and have wait_event_interruptible_timeout in 
>> block_rq if this atomic is set).
>>
>> Don't follow you. This is exactly what the patch is doing. Not just 
>> for SD, but also for (e)MMC. I don't see your issue.
>>
>> No it doesn't. It defers the execution by arbitrary chosen time of 3
>> seconds.
>>
>>
>>
>> Do you mean that we should implement this for SD cards as well?
>>
>> Anyway, I don't understand what this should prevent a resume from
>> being executed for SD/SDIO/(e)MMC at all?
>>
>> Please elaborate.
>>
>>
>> I'm not going to, at least not in this thread. The method you propose
>> is a hack and it can not be accepted at least because it doesn't give
>> a damn about QoS. The whole idea of "let's unconditionally defer
>> resume of something for arbitrary amount of time no matter what" is
>> invalid.
>>
>> Thanks, Vitaly
>>
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc
  2011-12-14 15:06 [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc Ulf Hansson
  2011-12-14 15:06 ` [PATCH V2 1/2] mmc: block: Remove use of mmc_blk_set_blksize Ulf Hansson
  2011-12-14 15:06 ` [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume Ulf Hansson
@ 2012-01-03 15:24 ` Ulf Hansson
  2 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2012-01-03 15:24 UTC (permalink / raw)
  To: Chris Ball
  Cc: Ulf HANSSON, linux-mmc, Per FORLIN, Johan RUDHOLM, Lee Jones, vitalywool

Hi Chris,

I have received some great comments some Vitaly Wool around the patch 2
"mmc: Minimize resume-time by deferring resume".

It would be interesting to hear if you or someone else has any thinking 
around these patches. Vitaly were mainly thinking of a pm_qos approach, 
which still is a bit unclear to me how to implement.

Any suggestion would be great! :-)

Regarding patch 1:
mmc: block: Remove use of mmc_blk_set_blksize
This patch could be discussed separately without patch 2. Do you want me 
to push this patch on it's own?


BR
Ulf Hansson



Ulf HANSSON wrote:
> Typically an sd/mmc card takes around 200 - 1100 ms to
> initialize when the power to the card has been cut, which
> is what happens during a suspend/resume sequence.
> 
> All device's resume time adds up to the total kernel resume
> time. Some use cases requires the kernel to be resumed fast,
> to be able to meet deadlines. One use case example is WLAN
> SOFT_AP, but there are certainly more.
> 
> This patch serie deferres the resume for mmc/sd cards into
> a delayed work to be able to respond quickly on the suspend
> request. Earlier the blkdev were issuing a set_blksize as a
> part of the blkdev resume, which right now is removed since
> it is not needed. In future, if other block sizes than 512
> bytes is used, this needs to be re-implemented in a different
> manner.
> 
> A deferred resume feature has been discussed on the mmc-list
> previously but did not end up in a final patch/solution.
> 
> Ulf Hansson (2):
>   mmc: block: Remove use of mmc_blk_set_blksize
>   mmc: Minimize resume-time by deferring resume
> 
>  drivers/mmc/card/block.c |   35 +++++++++--------------------------
>  drivers/mmc/core/core.c  |   36 +++++++++++++++++++++++++++++++++++-
>  drivers/mmc/core/core.h  |    1 +
>  drivers/mmc/core/host.c  |    1 +
>  include/linux/mmc/host.h |   16 ++++++++++++++++
>  5 files changed, 62 insertions(+), 27 deletions(-)
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-01-03 15:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-14 15:06 [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc Ulf Hansson
2011-12-14 15:06 ` [PATCH V2 1/2] mmc: block: Remove use of mmc_blk_set_blksize Ulf Hansson
2011-12-14 15:06 ` [PATCH V2 2/2] mmc: Minimize resume-time by deferring resume Ulf Hansson
     [not found]   ` <CAMJBoFNU_+Ah8oxXy2Smr5L5oZ4wjZ1m8v6sp-kVcxgk4kYmNg@mail.gmail.com>
2011-12-14 15:29     ` Ulf Hansson
     [not found]       ` <CAMJBoFM9v9ugDL8gtNJ+AMCKmvOsUQHEmEf13VDXko41AkFfFA@mail.gmail.com>
     [not found]         ` <CAMJBoFPcBzezFDRXpL36zJZTttGgUFzDCUaFC69Pxg+gEWcAOw@mail.gmail.com>
2011-12-14 16:43           ` Ulf Hansson
     [not found]             ` <CAMJBoFPSEtM1rQ7Rq8d+Y6Qgd_GKq2o4p=vk60wZdbJ_Mhk_Gw@mail.gmail.com>
2011-12-15  9:22               ` Ulf Hansson
     [not found]                 ` <CAMJBoFN5tub+Lv5p2Ae3UQY48G2TMBBmdkdsA1TNDqf2stDGVg@mail.gmail.com>
2011-12-19 11:03                   ` Ulf Hansson
     [not found]                   ` <4EEF0541.6070306@stericsson.com>
2011-12-19 12:21                     ` Ulf Hansson
2012-01-03 15:24 ` [PATCH V2 0/2] mmc: Minimize resume time for sd/mmc Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.