Skip to contentSkip to conversation

wait while adding MMC host to ensure root mounts

PATCH Superseded Sergey Yanovich 7 participants 24 messages Mar 14, 2013 – Apr 13, 2013 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 wait while adding MMC host to ensure root mounts

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<---

---
 drivers/base/dd.c       |    1 -
 drivers/mmc/core/core.c |    4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)
drivers/base/dd.c +0 -1
index 218f1e6..61d3e1b 100644
@@ -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();
 }
drivers/mmc/core/core.c +3 -1
index aaed768..9790323 100644
@@ -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)
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH v2 1/1 wait while adding MMC host to ensure root mounts

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<---

---
changes for v2:
- removed exporting as symbol is in the same file

 drivers/mmc/core/core.c |    1 +
 1 file changed, 1 insertion(+)
drivers/mmc/core/core.c +1 -0
index aaed768..7196888 100644
@@ -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)
Signature · 1 line
1.7.10.4
#2 lore raw

2013/3/14, Sergey Yanovich <[email protected]>:

Quoted 18 lines

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

Quoted 6 lines
Kernel panic - not syncing: VFS: Unable to mount root fs on

unknown-block(0,0)
---8<---

---

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

Thanks.

#3 lore raw

On 14/03/13 08:08, Namjae Jeon wrote:> 2013/3/14, Sergey Yanovich <[email protected]>:

Quoted 5 lines
Quoted 2 lines
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.

#4 lore raw
In reply to Namjae Jeon
Quoted 23 lines · Namjae Jeon

2013/3/14, Sergey Yanovich <[email protected]>:

Quoted 22 lines

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<---

Quoted 5 lines
Quoted 2 lines

---

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

Quoted 3 lines

Thanks.

#5 lore raw
In reply to Sergey Yanovich

Hi Sergey,

Quoted 32 lines · 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<---

---
changes for v2:
- removed exporting as symbol is in the same file

 drivers/mmc/core/core.c |    1 +
 1 file changed, 1 insertion(+)
drivers/mmc/core/core.c +0 -0
index aaed768..7196888 100644

drivers/mmc/core/core.c void mmc_start_host(struct mmc_host *host) · line 2231

Quoted 7 lines
drivers/mmc/core/core.c +1 -0
@@ -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.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#6 lore raw
Quoted 41 lines · Chris Ball

Hi Sergey,

Quoted 38 lines · 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<---

---
changes for v2:
- removed exporting as symbol is in the same file

 drivers/mmc/core/core.c |    1 +
 1 file changed, 1 insertion(+)
drivers/mmc/core/core.c +1 -0
index aaed768..7196888 100644
@@ -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();
 }
 
Quoted 8 lines

drivers/mmc/core/core.c void mmc_start_host(struct mmc_host *host) · line 2231

Quoted 1 line
drivers/mmc/core/core.c +0 -0
 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 <[email protected]> <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

#7 lore raw

Hi,

Quoted 2 lines · Ulf Hansson

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.

Quoted 5 lines

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.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#8 lore raw
Quoted 15 lines · Chris Ball

Hi,

Quoted 2 lines · Ulf Hansson

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.

Quoted 5 lines

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.

Quoted 6 lines

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.

#9 lore raw
Quoted 11 lines · Adrian Hunter

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
Quoted 1 line

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();

#10 lore raw
Quoted 7 lines · Sergey Yanovich
Quoted 6 lines · Adrian Hunter

On my system it is significant:

Before the patch:

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

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

Quoted 7 lines

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.

Review

Quoted 10 lines
Quoted 2 lines

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:

(quoted diff) +2 -1
-	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();
#11 lore raw
Quoted 1 line · Adrian Hunter

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.

Quoted 11 lines

Review

Quoted 5 lines

Maybe introduce mmc_is_hosting_root() and do something like:

(quoted diff) +2 -1
-	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.

#12 lore raw
Quoted 6 lines · Sergey Yanovich
Quoted 1 line · Adrian Hunter

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.

Quoted 9 lines
Quoted 9 lines

Review

Quoted 6 lines

Maybe introduce mmc_is_hosting_root() and do something like:

(quoted diff) +2 -1
-	mmc_flush_scheduled_work();
+	if (mmc_is_hosting_root())
+		mmc_flush_scheduled_work();

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

Quoted 6 lines
Quoted 3 lines
	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.

Quoted 2 lines

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.

Quoted 5 lines

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.

Quoted 8 lines

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.

#13 lore raw
Quoted 9 lines · Adrian Hunter
Quoted 6 lines · Sergey Yanovich
Quoted 1 line · Adrian Hunter

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")?

#14 lore raw
In reply to Chris Ball
Quoted 1 line · Chris Ball

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?

#15 lore raw

Hi,

Quoted 11 lines · Sergey Yanovich
Quoted 1 line · Chris Ball

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.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child

A message that is not in the archive (sent off-list or not yet received): CAPDyKFrF8pmo7mfa_1Vi2n=JBFd1h+VULQ1hJB75=DmNx0erdw@mail.gmail.com

#17 lore raw
Quoted 9 lines · Ulf Hansson

Den 13 apr 2013 12:49 skrev "Sergey Yanovich" <[email protected]>:

Quoted 1 line

A possible solution is to make card a separate device (now only the

host

Quoted 2 lines

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.

Quoted 3 lines

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.

#18 lore raw
In reply to Ulf Hansson

Hi Ulf,

Quoted 5 lines · Ulf Hansson

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.

#19 lore raw
Quoted 17 lines · Сергей Янович

Hi Ulf,

Quoted 5 lines · Ulf Hansson

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.

Quoted 4 lines

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

#20 lore raw
Quoted 5 lines · Ulf Hansson

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).

Quoted 7 lines

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.

#21 lore raw
Quoted 21 lines · Sergey Yanovich
Quoted 5 lines · Ulf Hansson

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).

Quoted 7 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

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

#22 lore raw
Quoted 9 lines · Ulf Hansson
Quoted 3 lines · Sergey Yanovich

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.

Quoted 5 lines
Quoted 2 lines

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.

Quoted 3 lines

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'.

#23 lore raw
In reply to Sergey Yanovich
Quoted 30 lines · 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<---

---
 drivers/base/dd.c       |    1 -
 drivers/mmc/core/core.c |    4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)
drivers/base/dd.c +0 -0
index 218f1e6..61d3e1b 100644

drivers/base/dd.c int driver_probe_done(void) · line 350

Quoted 7 lines
drivers/base/dd.c +0 -1
@@ -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.

Quoted 5 lines
drivers/mmc/core/core.c +0 -0
index aaed768..9790323 100644

drivers/mmc/core/core.c static int mmc_schedule_delayed_work(struct delayed_work *work, · line 98

Quoted 9 lines
drivers/mmc/core/core.c +2 -1
@@ -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?

drivers/mmc/core/core.c void mmc_start_host(struct mmc_host *host) · line 2229

Quoted 7 lines
drivers/mmc/core/core.c +1 -0
 #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

#24 lore raw
Quoted 6 lines · Greg Kroah-Hartman
Quoted 3 lines · Sergey Yanovich

/* 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.

Quoted 7 lines

drivers/mmc/core/core.c void mmc_start_host(struct mmc_host *host) · line 2229

Quoted 2 lines
  	mmc_detect_change(host, 0);
drivers/mmc/core/core.c +1 -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.