diff mbox

mmc: core: Optimize boot time by detecting cards simultaneously

Message ID 1448982916-21485-1-git-send-email-ulf.hansson@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Ulf Hansson Dec. 1, 2015, 3:15 p.m. UTC
The mmc workqueue is an ordered workqueue, allowing only one work to
execute per given time. As this workqueue is used for card detection, the
conseqeunce is that cards will be detected one by one waiting for each
other.

Moreover, most of the time spent during card initialization is waiting for
the card's internal firmware to be ready. From a CPU perspective this
typically means waiting for a completion variable to be kicked via an
IRQ-handler or waiting for a sleep timer to finish.

This behaviour of detecting/initializing cards is sub-optimal, especially
for SOCs having several controllers/cards.

Let's convert to use the system_freezable_wq for the mmc detect works.
This enables several works to be executed simultaneously and thus also
cards to be detected like so.

Tests on UX500, which holds two eMMC cards and an SD-card (actually also
an SDIO card, currently not detected), shows a significant improved
behaviour due to this change.

Before this change, both the eMMC cards waited for the SD card to be
initialized as its detect work entered the workqueue first. In some cases,
depending on the characteristic of the SD-card, they got delayed 1-1.5 s.

Additionally for the second eMMC, it needed to wait for the first eMMC to
be initialized which added another 120-190 ms.

Converting to the system_freezable_wq, removed these delays and made both
the eMMC cards available far earlier in the boot sequence.

Selecting the system_freezable_wq, in favour of for example the system_wq,
is because we need card detection mechanism to be disabled once userspace
are frozen during system PM. Currently the mmc core deal with this via PM
notifiers, but following patches may utilize the behaviour of the
system_freezable_wq, to simplify the use of the PM notifiers.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/core.c | 31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

Comments

Ulf Hansson Dec. 14, 2015, 3:50 p.m. UTC | #1
On 1 December 2015 at 16:15, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The mmc workqueue is an ordered workqueue, allowing only one work to
> execute per given time. As this workqueue is used for card detection, the
> conseqeunce is that cards will be detected one by one waiting for each
> other.
>
> Moreover, most of the time spent during card initialization is waiting for
> the card's internal firmware to be ready. From a CPU perspective this
> typically means waiting for a completion variable to be kicked via an
> IRQ-handler or waiting for a sleep timer to finish.
>
> This behaviour of detecting/initializing cards is sub-optimal, especially
> for SOCs having several controllers/cards.
>
> Let's convert to use the system_freezable_wq for the mmc detect works.
> This enables several works to be executed simultaneously and thus also
> cards to be detected like so.
>
> Tests on UX500, which holds two eMMC cards and an SD-card (actually also
> an SDIO card, currently not detected), shows a significant improved
> behaviour due to this change.
>
> Before this change, both the eMMC cards waited for the SD card to be
> initialized as its detect work entered the workqueue first. In some cases,
> depending on the characteristic of the SD-card, they got delayed 1-1.5 s.
>
> Additionally for the second eMMC, it needed to wait for the first eMMC to
> be initialized which added another 120-190 ms.
>
> Converting to the system_freezable_wq, removed these delays and made both
> the eMMC cards available far earlier in the boot sequence.
>
> Selecting the system_freezable_wq, in favour of for example the system_wq,
> is because we need card detection mechanism to be disabled once userspace
> are frozen during system PM. Currently the mmc core deal with this via PM
> notifiers, but following patches may utilize the behaviour of the
> system_freezable_wq, to simplify the use of the PM notifiers.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Even-though the lack of reviewers, I have decided to give this a try
and pushed it for my next branch.

If it isn't clear by the above change-log:
A side effect from this change is that mmc block partitions, may end
up being given other indexes than they had before. Whether that may
cause issues, depends on if the path to the rootfs have been
hard-coded or not.

The recommended way to solve this is to use UUID/PARTUUID.

An important note, there has *never* been any guarantees that mmc
block partitions get the same index at every boot, as that depends on
the probe order of the mmc host devices and whether a removable card
is inserted or not.
For those platforms that don't take this into account, suffers from a
fragile boot dependency and needs to be fixed.

I will check reports from the kernelci.org as my next branch is being
tested from there. Of course I appreciate any other issues being
reported!

Kind regards
Uffe

> ---
>  drivers/mmc/core/core.c | 31 ++++++++-----------------------
>  1 file changed, 8 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 910aa25..f95d41f 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -55,7 +55,6 @@
>   */
>  #define MMC_BKOPS_MAX_TIMEOUT  (4 * 60 * 1000) /* max time to wait in ms */
>
> -static struct workqueue_struct *workqueue;
>  static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
>
>  /*
> @@ -66,21 +65,16 @@ static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
>  bool use_spi_crc = 1;
>  module_param(use_spi_crc, bool, 0);
>
> -/*
> - * Internal function. Schedule delayed work in the MMC work queue.
> - */
>  static int mmc_schedule_delayed_work(struct delayed_work *work,
>                                      unsigned long delay)
>  {
> -       return queue_delayed_work(workqueue, work, delay);
> -}
> -
> -/*
> - * Internal function. Flush all scheduled work from the MMC work queue.
> - */
> -static void mmc_flush_scheduled_work(void)
> -{
> -       flush_workqueue(workqueue);
> +       /*
> +        * We use the system_freezable_wq, because of two reasons.
> +        * First, it allows several works (not the same work item) to be
> +        * executed simultaneously. Second, the queue becomes frozen when
> +        * userspace becomes frozen during system PM.
> +        */
> +       return queue_delayed_work(system_freezable_wq, work, delay);
>  }
>
>  #ifdef CONFIG_FAIL_MMC_REQUEST
> @@ -2669,7 +2663,6 @@ void mmc_stop_host(struct mmc_host *host)
>
>         host->rescan_disable = 1;
>         cancel_delayed_work_sync(&host->detect);
> -       mmc_flush_scheduled_work();
>
>         /* clear pm flags now and let card drivers set them as needed */
>         host->pm_flags = 0;
> @@ -2852,13 +2845,9 @@ static int __init mmc_init(void)
>  {
>         int ret;
>
> -       workqueue = alloc_ordered_workqueue("kmmcd", 0);
> -       if (!workqueue)
> -               return -ENOMEM;
> -
>         ret = mmc_register_bus();
>         if (ret)
> -               goto destroy_workqueue;
> +               return ret;
>
>         ret = mmc_register_host_class();
>         if (ret)
> @@ -2874,9 +2863,6 @@ unregister_host_class:
>         mmc_unregister_host_class();
>  unregister_bus:
>         mmc_unregister_bus();
> -destroy_workqueue:
> -       destroy_workqueue(workqueue);
> -
>         return ret;
>  }
>
> @@ -2885,7 +2871,6 @@ static void __exit mmc_exit(void)
>         sdio_unregister_bus();
>         mmc_unregister_host_class();
>         mmc_unregister_bus();
> -       destroy_workqueue(workqueue);
>  }
>
>  subsys_initcall(mmc_init);
> --
> 1.9.1
>
--
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
Jaehoon Chung Dec. 15, 2015, 11:22 p.m. UTC | #2
On 12/15/2015 12:50 AM, Ulf Hansson wrote:
> On 1 December 2015 at 16:15, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> The mmc workqueue is an ordered workqueue, allowing only one work to
>> execute per given time. As this workqueue is used for card detection, the
>> conseqeunce is that cards will be detected one by one waiting for each
>> other.
>>
>> Moreover, most of the time spent during card initialization is waiting for
>> the card's internal firmware to be ready. From a CPU perspective this
>> typically means waiting for a completion variable to be kicked via an
>> IRQ-handler or waiting for a sleep timer to finish.
>>
>> This behaviour of detecting/initializing cards is sub-optimal, especially
>> for SOCs having several controllers/cards.
>>
>> Let's convert to use the system_freezable_wq for the mmc detect works.
>> This enables several works to be executed simultaneously and thus also
>> cards to be detected like so.
>>
>> Tests on UX500, which holds two eMMC cards and an SD-card (actually also
>> an SDIO card, currently not detected), shows a significant improved
>> behaviour due to this change.
>>
>> Before this change, both the eMMC cards waited for the SD card to be
>> initialized as its detect work entered the workqueue first. In some cases,
>> depending on the characteristic of the SD-card, they got delayed 1-1.5 s.
>>
>> Additionally for the second eMMC, it needed to wait for the first eMMC to
>> be initialized which added another 120-190 ms.

how did you check this? I also want to test with similar environment. :)

>>
>> Converting to the system_freezable_wq, removed these delays and made both
>> the eMMC cards available far earlier in the boot sequence.
>>
>> Selecting the system_freezable_wq, in favour of for example the system_wq,
>> is because we need card detection mechanism to be disabled once userspace
>> are frozen during system PM. Currently the mmc core deal with this via PM
>> notifiers, but following patches may utilize the behaviour of the
>> system_freezable_wq, to simplify the use of the PM notifiers.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> 
> Even-though the lack of reviewers, I have decided to give this a try
> and pushed it for my next branch.
> 
> If it isn't clear by the above change-log:
> A side effect from this change is that mmc block partitions, may end
> up being given other indexes than they had before. Whether that may
> cause issues, depends on if the path to the rootfs have been
> hard-coded or not.

What does "mmc block partitions, may end up being given other indexes than they had before." mean?
Is it possible to increase the index for partition number?
If my understanding is right...mmcblk's partition index is not fixed.
Sometime eMMC should be mmcblk0pX or mmcblk1px..right? hmm...

I need to check more...with other boards and SoC.

Best Regards,
Jaehoon Chung

> 
> The recommended way to solve this is to use UUID/PARTUUID.
> 
> An important note, there has *never* been any guarantees that mmc
> block partitions get the same index at every boot, as that depends on
> the probe order of the mmc host devices and whether a removable card
> is inserted or not.
> For those platforms that don't take this into account, suffers from a
> fragile boot dependency and needs to be fixed.
> 
> I will check reports from the kernelci.org as my next branch is being
> tested from there. Of course I appreciate any other issues being
> reported!
> 
> Kind regards
> Uffe
> 
>> ---
>>  drivers/mmc/core/core.c | 31 ++++++++-----------------------
>>  1 file changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 910aa25..f95d41f 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -55,7 +55,6 @@
>>   */
>>  #define MMC_BKOPS_MAX_TIMEOUT  (4 * 60 * 1000) /* max time to wait in ms */
>>
>> -static struct workqueue_struct *workqueue;
>>  static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
>>
>>  /*
>> @@ -66,21 +65,16 @@ static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
>>  bool use_spi_crc = 1;
>>  module_param(use_spi_crc, bool, 0);
>>
>> -/*
>> - * Internal function. Schedule delayed work in the MMC work queue.
>> - */
>>  static int mmc_schedule_delayed_work(struct delayed_work *work,
>>                                      unsigned long delay)
>>  {
>> -       return queue_delayed_work(workqueue, work, delay);
>> -}
>> -
>> -/*
>> - * Internal function. Flush all scheduled work from the MMC work queue.
>> - */
>> -static void mmc_flush_scheduled_work(void)
>> -{
>> -       flush_workqueue(workqueue);
>> +       /*
>> +        * We use the system_freezable_wq, because of two reasons.
>> +        * First, it allows several works (not the same work item) to be
>> +        * executed simultaneously. Second, the queue becomes frozen when
>> +        * userspace becomes frozen during system PM.
>> +        */
>> +       return queue_delayed_work(system_freezable_wq, work, delay);
>>  }
>>
>>  #ifdef CONFIG_FAIL_MMC_REQUEST
>> @@ -2669,7 +2663,6 @@ void mmc_stop_host(struct mmc_host *host)
>>
>>         host->rescan_disable = 1;
>>         cancel_delayed_work_sync(&host->detect);
>> -       mmc_flush_scheduled_work();
>>
>>         /* clear pm flags now and let card drivers set them as needed */
>>         host->pm_flags = 0;
>> @@ -2852,13 +2845,9 @@ static int __init mmc_init(void)
>>  {
>>         int ret;
>>
>> -       workqueue = alloc_ordered_workqueue("kmmcd", 0);
>> -       if (!workqueue)
>> -               return -ENOMEM;
>> -
>>         ret = mmc_register_bus();
>>         if (ret)
>> -               goto destroy_workqueue;
>> +               return ret;
>>
>>         ret = mmc_register_host_class();
>>         if (ret)
>> @@ -2874,9 +2863,6 @@ unregister_host_class:
>>         mmc_unregister_host_class();
>>  unregister_bus:
>>         mmc_unregister_bus();
>> -destroy_workqueue:
>> -       destroy_workqueue(workqueue);
>> -
>>         return ret;
>>  }
>>
>> @@ -2885,7 +2871,6 @@ static void __exit mmc_exit(void)
>>         sdio_unregister_bus();
>>         mmc_unregister_host_class();
>>         mmc_unregister_bus();
>> -       destroy_workqueue(workqueue);
>>  }
>>
>>  subsys_initcall(mmc_init);
>> --
>> 1.9.1
>>
> --
> 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
Ulf Hansson Dec. 16, 2015, 9:07 a.m. UTC | #3
On 16 December 2015 at 00:22, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 12/15/2015 12:50 AM, Ulf Hansson wrote:
>> On 1 December 2015 at 16:15, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> The mmc workqueue is an ordered workqueue, allowing only one work to
>>> execute per given time. As this workqueue is used for card detection, the
>>> conseqeunce is that cards will be detected one by one waiting for each
>>> other.
>>>
>>> Moreover, most of the time spent during card initialization is waiting for
>>> the card's internal firmware to be ready. From a CPU perspective this
>>> typically means waiting for a completion variable to be kicked via an
>>> IRQ-handler or waiting for a sleep timer to finish.
>>>
>>> This behaviour of detecting/initializing cards is sub-optimal, especially
>>> for SOCs having several controllers/cards.
>>>
>>> Let's convert to use the system_freezable_wq for the mmc detect works.
>>> This enables several works to be executed simultaneously and thus also
>>> cards to be detected like so.
>>>
>>> Tests on UX500, which holds two eMMC cards and an SD-card (actually also
>>> an SDIO card, currently not detected), shows a significant improved
>>> behaviour due to this change.
>>>
>>> Before this change, both the eMMC cards waited for the SD card to be
>>> initialized as its detect work entered the workqueue first. In some cases,
>>> depending on the characteristic of the SD-card, they got delayed 1-1.5 s.
>>>
>>> Additionally for the second eMMC, it needed to wait for the first eMMC to
>>> be initialized which added another 120-190 ms.
>
> how did you check this? I also want to test with similar environment. :)

To find the improved card detection times, I just used the kernel boot
log and checked the time stamps for when the cards get detected. I ran
the tests ~10 times and picked up some average values.

To notice the improvement you need at least two cards to be inserted
during boot. Perhaps you have a platform with one eMMC and one SD card
slot, you can test.

>
>>>
>>> Converting to the system_freezable_wq, removed these delays and made both
>>> the eMMC cards available far earlier in the boot sequence.
>>>
>>> Selecting the system_freezable_wq, in favour of for example the system_wq,
>>> is because we need card detection mechanism to be disabled once userspace
>>> are frozen during system PM. Currently the mmc core deal with this via PM
>>> notifiers, but following patches may utilize the behaviour of the
>>> system_freezable_wq, to simplify the use of the PM notifiers.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> Even-though the lack of reviewers, I have decided to give this a try
>> and pushed it for my next branch.
>>
>> If it isn't clear by the above change-log:
>> A side effect from this change is that mmc block partitions, may end
>> up being given other indexes than they had before. Whether that may
>> cause issues, depends on if the path to the rootfs have been
>> hard-coded or not.
>
> What does "mmc block partitions, may end up being given other indexes than they had before." mean?
> Is it possible to increase the index for partition number?
> If my understanding is right...mmcblk's partition index is not fixed.
> Sometime eMMC should be mmcblk0pX or mmcblk1px..right? hmm...

You have understood correctly. The mmcblk[n], can get different values
for n, depending in which order the cards gets detected.

>
> I need to check more...with other boards and SoC.

Sounds great, thanks for helping out!

Kind regards
Uffe
--
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
Alan Cooper Dec. 17, 2015, 7:47 p.m. UTC | #4
On Wed, Dec 16, 2015 at 4:07 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> To find the improved card detection times, I just used the kernel boot
> log and checked the time stamps for when the cards get detected. I ran
> the tests ~10 times and picked up some average values.
>
> To notice the improvement you need at least two cards to be inserted
> during boot. Perhaps you have a platform with one eMMC and one SD card
> slot, you can test.

I tested the patch on one of our ARM bases SoC's. The SoC has an SD
card slot on the first controller and an eMMC device on the second
controller. I found ~100ms improvement on the time it took for the
rootfs on the eMMC device to be ready for use. Interestingly, I did
see the SD/eMMC order swap once in 20 tries. This is not an issue for
us as we already have to use UUID for the eMMC rootfs in case an SD
card is installed.

Al
--
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
Ulf Hansson Dec. 17, 2015, 9:49 p.m. UTC | #5
On 17 December 2015 at 20:47, Alan Cooper <alcooperx@gmail.com> wrote:
> On Wed, Dec 16, 2015 at 4:07 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> To find the improved card detection times, I just used the kernel boot
>> log and checked the time stamps for when the cards get detected. I ran
>> the tests ~10 times and picked up some average values.
>>
>> To notice the improvement you need at least two cards to be inserted
>> during boot. Perhaps you have a platform with one eMMC and one SD card
>> slot, you can test.
>
> I tested the patch on one of our ARM bases SoC's. The SoC has an SD
> card slot on the first controller and an eMMC device on the second
> controller. I found ~100ms improvement on the time it took for the
> rootfs on the eMMC device to be ready for use. Interestingly, I did
> see the SD/eMMC order swap once in 20 tries. This is not an issue for
> us as we already have to use UUID for the eMMC rootfs in case an SD
> card is installed.
>
> Al

Al, thanks for your effort in testing. I assume I can apply a
tested-by tag for that!

BTW, if you want to see an even better improvement, just find a
crappier SD card as I guess you have a quite good one. :-)

Kind regards
Uffe
--
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
Jaehoon Chung Dec. 18, 2015, 1:28 a.m. UTC | #6
Hi,

On 12/18/2015 06:49 AM, Ulf Hansson wrote:
> On 17 December 2015 at 20:47, Alan Cooper <alcooperx@gmail.com> wrote:
>> On Wed, Dec 16, 2015 at 4:07 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> To find the improved card detection times, I just used the kernel boot
>>> log and checked the time stamps for when the cards get detected. I ran
>>> the tests ~10 times and picked up some average values.
>>>
>>> To notice the improvement you need at least two cards to be inserted
>>> during boot. Perhaps you have a platform with one eMMC and one SD card
>>> slot, you can test.
>>
>> I tested the patch on one of our ARM bases SoC's. The SoC has an SD
>> card slot on the first controller and an eMMC device on the second
>> controller. I found ~100ms improvement on the time it took for the
>> rootfs on the eMMC device to be ready for use. Interestingly, I did
>> see the SD/eMMC order swap once in 20 tries. This is not an issue for
>> us as we already have to use UUID for the eMMC rootfs in case an SD
>> card is installed.
>>
>> Al
> 
> Al, thanks for your effort in testing. I assume I can apply a
> tested-by tag for that!
> 
> BTW, if you want to see an even better improvement, just find a
> crappier SD card as I guess you have a quite good one. :-)

I didn't test with this yet..actually, i have tested with only one board(Exynos4412).
There is no the changing index. (Not too much for testing)
but in my experiment, our platform(Tizen) should be produced the problem in SD-card case.
I need to check with SystemFW guys. I will check this..ASAP. 

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> --
> 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
Shawn Lin Dec. 18, 2015, 3:05 a.m. UTC | #7
On 2015/12/18 9:28, Jaehoon Chung wrote:
> Hi,
>
> On 12/18/2015 06:49 AM, Ulf Hansson wrote:
>> On 17 December 2015 at 20:47, Alan Cooper <alcooperx@gmail.com> wrote:
>>> On Wed, Dec 16, 2015 at 4:07 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> To find the improved card detection times, I just used the kernel boot
>>>> log and checked the time stamps for when the cards get detected. I ran
>>>> the tests ~10 times and picked up some average values.
>>>>
>>>> To notice the improvement you need at least two cards to be inserted
>>>> during boot. Perhaps you have a platform with one eMMC and one SD card
>>>> slot, you can test.
>>>
>>> I tested the patch on one of our ARM bases SoC's. The SoC has an SD
>>> card slot on the first controller and an eMMC device on the second
>>> controller. I found ~100ms improvement on the time it took for the
>>> rootfs on the eMMC device to be ready for use. Interestingly, I did
>>> see the SD/eMMC order swap once in 20 tries. This is not an issue for
>>> us as we already have to use UUID for the eMMC rootfs in case an SD
>>> card is installed.
>>>

hi Ulf,
     I'm interested in this patch since I also spent a little time 
optimzing the boot time and sequence for my local branch based on kernel 
3.1x. With you patch, it seems, the tablet only achieves very small 
improvement for the SD controller(less than 20ms).

My test environment:
Kernel: 3.14
Slot: 1 emmc(1st) + 1 SD card(2nd) + 1 sdio card(3rd)

I happen to run massive reboot test for my stable kernel branch this 
weekend, so I will apply this patch to see if any problem occurs. :)



>>> Al
>>
>> Al, thanks for your effort in testing. I assume I can apply a
>> tested-by tag for that!
>>
>> BTW, if you want to see an even better improvement, just find a
>> crappier SD card as I guess you have a quite good one. :-)
>
> I didn't test with this yet..actually, i have tested with only one board(Exynos4412).
> There is no the changing index. (Not too much for testing)
> but in my experiment, our platform(Tizen) should be produced the problem in SD-card case.
> I need to check with SystemFW guys. I will check this..ASAP.
>
> Best Regards,
> Jaehoon Chung
>
>>
>> Kind regards
>> Uffe
>> --
>> 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
>
>
>
Ulf Hansson Dec. 28, 2015, 9:31 a.m. UTC | #8
On 18 December 2015 at 04:05, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> On 2015/12/18 9:28, Jaehoon Chung wrote:
>>
>> Hi,
>>
>> On 12/18/2015 06:49 AM, Ulf Hansson wrote:
>>>
>>> On 17 December 2015 at 20:47, Alan Cooper <alcooperx@gmail.com> wrote:
>>>>
>>>> On Wed, Dec 16, 2015 at 4:07 AM, Ulf Hansson <ulf.hansson@linaro.org>
>>>> wrote:
>>>>>
>>>>> To find the improved card detection times, I just used the kernel boot
>>>>> log and checked the time stamps for when the cards get detected. I ran
>>>>> the tests ~10 times and picked up some average values.
>>>>>
>>>>> To notice the improvement you need at least two cards to be inserted
>>>>> during boot. Perhaps you have a platform with one eMMC and one SD card
>>>>> slot, you can test.
>>>>
>>>>
>>>> I tested the patch on one of our ARM bases SoC's. The SoC has an SD
>>>> card slot on the first controller and an eMMC device on the second
>>>> controller. I found ~100ms improvement on the time it took for the
>>>> rootfs on the eMMC device to be ready for use. Interestingly, I did
>>>> see the SD/eMMC order swap once in 20 tries. This is not an issue for
>>>> us as we already have to use UUID for the eMMC rootfs in case an SD
>>>> card is installed.
>>>>
>

Hi Shawn,

Thanks for helping out testing.

> hi Ulf,
>     I'm interested in this patch since I also spent a little time optimzing
> the boot time and sequence for my local branch based on kernel 3.1x. With
> you patch, it seems, the tablet only achieves very small improvement for the
> SD controller(less than 20ms).
>
> My test environment:
> Kernel: 3.14
> Slot: 1 emmc(1st) + 1 SD card(2nd) + 1 sdio card(3rd)

How much time you *gain* comparing to the original behaviour for each
card, depends on the ->probe() order of your mmc hosts.

I would guess that the host that holds the eMMC is the one being
probed first, right?
In general eMMC is quite fast to initialize, comparing to SD-cards and
that's probably the reason to why you don't see that much
improvements.

Although, for your SDIO card you should see a greater improvement as
this don't need to wait for your SD and eMMC to be initialized.

>
> I happen to run massive reboot test for my stable kernel branch this
> weekend, so I will apply this patch to see if any problem occurs. :)
>

Great!

As you haven't reported any problems, I suppose the tests went well!?

Kind regards
Uffe
--
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
Shawn Lin Dec. 28, 2015, 9:44 a.m. UTC | #9
On 2015/12/28 17:31, Ulf Hansson wrote:
> On 18 December 2015 at 04:05, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> On 2015/12/18 9:28, Jaehoon Chung wrote:
>>>
>>> Hi,
>>>
>>> On 12/18/2015 06:49 AM, Ulf Hansson wrote:
>>>>
>>>> On 17 December 2015 at 20:47, Alan Cooper <alcooperx@gmail.com> wrote:
>>>>>
>>>>> On Wed, Dec 16, 2015 at 4:07 AM, Ulf Hansson <ulf.hansson@linaro.org>
>>>>> wrote:
>>>>>>
>>>>>> To find the improved card detection times, I just used the kernel boot
>>>>>> log and checked the time stamps for when the cards get detected. I ran
>>>>>> the tests ~10 times and picked up some average values.
>>>>>>
>>>>>> To notice the improvement you need at least two cards to be inserted
>>>>>> during boot. Perhaps you have a platform with one eMMC and one SD card
>>>>>> slot, you can test.
>>>>>
>>>>>
>>>>> I tested the patch on one of our ARM bases SoC's. The SoC has an SD
>>>>> card slot on the first controller and an eMMC device on the second
>>>>> controller. I found ~100ms improvement on the time it took for the
>>>>> rootfs on the eMMC device to be ready for use. Interestingly, I did
>>>>> see the SD/eMMC order swap once in 20 tries. This is not an issue for
>>>>> us as we already have to use UUID for the eMMC rootfs in case an SD
>>>>> card is installed.
>>>>>
>>
>
> Hi Shawn,
>
> Thanks for helping out testing.
>
>> hi Ulf,
>>      I'm interested in this patch since I also spent a little time optimzing
>> the boot time and sequence for my local branch based on kernel 3.1x. With
>> you patch, it seems, the tablet only achieves very small improvement for the
>> SD controller(less than 20ms).
>>
>> My test environment:
>> Kernel: 3.14
>> Slot: 1 emmc(1st) + 1 SD card(2nd) + 1 sdio card(3rd)
>
> How much time you *gain* comparing to the original behaviour for each
> card, depends on the ->probe() order of your mmc hosts.
>
> I would guess that the host that holds the eMMC is the one being
> probed first, right?
> In general eMMC is quite fast to initialize, comparing to SD-cards and
> that's probably the reason to why you don't see that much
> improvements.
>

yes, my emmc is the first order to be probed. And sdio gain nearly 80ms 
with your patch.

No any problems found yet. So I think this patch is good :)

> Although, for your SDIO card you should see a greater improvement as
> this don't need to wait for your SD and eMMC to be initialized.
>
>>
>> I happen to run massive reboot test for my stable kernel branch this
>> weekend, so I will apply this patch to see if any problem occurs. :)
>>
>
> Great!
>
> As you haven't reported any problems, I suppose the tests went well!?
>
> Kind regards
> Uffe
> --
> 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/core/core.c b/drivers/mmc/core/core.c
index 910aa25..f95d41f 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -55,7 +55,6 @@ 
  */
 #define MMC_BKOPS_MAX_TIMEOUT	(4 * 60 * 1000) /* max time to wait in ms */
 
-static struct workqueue_struct *workqueue;
 static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
 
 /*
@@ -66,21 +65,16 @@  static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
 bool use_spi_crc = 1;
 module_param(use_spi_crc, bool, 0);
 
-/*
- * Internal function. Schedule delayed work in the MMC work queue.
- */
 static int mmc_schedule_delayed_work(struct delayed_work *work,
 				     unsigned long delay)
 {
-	return queue_delayed_work(workqueue, work, delay);
-}
-
-/*
- * Internal function. Flush all scheduled work from the MMC work queue.
- */
-static void mmc_flush_scheduled_work(void)
-{
-	flush_workqueue(workqueue);
+	/*
+	 * We use the system_freezable_wq, because of two reasons.
+	 * First, it allows several works (not the same work item) to be
+	 * executed simultaneously. Second, the queue becomes frozen when
+	 * userspace becomes frozen during system PM.
+	 */
+	return queue_delayed_work(system_freezable_wq, work, delay);
 }
 
 #ifdef CONFIG_FAIL_MMC_REQUEST
@@ -2669,7 +2663,6 @@  void mmc_stop_host(struct mmc_host *host)
 
 	host->rescan_disable = 1;
 	cancel_delayed_work_sync(&host->detect);
-	mmc_flush_scheduled_work();
 
 	/* clear pm flags now and let card drivers set them as needed */
 	host->pm_flags = 0;
@@ -2852,13 +2845,9 @@  static int __init mmc_init(void)
 {
 	int ret;
 
-	workqueue = alloc_ordered_workqueue("kmmcd", 0);
-	if (!workqueue)
-		return -ENOMEM;
-
 	ret = mmc_register_bus();
 	if (ret)
-		goto destroy_workqueue;
+		return ret;
 
 	ret = mmc_register_host_class();
 	if (ret)
@@ -2874,9 +2863,6 @@  unregister_host_class:
 	mmc_unregister_host_class();
 unregister_bus:
 	mmc_unregister_bus();
-destroy_workqueue:
-	destroy_workqueue(workqueue);
-
 	return ret;
 }
 
@@ -2885,7 +2871,6 @@  static void __exit mmc_exit(void)
 	sdio_unregister_bus();
 	mmc_unregister_host_class();
 	mmc_unregister_bus();
-	destroy_workqueue(workqueue);
 }
 
 subsys_initcall(mmc_init);