[PATCH] wait while adding MMC host to ensure root mounts

[PATCH] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

MMC hosts are added asynchronously. We need to wait until detect returns to
avoid failed root filesystem mounts.
---8<---
VFS: Cannot open root device "mmcblk0p1" or unknown-block(0,0): error -6
Please append a correct "root=" boot option; here are the available partitions:
mmc0: host does not support reading read-only switch. assuming write-enable.
1f00             256 mtdblock0  (driver?)
1f01             256 mtdblock1  (driver?)
1f02            2560 mtdblock2 mmc0: new SDHC card at address b368
 (driver?)
1f03           29696 mtdblock3  (driver?)
1f04           16384 mtdblock4 mmcblk0: mmc0:b368 USD   3.72 GiB
 (driver?)
 mmcblk0: p1
b300         3910656 mmcblk0  driver: mmcblk
  b301         3906560 mmcblk0p1 00000000-01
Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
---8<---

Signed-off-by: Sergey Yanovich <ynvich@gmail.com>
---
 drivers/base/dd.c       |    1 -
 drivers/mmc/core/core.c |    4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 218f1e6..61d3e1b 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -345,7 +345,6 @@ int driver_probe_done(void)
 void wait_for_device_probe(void)
 {
 	/* wait for the known devices to complete their probing */
-
 	wait_event(probe_waitqueue, atomic_read(&probe_count) == 0);
 	async_synchronize_full();
 }
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index aaed768..9790323 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -91,10 +91,11 @@ static int mmc_schedule_delayed_work(struct delayed_work *work,
 /*
  * Internal function. Flush all scheduled work from the MMC work queue.
  */
-static void mmc_flush_scheduled_work(void)
+void mmc_flush_scheduled_work(void)
 {
 	flush_workqueue(workqueue);
 }
+EXPORT_SYMBOL(mmc_flush_scheduled_work);
 
 #ifdef CONFIG_FAIL_MMC_REQUEST
 
@@ -2225,6 +2226,7 @@ void mmc_start_host(struct mmc_host *host)
 	host->rescan_disable = 0;
 	mmc_power_up(host);
 	mmc_detect_change(host, 0);
+	mmc_flush_scheduled_work();
 }
 
 void mmc_stop_host(struct mmc_host *host)
-- 
1.7.10.4

[PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

MMC hosts are added asynchronously. We need to wait until detect returns to
avoid failed root filesystem mounts.
---8<---
VFS: Cannot open root device "mmcblk0p1" or unknown-block(0,0): error -6
Please append a correct "root=" boot option; here are the available partitions:
mmc0: host does not support reading read-only switch. assuming write-enable.
1f00             256 mtdblock0  (driver?)
1f01             256 mtdblock1  (driver?)
1f02            2560 mtdblock2 mmc0: new SDHC card at address b368
 (driver?)
1f03           29696 mtdblock3  (driver?)
1f04           16384 mtdblock4 mmcblk0: mmc0:b368 USD   3.72 GiB
 (driver?)
 mmcblk0: p1
b300         3910656 mmcblk0  driver: mmcblk
  b301         3906560 mmcblk0p1 00000000-01
Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
---8<---

Signed-off-by: Sergey Yanovich <ynvich@gmail.com>
---
changes for v2:
- removed exporting as symbol is in the same file

 drivers/mmc/core/core.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index aaed768..7196888 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2225,6 +2225,7 @@ void mmc_start_host(struct mmc_host *host)
 	host->rescan_disable = 0;
 	mmc_power_up(host);
 	mmc_detect_change(host, 0);
+	mmc_flush_scheduled_work();
 }
 
 void mmc_stop_host(struct mmc_host *host)
-- 
1.7.10.4

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Namjae Jeon]

2013/3/14, Sergey Yanovich <ynvich@gmail.com>:
> MMC hosts are added asynchronously. We need to wait until detect returns to
> avoid failed root filesystem mounts.
> ---8<---
> VFS: Cannot open root device "mmcblk0p1" or unknown-block(0,0): error -6
> Please append a correct "root=" boot option; here are the available
> partitions:
> mmc0: host does not support reading read-only switch. assuming
> write-enable.
> 1f00             256 mtdblock0  (driver?)
> 1f01             256 mtdblock1  (driver?)
> 1f02            2560 mtdblock2 mmc0: new SDHC card at address b368
>  (driver?)
> 1f03           29696 mtdblock3  (driver?)
> 1f04           16384 mtdblock4 mmcblk0: mmc0:b368 USD   3.72 GiB
>  (driver?)
>  mmcblk0: p1
> b300         3910656 mmcblk0  driver: mmcblk
>   b301         3906560 mmcblk0p1 00000000-01
> Kernel panic - not syncing: VFS: Unable to mount root fs on
> unknown-block(0,0)
> ---8<---
>
> Signed-off-by: Sergey Yanovich <ynvich@gmail.com>
> ---
Hi.
Have you ever tried to use rootwait or rootdelay on command line ?
If no, You can use them.

Thanks.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On 14/03/13 08:08, Namjae Jeon wrote:> 2013/3/14, Sergey Yanovich <ynvich@gmail.com>:
>> Kernel panic - not syncing: VFS: Unable to mount root fs on
>> unknown-block(0,0)

> Have you ever tried to use rootwait or rootdelay on command line ?
> If no, You can use them.

Those options work. However, they introduce a delay in the range of hundreds milliseconds and seconds respectively. They delay is not required. If a cards is present, mmc_rescan will return synchronously with card initialized.

prepare_namespace() uses wait_for_device_probe(). The latter assumes that all "known devices" have initialized by the time it returns. MMC is not currently delivering on the assumptions. It will with the patch applied.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Jaehoon Chung]

On 03/14/2013 01:08 PM, Namjae Jeon wrote:
> 2013/3/14, Sergey Yanovich <ynvich@gmail.com>:
>> MMC hosts are added asynchronously. We need to wait until detect returns to
>> avoid failed root filesystem mounts.
>> ---8<---
>> VFS: Cannot open root device "mmcblk0p1" or unknown-block(0,0): error -6
>> Please append a correct "root=" boot option; here are the available
>> partitions:
>> mmc0: host does not support reading read-only switch. assuming
>> write-enable.
>> 1f00             256 mtdblock0  (driver?)
>> 1f01             256 mtdblock1  (driver?)
>> 1f02            2560 mtdblock2 mmc0: new SDHC card at address b368
>>  (driver?)
>> 1f03           29696 mtdblock3  (driver?)
>> 1f04           16384 mtdblock4 mmcblk0: mmc0:b368 USD   3.72 GiB
>>  (driver?)
>>  mmcblk0: p1
>> b300         3910656 mmcblk0  driver: mmcblk
>>   b301         3906560 mmcblk0p1 00000000-01
>> Kernel panic - not syncing: VFS: Unable to mount root fs on
>> unknown-block(0,0)
>> ---8<---
>>
>> Signed-off-by: Sergey Yanovich <ynvich@gmail.com>
>> ---
> Hi.
> Have you ever tried to use rootwait or rootdelay on command line ?
> If no, You can use them.
I agreed the Namjae's suggestion..if you use the rootwait or rootdelay, it's waiting for mounting rootfs.

Best Regards,
Jaehoon Chung
> 
> Thanks.
> 

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Chris Ball]

Hi Sergey,

On Wed, Mar 13 2013, Sergey Yanovich wrote:
> MMC hosts are added asynchronously. We need to wait until detect returns to
> avoid failed root filesystem mounts.
> ---8<---
> VFS: Cannot open root device "mmcblk0p1" or unknown-block(0,0): error -6
> Please append a correct "root=" boot option; here are the available partitions:
> mmc0: host does not support reading read-only switch. assuming write-enable.
> 1f00             256 mtdblock0  (driver?)
> 1f01             256 mtdblock1  (driver?)
> 1f02            2560 mtdblock2 mmc0: new SDHC card at address b368
>  (driver?)
> 1f03           29696 mtdblock3  (driver?)
> 1f04           16384 mtdblock4 mmcblk0: mmc0:b368 USD   3.72 GiB
>  (driver?)
>  mmcblk0: p1
> b300         3910656 mmcblk0  driver: mmcblk
>   b301         3906560 mmcblk0p1 00000000-01
> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
> ---8<---
>
> Signed-off-by: Sergey Yanovich <ynvich@gmail.com>
> ---
> changes for v2:
> - removed exporting as symbol is in the same file
>
>  drivers/mmc/core/core.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index aaed768..7196888 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2225,6 +2225,7 @@ void mmc_start_host(struct mmc_host *host)
>  	host->rescan_disable = 0;
>  	mmc_power_up(host);
>  	mmc_detect_change(host, 0);
> +	mmc_flush_scheduled_work();
>  }
>  
>  void mmc_stop_host(struct mmc_host *host)

Thanks, this looks okay to me, I've pushed it to mmc-next for 3.10.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Ulf Hansson]

On 22 March 2013 18:03, Chris Ball <cjb@laptop.org> wrote:
> Hi Sergey,
>
> On Wed, Mar 13 2013, Sergey Yanovich wrote:
>> MMC hosts are added asynchronously. We need to wait until detect returns to
>> avoid failed root filesystem mounts.
>> ---8<---
>> VFS: Cannot open root device "mmcblk0p1" or unknown-block(0,0): error -6
>> Please append a correct "root=" boot option; here are the available partitions:
>> mmc0: host does not support reading read-only switch. assuming write-enable.
>> 1f00             256 mtdblock0  (driver?)
>> 1f01             256 mtdblock1  (driver?)
>> 1f02            2560 mtdblock2 mmc0: new SDHC card at address b368
>>  (driver?)
>> 1f03           29696 mtdblock3  (driver?)
>> 1f04           16384 mtdblock4 mmcblk0: mmc0:b368 USD   3.72 GiB
>>  (driver?)
>>  mmcblk0: p1
>> b300         3910656 mmcblk0  driver: mmcblk
>>   b301         3906560 mmcblk0p1 00000000-01
>> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
>> ---8<---
>>
>> Signed-off-by: Sergey Yanovich <ynvich@gmail.com>
>> ---
>> changes for v2:
>> - removed exporting as symbol is in the same file
>>
>>  drivers/mmc/core/core.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index aaed768..7196888 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -2225,6 +2225,7 @@ void mmc_start_host(struct mmc_host *host)
>>       host->rescan_disable = 0;
>>       mmc_power_up(host);
>>       mmc_detect_change(host, 0);
>> +     mmc_flush_scheduled_work();
>>  }
>>
>>  void mmc_stop_host(struct mmc_host *host)
>
> Thanks, this looks okay to me, I've pushed it to mmc-next for 3.10.
>
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child

Hi Chris,

I noticed you merged this. I thought the idea was to use the rootwait
or rootdelay?

Moreover, this patch will have bad impact on booting the kernel, since
every host device that has scheduled a detect work from it's probe
function will also wait for it to finish. Even if it is the boot
device of not. If this is needed, I would prefer that a host cap is
used.

Kind regards
Ulf Hansson

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Chris Ball]

Hi,

On Wed, Mar 27 2013, Ulf Hansson wrote:
> I noticed you merged this. I thought the idea was to use the rootwait
> or rootdelay?

That's necessary before the patch, but it would be better if we didn't
have to pass rootwait, all else being equal.

> Moreover, this patch will have bad impact on booting the kernel, since
> every host device that has scheduled a detect work from it's probe
> function will also wait for it to finish. Even if it is the boot
> device of not. If this is needed, I would prefer that a host cap is
> used.

I see, you're worried about a performance regression where every boot
takes longer than it used to while MMC quiesces.  That's fair.  Do you
think you could tell me how much delay this adds to a boot for you, so
that we can consider whether the usability improvement is worth it?

If the delay's significant, I agree with you and will revert this patch.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Adrian Hunter]

On 27/03/13 13:57, Chris Ball wrote:
> Hi,
> 
> On Wed, Mar 27 2013, Ulf Hansson wrote:
>> I noticed you merged this. I thought the idea was to use the rootwait
>> or rootdelay?
> 
> That's necessary before the patch, but it would be better if we didn't
> have to pass rootwait, all else being equal.
> 
>> Moreover, this patch will have bad impact on booting the kernel, since
>> every host device that has scheduled a detect work from it's probe
>> function will also wait for it to finish. Even if it is the boot
>> device of not. If this is needed, I would prefer that a host cap is
>> used.
> 
> I see, you're worried about a performance regression where every boot
> takes longer than it used to while MMC quiesces.  That's fair.  Do you
> think you could tell me how much delay this adds to a boot for you, so
> that we can consider whether the usability improvement is worth it?
> 
> If the delay's significant, I agree with you and will revert this patch.

On my system it is significant:

Before the patch:

[    1.625623] VFS: Mounted root (ext4 filesystem) readonly on device 179:2.

After the patch:

[    1.935851] VFS: Mounted root (ext4 filesystem) readonly on device 179:2.

That is an addition of 310 ms which is 19% performance degradation.

Please revert the patch.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On Tue, 2013-04-02 at 16:36 +0300, Adrian Hunter wrote:
> On my system it is significant:
> 
> Before the patch:
> 
> [    1.625623] VFS: Mounted root (ext4 filesystem) readonly on device 179:2.
> 
> After the patch:
> 
> [    1.935851] VFS: Mounted root (ext4 filesystem) readonly on device 179:2.
> 
> That is an addition of 310 ms which is 19% performance degradation.

Are you sure the delay is caused by mmc?

On my intel laptop (userspace is Debian/unstable):
[    1.542339] EXT4-fs (sda2): mounted filesystem with ordered data mode. Opts: (null)
...
[    2.735851] mmc0: new high speed SD card at address e624
[    2.742289] mmcblk0: mmc0:e624 SU02G 1.84 GiB 
[    2.752317]  mmcblk0: p1

> Please revert the patch.

Chris, could provide a pointer on how to improve the patch?

Maybe introduce mmc_is_hosting_root() and do something like:

-	mmc_flush_scheduled_work();
+	if (mmc_is_hosting_root())
+		mmc_flush_scheduled_work();

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Adrian Hunter]

On 02/04/13 17:24, Sergey Yanovich wrote:
> On Tue, 2013-04-02 at 16:36 +0300, Adrian Hunter wrote:
>> On my system it is significant:
>>
>> Before the patch:
>>
>> [    1.625623] VFS: Mounted root (ext4 filesystem) readonly on device 179:2.
>>
>> After the patch:
>>
>> [    1.935851] VFS: Mounted root (ext4 filesystem) readonly on device 179:2.
>>
>> That is an addition of 310 ms which is 19% performance degradation.
> 
> Are you sure the delay is caused by mmc?

Yes

> 
> On my intel laptop (userspace is Debian/unstable):
> [    1.542339] EXT4-fs (sda2): mounted filesystem with ordered data mode. Opts: (null)
> ...
> [    2.735851] mmc0: new high speed SD card at address e624
> [    2.742289] mmcblk0: mmc0:e624 SU02G 1.84 GiB 
> [    2.752317]  mmcblk0: p1


No,  I am booting from eMMC.

> 
>> Please revert the patch.
> 
> Chris, could provide a pointer on how to improve the patch?
> 
> Maybe introduce mmc_is_hosting_root() and do something like:
> 
> -	mmc_flush_scheduled_work();
> +	if (mmc_is_hosting_root())
> +		mmc_flush_scheduled_work();

No, I am booting from eMMC.  Perhaps a host capability:

	if (host->caps2 & MMC_CAP2_ROOTWAIT)
		mmc_flush_scheduled_work();

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On Thu, 2013-04-04 at 09:35 +0300, Adrian Hunter wrote:
> No,  I am booting from eMMC.

Well, in this case you should be aware, that your system is not
concurrency-safe without the patch. It may or may not boot each time
depending on the large number of factors.

> > Maybe introduce mmc_is_hosting_root() and do something like:
> > 
> > -	mmc_flush_scheduled_work();
> > +	if (mmc_is_hosting_root())
> > +		mmc_flush_scheduled_work();
> 
> No, I am booting from eMMC.  Perhaps a host capability:
> 
> 	if (host->caps2 & MMC_CAP2_ROOTWAIT)
> 		mmc_flush_scheduled_work();
> 

Neither my variant, nor yours will help to handle the increased boot
time.

The root cause is that probing several devices is done sequentially and
mmc was reporting end of its probing before it was actually happening.
My patch makes mmc report end of probing on-time. The correct way to fix
the additional delay, my patch introduces, is to rewrite the probing to
be parallel instead of sequential. I understand that it is much easier
just to revert the patch.

If the patch is reverted, something like this somewhere in
'init/do_mounts.c' could conditionally activate 'root_wait':

	if (mmc_is_hosting_root())
		root_wait = 1;

IMHO this is wrong and my patch is right, but better this than broken
mmc boot.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Adrian Hunter]

On 04/04/13 13:59, Sergey Yanovich wrote:
> On Thu, 2013-04-04 at 09:35 +0300, Adrian Hunter wrote:
>> No,  I am booting from eMMC.
> 
> Well, in this case you should be aware, that your system is not
> concurrency-safe without the patch. It may or may not boot each time
> depending on the large number of factors.

Not true.  You know nothing of my boot time characteristics.

> 
>>> Maybe introduce mmc_is_hosting_root() and do something like:
>>>
>>> -	mmc_flush_scheduled_work();
>>> +	if (mmc_is_hosting_root())
>>> +		mmc_flush_scheduled_work();
>>
>> No, I am booting from eMMC.  Perhaps a host capability:
>>
>> 	if (host->caps2 & MMC_CAP2_ROOTWAIT)
>> 		mmc_flush_scheduled_work();
>>
> 
> Neither my variant, nor yours will help to handle the increased boot
> time.

Not true.  I would not set MMC_CAP2_ROOTWAIT.

> The root cause is that probing several devices is done sequentially and
> mmc was reporting end of its probing before it was actually happening.

Not true.  The probe of the MMC Host Controller finishes when the host
controller is initialized.

> My patch makes mmc report end of probing on-time. The correct way to fix
> the additional delay, my patch introduces, is to rewrite the probing to
> be parallel instead of sequential. I understand that it is much easier
> just to revert the patch.
> 
> If the patch is reverted, something like this somewhere in
> 'init/do_mounts.c' could conditionally activate 'root_wait':
> 
> 	if (mmc_is_hosting_root())
> 		root_wait = 1;
> 
> IMHO this is wrong and my patch is right, but better this than broken
> mmc boot.

No.  Your patch is not right for my platform.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On Thu, 2013-04-04 at 14:35 +0300, Adrian Hunter wrote:
> On 04/04/13 13:59, Sergey Yanovich wrote:
> > On Thu, 2013-04-04 at 09:35 +0300, Adrian Hunter wrote:
> >> No,  I am booting from eMMC.
> > 
> > Well, in this case you should be aware, that your system is not
> > concurrency-safe without the patch. It may or may not boot each time
> > depending on the large number of factors.
> 
> Not true.  You know nothing of my boot time characteristics.

Well, I assumed you use linux kernel recent enough to include the patch
in question. This assumption may well be wrong.

Could you please elaborate how you avoid a situation which brought up
this patch ("all probes finish and mmc card is not ready")?

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On Wed, 2013-03-27 at 07:57 -0400, Chris Ball wrote:
> If the delay's significant, I agree with you and will revert this patch.

The patch was reverted. The problem is back. Filed bug:
https://bugzilla.kernel.org/show_bug.cgi?id=56561

A possible solution is to make card a separate device (now only the host
is a device). In this case, the probing could be done asynchronously
without breaking the assumption in prepare_namespace().

Chris, could you comment?

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Chris Ball]

Hi,

On Sat, Apr 13 2013, Sergey Yanovich wrote:
> On Wed, 2013-03-27 at 07:57 -0400, Chris Ball wrote:
>> If the delay's significant, I agree with you and will revert this patch.
>
> The patch was reverted. The problem is back. Filed bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=56561
>
> A possible solution is to make card a separate device (now only the host
> is a device). In this case, the probing could be done asynchronously
> without breaking the assumption in prepare_namespace().
>
> Chris, could you comment?

The same problem (of requiring rootwait) exists when trying to boot
from a SCSI/USB device, so I don't think there's any MMC-specific
problem here.  Most people who aren't using an initrd have to supply
rootwait.

I liked the idea behind your patch, but the performance regression
isn't acceptable.  If we can't find a way to conditionalize the call
to mmc_flush_scheduled_work() on being in a situation where we're
definitely about to panic, I think the only option would be to try
to come up with a solution at the layers above MMC.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On Sat, 2013-04-13 at 13:56 +0200, Ulf Hansson wrote:
> 
> Den 13 apr 2013 12:49 skrev "Sergey Yanovich" <ynvich@gmail.com>:
> > A possible solution is to make card a separate device (now only the
> host
> > is a device). In this case, the probing could be done asynchronously
> > without breaking the assumption in prepare_namespace().
> 
> Actually the card is already a separate device, which I connected to
> the mmc bus.

Great. I failed to see that.

> It is an interesting idea, but the problem is that the card device is
> only created and added on the fly when a card has been properly
> detected.

If we include 'detecting' in 'probing' and then do 'probing'
asynchronously, we will neither have race condition, nor boot time
regression.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Сергей Янович]

Hi Ulf,

On 27 March 2013 15:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Moreover, this patch will have bad impact on booting the kernel, since
> every host device that has scheduled a detect work from it's probe
> function will also wait for it to finish. Even if it is the boot
> device of not. If this is needed, I would prefer that a host cap is
> used.

Do you have any profiling data supporting bad impact on boot?

It should be it in the range of dozens us, if any. Without the patch,
approx. 30% of boots succeeded with CONFIG_PREEMT and aprox. 95%
without CONFIG_PREEMT. mmc_rescan is just a few instructions, if there
is no card present. On boot and with a card present, it might only
sleep in the host implementation.

Anyway, something had to be done about mmc boot. rootdelay will not
report error if the card is absent or its partition table is damaged.
rootdelay is a workaround by definition, so it may occasionally fail
if it is too small. Big rootdealy has a clear bad impact on boot time.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Ulf Hansson]

On 27 March 2013 13:28, Сергей Янович <ynvich@gmail.com> wrote:
> Hi Ulf,
>
> On 27 March 2013 15:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Moreover, this patch will have bad impact on booting the kernel, since
>> every host device that has scheduled a detect work from it's probe
>> function will also wait for it to finish. Even if it is the boot
>> device of not. If this is needed, I would prefer that a host cap is
>> used.
>
> Do you have any profiling data supporting bad impact on boot?
>
> It should be it in the range of dozens us, if any. Without the patch,
> approx. 30% of boots succeeded with CONFIG_PREEMT and aprox. 95%
> without CONFIG_PREEMT. mmc_rescan is just a few instructions, if there
> is no card present. On boot and with a card present, it might only
> sleep in the host implementation.
>
> Anyway, something had to be done about mmc boot. rootdelay will not
> report error if the card is absent or its partition table is damaged.
> rootdelay is a workaround by definition, so it may occasionally fail
> if it is too small. Big rootdealy has a clear bad impact on boot time.

Consider a platform having two eMMCs and one SD-card. Each eMMC card
will take around 400 ms to initialize and the SD-card 700 ms. The card
initialization times are real examples from eMMCs and SD-cards,
moreover those are typical values not worth cases. In total we have
around 1.5 s to initialize the cards.

Now, suppose you boot using an initrd image. Thus neither of the cards
needs to be accessible immediately after the kernel has booted. It all
depends what the init process decides to do. With this patch the init
process will always be delayed to wait for each and every card to be
initialized. I would prefer a solution where this can be configurable
somehow, since certainly this is not the scenario you want for all
cases.

Kind regards
Ulf Hansson

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On Tue, 2013-04-02 at 12:13 +0200, Ulf Hansson wrote:
> Consider a platform having two eMMCs and one SD-card. Each eMMC card
> will take around 400 ms to initialize and the SD-card 700 ms. The card
> initialization times are real examples from eMMCs and SD-cards,
> moreover those are typical values not worth cases. In total we have
> around 1.5 s to initialize the cards.

We have a separate workqueue per host, so all probes go in parallel.
They also go in parallel with probing for other devices. So the actual
delay is 700 ms minus maximum probing time for other devices. The time
is either zero or small (dozen us) on the hardware I have access to
(intel laptop, arm controller).

> Now, suppose you boot using an initrd image. Thus neither of the cards
> needs to be accessible immediately after the kernel has booted. It all
> depends what the init process decides to do. With this patch the init
> process will always be delayed to wait for each and every card to be
> initialized. I would prefer a solution where this can be configurable
> somehow, since certainly this is not the scenario you want for all
> cases.

If the system is booted using initrd and root is not on an mmc card,
then mmc modules can be omitted from initrd. The probing will happen
only after root is mounted.

If root is on an mmc, kernel needs to wait for the mmc probe.

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Ulf Hansson]

On 2 April 2013 12:35, Sergey Yanovich <ynvich@gmail.com> wrote:
> On Tue, 2013-04-02 at 12:13 +0200, Ulf Hansson wrote:
>> Consider a platform having two eMMCs and one SD-card. Each eMMC card
>> will take around 400 ms to initialize and the SD-card 700 ms. The card
>> initialization times are real examples from eMMCs and SD-cards,
>> moreover those are typical values not worth cases. In total we have
>> around 1.5 s to initialize the cards.
>
> We have a separate workqueue per host, so all probes go in parallel.
> They also go in parallel with probing for other devices. So the actual
> delay is 700 ms minus maximum probing time for other devices. The time
> is either zero or small (dozen us) on the hardware I have access to
> (intel laptop, arm controller).
>
>> Now, suppose you boot using an initrd image. Thus neither of the cards
>> needs to be accessible immediately after the kernel has booted. It all
>> depends what the init process decides to do. With this patch the init
>> process will always be delayed to wait for each and every card to be
>> initialized. I would prefer a solution where this can be configurable
>> somehow, since certainly this is not the scenario you want for all
>> cases.
>
> If the system is booted using initrd and root is not on an mmc card,
> then mmc modules can be omitted from initrd. The probing will happen
> only after root is mounted.

This will not solve the problem when having one device intended for
rootfs and some other for something else. Of course, as long as the
devices uses the same mmc module. Once inserted, all devices will be
probed.

>
> If root is on an mmc, kernel needs to wait for the mmc probe.
>

True, although your patch is preventing the parallelism and instead
doing things in synchronized manner.

I think we must discuss alternative solutions instead.

Like an "mmc detect flush" mechanism or a "new card device notification" event.

Kind regards
Ulf Hansson

Re: [PATCH v2] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On Tue, 2013-04-02 at 19:45 +0200, Ulf Hansson wrote:
> On 2 April 2013 12:35, Sergey Yanovich <ynvich@gmail.com> wrote:
> > If the system is booted using initrd and root is not on an mmc card,
> > then mmc modules can be omitted from initrd. The probing will happen
> > only after root is mounted.
> 
> This will not solve the problem when having one device intended for
> rootfs and some other for something else. Of course, as long as the
> devices uses the same mmc module. Once inserted, all devices will be
> probed.

I agree that is this special case there will be boot time regression.
However, this case is not guaranteed to boot without the patch or some
workaround.

> > If root is on an mmc, kernel needs to wait for the mmc probe.
> >
> 
> True, although your patch is preventing the parallelism and instead
> doing things in synchronized manner.

mount_root() assumes it has waited for "known devices to complete their
probing" [init/do_mounts.c:545]. The patch has brought mmc into
compliance with the assumption.

If several devices can be probed in parallel, the bus should do it, but
not the driver.

> I think we must discuss alternative solutions instead.
> 
> Like an "mmc detect flush" mechanism or a "new card device notification" event.

There are 2 events to trigger root mount:
1. all known devices complete their probing
2. 1 is true and root_wait is specified and root device is found

So I see the only fast alternative to my patch: if root is on an mmc
card, set root_wait to 'true'.

Re: [PATCH] wait while adding MMC host to ensure root mounts

[Greg Kroah-Hartman]

On Thu, Mar 14, 2013 at 05:06:23AM +0400, Sergey Yanovich wrote:
> MMC hosts are added asynchronously. We need to wait until detect returns to
> avoid failed root filesystem mounts.
> ---8<---
> VFS: Cannot open root device "mmcblk0p1" or unknown-block(0,0): error -6
> Please append a correct "root=" boot option; here are the available partitions:
> mmc0: host does not support reading read-only switch. assuming write-enable.
> 1f00             256 mtdblock0  (driver?)
> 1f01             256 mtdblock1  (driver?)
> 1f02            2560 mtdblock2 mmc0: new SDHC card at address b368
>  (driver?)
> 1f03           29696 mtdblock3  (driver?)
> 1f04           16384 mtdblock4 mmcblk0: mmc0:b368 USD   3.72 GiB
>  (driver?)
>  mmcblk0: p1
> b300         3910656 mmcblk0  driver: mmcblk
>   b301         3906560 mmcblk0p1 00000000-01
> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
> ---8<---
> 
> Signed-off-by: Sergey Yanovich <ynvich@gmail.com>
> ---
>  drivers/base/dd.c       |    1 -
>  drivers/mmc/core/core.c |    4 +++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 218f1e6..61d3e1b 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -345,7 +345,6 @@ int driver_probe_done(void)
>  void wait_for_device_probe(void)
>  {
>  	/* wait for the known devices to complete their probing */
> -
>  	wait_event(probe_waitqueue, atomic_read(&probe_count) == 0);
>  	async_synchronize_full();
>  }

You didn't need to change this file, please don't.


> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index aaed768..9790323 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -91,10 +91,11 @@ static int mmc_schedule_delayed_work(struct delayed_work *work,
>  /*
>   * Internal function. Flush all scheduled work from the MMC work queue.
>   */
> -static void mmc_flush_scheduled_work(void)
> +void mmc_flush_scheduled_work(void)
>  {
>  	flush_workqueue(workqueue);
>  }
> +EXPORT_SYMBOL(mmc_flush_scheduled_work);

EXPORT_SYMBOL_GPL()?

Wait, why are you exporting it at all?

>  #ifdef CONFIG_FAIL_MMC_REQUEST
>  
> @@ -2225,6 +2226,7 @@ void mmc_start_host(struct mmc_host *host)
>  	host->rescan_disable = 0;
>  	mmc_power_up(host);
>  	mmc_detect_change(host, 0);
> +	mmc_flush_scheduled_work();

If this is the only fix, then you don't need to change it from being
static or need to export it.

Please be more careful when making kernel patches.

greg k-h

Re: [PATCH] wait while adding MMC host to ensure root mounts

[Sergey Yanovich]

On 14/03/13 06:47, Greg Kroah-Hartman wrote:
> On Thu, Mar 14, 2013 at 05:06:23AM +0400, Sergey Yanovich wrote:
>>   	/* wait for the known devices to complete their probing */
>> -
>>   	wait_event(probe_waitqueue, atomic_read(&probe_count) == 0);
>
> You didn't need to change this file, please don't.

Got the point.

>>   	mmc_detect_change(host, 0);
>> +	mmc_flush_scheduled_work();
>
> If this is the only fix, then you don't need to change it from being
> static or need to export it.
>
> Please be more careful when making kernel patches.

I will. I apologize for spamming.