All of lore.kernel.org
 help / color / mirror / Atom feed
* [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
@ 2022-11-16  6:16 Peter Korsgaard
  2022-11-24 10:35 ` Peter Korsgaard
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Korsgaard @ 2022-11-16  6:16 UTC (permalink / raw)
  To: Mike Snitzer, Helen Koike, dm-devel, Jonathan Corbet; +Cc: Peter Korsgaard

Just calling wait_for_device_probe() is not enough to ensure that
asynchronously probed block devices are available (E.G.  mmc, usb, ..), so
add a "dm-mod.waitfor=<device1>[,..,<deviceN>]" parameter to get dm-init to
explicitly wait for specific block devices before initializing the tables
with logic similar to the rootwait logic in init/do_mounts.c.

E.G. with dm-verity on mmc with
dm-mod.waitfor="PARTLABEL=hash-a,PARTLABEL=root-a"

[    0.671671] device-mapper: init: waiting for all devices to be available before creating mapped devices
[    0.671679] device-mapper: init: waiting for PARTLABEL=hash-a
[    0.710695] mmc0: new HS200 MMC card at address 0001
[    0.711158] mmcblk0: mmc0:0001 004GA0 3.69 GiB
[    0.715954] mmcblk0boot0: mmc0:0001 004GA0 partition 1 2.00 MiB
[    0.722085] mmcblk0boot1: mmc0:0001 004GA0 partition 2 2.00 MiB
[    0.728093] mmcblk0rpmb: mmc0:0001 004GA0 partition 3 512 KiB, chardev (249:0)
[    0.738274]  mmcblk0: p1 p2 p3 p4 p5 p6 p7
[    0.751282] device-mapper: init: waiting for PARTLABEL=root-a
[    0.751306] device-mapper: init: all devices available
[    0.751683] device-mapper: verity: sha256 using implementation "sha256-generic"
[    0.759344] device-mapper: ioctl: dm-0 (vroot) is ready
[    0.766540] VFS: Mounted root (squashfs filesystem) readonly on device 254:0.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
Changes since v1:
- Fix s/dm-init/dm-mod/ typo in documentation, drop trailing newline

 .../admin-guide/device-mapper/dm-init.rst     |  8 +++++++
 drivers/md/dm-init.c                          | 23 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/device-mapper/dm-init.rst b/Documentation/admin-guide/device-mapper/dm-init.rst
index e5242ff17e9b..981d6a907699 100644
--- a/Documentation/admin-guide/device-mapper/dm-init.rst
+++ b/Documentation/admin-guide/device-mapper/dm-init.rst
@@ -123,3 +123,11 @@ Other examples (per target):
     0 1638400 verity 1 8:1 8:2 4096 4096 204800 1 sha256
     fb1a5a0f00deb908d8b53cb270858975e76cf64105d412ce764225d53b8f3cfd
     51934789604d1b92399c52e7cb149d1b3a1b74bbbcb103b2a0aaacbed5c08584
+
+For setups using device-mapper on top of asynchronously probed block
+devices (MMC, USB, ..), it may be necessary to tell dm-init to
+explicitly wait for them to become available before setting up the
+device-mapper tables. This can be done with the "dm-mod.waitfor="
+module parameter, which takes a list of devices to wait for::
+
+  dm-mod.waitfor=<device1>[,..,<deviceN>]
diff --git a/drivers/md/dm-init.c b/drivers/md/dm-init.c
index b0c45c6ebe0b..fc70b568e072 100644
--- a/drivers/md/dm-init.c
+++ b/drivers/md/dm-init.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/ctype.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/device-mapper.h>
 #include <linux/init.h>
@@ -18,12 +19,17 @@
 #define DM_MAX_DEVICES 256
 #define DM_MAX_TARGETS 256
 #define DM_MAX_STR_SIZE 4096
+#define DM_MAX_WAITFOR 256
 
 static char *create;
 
+static char *waitfor[DM_MAX_WAITFOR];
+
 /*
  * Format: dm-mod.create=<name>,<uuid>,<minor>,<flags>,<table>[,<table>+][;<name>,<uuid>,<minor>,<flags>,<table>[,<table>+]+]
  * Table format: <start_sector> <num_sectors> <target_type> <target_args>
+ * Block devices to wait for to become available before setting up tables:
+ * dm-mod.waitfor=<device1>[,..,<deviceN>]
  *
  * See Documentation/admin-guide/device-mapper/dm-init.rst for dm-mod.create="..." format
  * details.
@@ -266,7 +272,7 @@ static int __init dm_init_init(void)
 	struct dm_device *dev;
 	LIST_HEAD(devices);
 	char *str;
-	int r;
+	int i, r;
 
 	if (!create)
 		return 0;
@@ -286,6 +292,18 @@ static int __init dm_init_init(void)
 	DMINFO("waiting for all devices to be available before creating mapped devices");
 	wait_for_device_probe();
 
+	for (i = 0; i < ARRAY_SIZE(waitfor); i++) {
+		if (waitfor[i]) {
+			DMINFO("waiting for %s", waitfor[i]);
+
+			while (!dm_get_dev_t(waitfor[i]))
+				msleep(20);
+		}
+	}
+
+	if (waitfor[0])
+		DMINFO("all devices available");
+
 	list_for_each_entry(dev, &devices, list) {
 		if (dm_early_create(&dev->dmi, dev->table,
 				    dev->target_args_array))
@@ -301,3 +319,6 @@ late_initcall(dm_init_init);
 
 module_param(create, charp, 0);
 MODULE_PARM_DESC(create, "Create a mapped device in early boot");
+
+module_param_array(waitfor, charp, NULL, 0);
+MODULE_PARM_DESC(waitfor, "Devices to wait for before setting up tables");
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
  2022-11-16  6:16 [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices Peter Korsgaard
@ 2022-11-24 10:35 ` Peter Korsgaard
  2022-11-30 23:07   ` Mike Snitzer
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Korsgaard @ 2022-11-24 10:35 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: fabio.aiuto, Helen Koike, michael, dm-devel, Jonathan Corbet

>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

 > Just calling wait_for_device_probe() is not enough to ensure that
 > asynchronously probed block devices are available (E.G.  mmc, usb, ..), so
 > add a "dm-mod.waitfor=<device1>[,..,<deviceN>]" parameter to get dm-init to
 > explicitly wait for specific block devices before initializing the tables
 > with logic similar to the rootwait logic in init/do_mounts.c.

 > E.G. with dm-verity on mmc with
 > dm-mod.waitfor="PARTLABEL=hash-a,PARTLABEL=root-a"

 > [    0.671671] device-mapper: init: waiting for all devices to be available before creating mapped devices
 > [    0.671679] device-mapper: init: waiting for PARTLABEL=hash-a
 > [    0.710695] mmc0: new HS200 MMC card at address 0001
 > [    0.711158] mmcblk0: mmc0:0001 004GA0 3.69 GiB
 > [    0.715954] mmcblk0boot0: mmc0:0001 004GA0 partition 1 2.00 MiB
 > [    0.722085] mmcblk0boot1: mmc0:0001 004GA0 partition 2 2.00 MiB
 > [    0.728093] mmcblk0rpmb: mmc0:0001 004GA0 partition 3 512 KiB, chardev (249:0)
 > [    0.738274]  mmcblk0: p1 p2 p3 p4 p5 p6 p7
 > [    0.751282] device-mapper: init: waiting for PARTLABEL=root-a
 > [    0.751306] device-mapper: init: all devices available
 > [    0.751683] device-mapper: verity: sha256 using implementation "sha256-generic"
 > [    0.759344] device-mapper: ioctl: dm-0 (vroot) is ready
 > [    0.766540] VFS: Mounted root (squashfs filesystem) readonly on device 254:0.

 > Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
 > ---
 > Changes since v1:
 > - Fix s/dm-init/dm-mod/ typo in documentation, drop trailing newline

Ping?

FYI: I was recently made aware that other patches fixing this issue
(but less nice, E.G. with a fixed delay) have been posted in the past,
so there seems to be a general need for something like this.

E.G:
https://lore.kernel.org/all/20220406154631.277107-1-fabio.aiuto@amarulasolutions.com/



 >  .../admin-guide/device-mapper/dm-init.rst     |  8 +++++++
 >  drivers/md/dm-init.c                          | 23 ++++++++++++++++++-
 >  2 files changed, 30 insertions(+), 1 deletion(-)

 > diff --git a/Documentation/admin-guide/device-mapper/dm-init.rst b/Documentation/admin-guide/device-mapper/dm-init.rst
 > index e5242ff17e9b..981d6a907699 100644
 > --- a/Documentation/admin-guide/device-mapper/dm-init.rst
 > +++ b/Documentation/admin-guide/device-mapper/dm-init.rst
 > @@ -123,3 +123,11 @@ Other examples (per target):
 >      0 1638400 verity 1 8:1 8:2 4096 4096 204800 1 sha256
 >      fb1a5a0f00deb908d8b53cb270858975e76cf64105d412ce764225d53b8f3cfd
 >      51934789604d1b92399c52e7cb149d1b3a1b74bbbcb103b2a0aaacbed5c08584
 > +
 > +For setups using device-mapper on top of asynchronously probed block
 > +devices (MMC, USB, ..), it may be necessary to tell dm-init to
 > +explicitly wait for them to become available before setting up the
 > +device-mapper tables. This can be done with the "dm-mod.waitfor="
 > +module parameter, which takes a list of devices to wait for::
 > +
 > +  dm-mod.waitfor=<device1>[,..,<deviceN>]
 > diff --git a/drivers/md/dm-init.c b/drivers/md/dm-init.c
 > index b0c45c6ebe0b..fc70b568e072 100644
 > --- a/drivers/md/dm-init.c
 > +++ b/drivers/md/dm-init.c
 > @@ -8,6 +8,7 @@
 >   */
 
 >  #include <linux/ctype.h>
 > +#include <linux/delay.h>
 >  #include <linux/device.h>
 >  #include <linux/device-mapper.h>
 >  #include <linux/init.h>
 > @@ -18,12 +19,17 @@
 >  #define DM_MAX_DEVICES 256
 >  #define DM_MAX_TARGETS 256
 >  #define DM_MAX_STR_SIZE 4096
 > +#define DM_MAX_WAITFOR 256
 
 >  static char *create;
 
 > +static char *waitfor[DM_MAX_WAITFOR];
 > +
 >  /*
 >   * Format: dm-mod.create=<name>,<uuid>,<minor>,<flags>,<table>[,<table>+][;<name>,<uuid>,<minor>,<flags>,<table>[,<table>+]+]
 >   * Table format: <start_sector> <num_sectors> <target_type> <target_args>
 > + * Block devices to wait for to become available before setting up tables:
 > + * dm-mod.waitfor=<device1>[,..,<deviceN>]
 >   *
 >   * See Documentation/admin-guide/device-mapper/dm-init.rst for dm-mod.create="..." format
 >   * details.
 > @@ -266,7 +272,7 @@ static int __init dm_init_init(void)
 >  	struct dm_device *dev;
 >  	LIST_HEAD(devices);
 >  	char *str;
 > -	int r;
 > +	int i, r;
 
 >  	if (!create)
 >  		return 0;
 > @@ -286,6 +292,18 @@ static int __init dm_init_init(void)
 >  	DMINFO("waiting for all devices to be available before creating mapped devices");
 >  	wait_for_device_probe();
 
 > +	for (i = 0; i < ARRAY_SIZE(waitfor); i++) {
 > +		if (waitfor[i]) {
 > +			DMINFO("waiting for %s", waitfor[i]);
 > +
 > +			while (!dm_get_dev_t(waitfor[i]))
 > +				msleep(20);
 > +		}
 > +	}
 > +
 > +	if (waitfor[0])
 > +		DMINFO("all devices available");
 > +
 >  	list_for_each_entry(dev, &devices, list) {
 >  		if (dm_early_create(&dev->dmi, dev->table,
 dev-> target_args_array))
 > @@ -301,3 +319,6 @@ late_initcall(dm_init_init);
 
 >  module_param(create, charp, 0);
 >  MODULE_PARM_DESC(create, "Create a mapped device in early boot");
 > +
 > +module_param_array(waitfor, charp, NULL, 0);
 > +MODULE_PARM_DESC(waitfor, "Devices to wait for before setting up tables");
 > -- 

 > 2.30.2


-- 
Bye, Peter Korsgaard

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
  2022-11-24 10:35 ` Peter Korsgaard
@ 2022-11-30 23:07   ` Mike Snitzer
  2022-12-01  7:28     ` Peter Korsgaard
  0 siblings, 1 reply; 8+ messages in thread
From: Mike Snitzer @ 2022-11-30 23:07 UTC (permalink / raw)
  To: Peter Korsgaard
  Cc: Jonathan Corbet, Mike Snitzer, dm-devel, fabio.aiuto,
	Helen Koike, michael

On Thu, Nov 24 2022 at  5:35P -0500,
Peter Korsgaard <peter@korsgaard.com> wrote:

> >>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:
> 
>  > Just calling wait_for_device_probe() is not enough to ensure that
>  > asynchronously probed block devices are available (E.G.  mmc, usb, ..), so
>  > add a "dm-mod.waitfor=<device1>[,..,<deviceN>]" parameter to get dm-init to
>  > explicitly wait for specific block devices before initializing the tables
>  > with logic similar to the rootwait logic in init/do_mounts.c.
> 
>  > E.G. with dm-verity on mmc with
>  > dm-mod.waitfor="PARTLABEL=hash-a,PARTLABEL=root-a"
> 
>  > [    0.671671] device-mapper: init: waiting for all devices to be available before creating mapped devices
>  > [    0.671679] device-mapper: init: waiting for PARTLABEL=hash-a
>  > [    0.710695] mmc0: new HS200 MMC card at address 0001
>  > [    0.711158] mmcblk0: mmc0:0001 004GA0 3.69 GiB
>  > [    0.715954] mmcblk0boot0: mmc0:0001 004GA0 partition 1 2.00 MiB
>  > [    0.722085] mmcblk0boot1: mmc0:0001 004GA0 partition 2 2.00 MiB
>  > [    0.728093] mmcblk0rpmb: mmc0:0001 004GA0 partition 3 512 KiB, chardev (249:0)
>  > [    0.738274]  mmcblk0: p1 p2 p3 p4 p5 p6 p7
>  > [    0.751282] device-mapper: init: waiting for PARTLABEL=root-a
>  > [    0.751306] device-mapper: init: all devices available
>  > [    0.751683] device-mapper: verity: sha256 using implementation "sha256-generic"
>  > [    0.759344] device-mapper: ioctl: dm-0 (vroot) is ready
>  > [    0.766540] VFS: Mounted root (squashfs filesystem) readonly on device 254:0.
> 
>  > Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
>  > ---
>  > Changes since v1:
>  > - Fix s/dm-init/dm-mod/ typo in documentation, drop trailing newline
> 
> Ping?
> 
> FYI: I was recently made aware that other patches fixing this issue
> (but less nice, E.G. with a fixed delay) have been posted in the past,
> so there seems to be a general need for something like this.
> 
> E.G:
> https://lore.kernel.org/all/20220406154631.277107-1-fabio.aiuto@amarulasolutions.com/
> 
> 
> 
>  >  .../admin-guide/device-mapper/dm-init.rst     |  8 +++++++
>  >  drivers/md/dm-init.c                          | 23 ++++++++++++++++++-
>  >  2 files changed, 30 insertions(+), 1 deletion(-)
> 
>  > diff --git a/Documentation/admin-guide/device-mapper/dm-init.rst b/Documentation/admin-guide/device-mapper/dm-init.rst
>  > index e5242ff17e9b..981d6a907699 100644
>  > --- a/Documentation/admin-guide/device-mapper/dm-init.rst
>  > +++ b/Documentation/admin-guide/device-mapper/dm-init.rst
>  > @@ -123,3 +123,11 @@ Other examples (per target):
>  >      0 1638400 verity 1 8:1 8:2 4096 4096 204800 1 sha256
>  >      fb1a5a0f00deb908d8b53cb270858975e76cf64105d412ce764225d53b8f3cfd
>  >      51934789604d1b92399c52e7cb149d1b3a1b74bbbcb103b2a0aaacbed5c08584
>  > +
>  > +For setups using device-mapper on top of asynchronously probed block
>  > +devices (MMC, USB, ..), it may be necessary to tell dm-init to
>  > +explicitly wait for them to become available before setting up the
>  > +device-mapper tables. This can be done with the "dm-mod.waitfor="
>  > +module parameter, which takes a list of devices to wait for::
>  > +
>  > +  dm-mod.waitfor=<device1>[,..,<deviceN>]
>  > diff --git a/drivers/md/dm-init.c b/drivers/md/dm-init.c
>  > index b0c45c6ebe0b..fc70b568e072 100644
>  > --- a/drivers/md/dm-init.c
>  > +++ b/drivers/md/dm-init.c
>  > @@ -8,6 +8,7 @@
>  >   */
>  
>  >  #include <linux/ctype.h>
>  > +#include <linux/delay.h>
>  >  #include <linux/device.h>
>  >  #include <linux/device-mapper.h>
>  >  #include <linux/init.h>
>  > @@ -18,12 +19,17 @@
>  >  #define DM_MAX_DEVICES 256
>  >  #define DM_MAX_TARGETS 256
>  >  #define DM_MAX_STR_SIZE 4096
>  > +#define DM_MAX_WAITFOR 256
>  
>  >  static char *create;
>  
>  > +static char *waitfor[DM_MAX_WAITFOR];
>  > +
>  >  /*
>  >   * Format: dm-mod.create=<name>,<uuid>,<minor>,<flags>,<table>[,<table>+][;<name>,<uuid>,<minor>,<flags>,<table>[,<table>+]+]
>  >   * Table format: <start_sector> <num_sectors> <target_type> <target_args>
>  > + * Block devices to wait for to become available before setting up tables:
>  > + * dm-mod.waitfor=<device1>[,..,<deviceN>]
>  >   *
>  >   * See Documentation/admin-guide/device-mapper/dm-init.rst for dm-mod.create="..." format
>  >   * details.
>  > @@ -266,7 +272,7 @@ static int __init dm_init_init(void)
>  >  	struct dm_device *dev;
>  >  	LIST_HEAD(devices);
>  >  	char *str;
>  > -	int r;
>  > +	int i, r;
>  
>  >  	if (!create)
>  >  		return 0;
>  > @@ -286,6 +292,18 @@ static int __init dm_init_init(void)
>  >  	DMINFO("waiting for all devices to be available before creating mapped devices");
>  >  	wait_for_device_probe();
>  
>  > +	for (i = 0; i < ARRAY_SIZE(waitfor); i++) {
>  > +		if (waitfor[i]) {
>  > +			DMINFO("waiting for %s", waitfor[i]);
>  > +
>  > +			while (!dm_get_dev_t(waitfor[i]))
>  > +				msleep(20);
>  > +		}
>  > +	}
>  > +
>  > +	if (waitfor[0])
>  > +		DMINFO("all devices available");
>  > +

Why 20?  Also, why is waiting indefinitely OK?  Would really like to
hear from other consumers of dm-init that this module param is useful
and needed.

Mike


>  >  	list_for_each_entry(dev, &devices, list) {
>  >  		if (dm_early_create(&dev->dmi, dev->table,
>  dev-> target_args_array))
>  > @@ -301,3 +319,6 @@ late_initcall(dm_init_init);
>  
>  >  module_param(create, charp, 0);
>  >  MODULE_PARM_DESC(create, "Create a mapped device in early boot");
>  > +
>  > +module_param_array(waitfor, charp, NULL, 0);
>  > +MODULE_PARM_DESC(waitfor, "Devices to wait for before setting up tables");
>  > -- 
> 
>  > 2.30.2
> 
> 
> -- 
> Bye, Peter Korsgaard
> 


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
  2022-11-30 23:07   ` Mike Snitzer
@ 2022-12-01  7:28     ` Peter Korsgaard
  2022-12-01 16:13       ` Mike Snitzer
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Korsgaard @ 2022-12-01  7:28 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Jonathan Corbet, Mike Snitzer, dm-devel, fabio.aiuto,
	Helen Koike, michael

>>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:

Hi,

 > On Thu, Nov 24 2022 at  5:35P -0500,
 > Peter Korsgaard <peter@korsgaard.com> wrote:

 >> > +	if (waitfor[0])
 >> > +		DMINFO("all devices available");
 >> > +

 > Why 20?  Also, why is waiting indefinitely OK?  Would really like to
 > hear from other consumers of dm-init that this module param is useful
 > and needed.

Mainly because of checkpatch.pl ;) prepare_namespace() uses msleep(5)
but checkpatch complains about it, so I made it 20 instead. It doesn't
really matter much, as long as it is small enough to not delay boots too
much (E.G. this is typically used for embedded system where boot time is
critical).

I would say that it DOES make sense to wait forever, similar to how it
is done in prepare_namespace() when rootwait is passed. This waitfor
bootargs is used for root-on-dm, so we cannot continue until the
underlying devices are available.

As mentioned,
https://lore.kernel.org/all/20220406154631.277107-1-fabio.aiuto@amarulasolutions.com/
is an alternatively (less nice) approach to solve the same issue, so yes
- It is useful and needed.

-- 
Bye, Peter Korsgaard

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
  2022-12-01  7:28     ` Peter Korsgaard
@ 2022-12-01 16:13       ` Mike Snitzer
  2022-12-01 16:38         ` Peter Korsgaard
  0 siblings, 1 reply; 8+ messages in thread
From: Mike Snitzer @ 2022-12-01 16:13 UTC (permalink / raw)
  To: Peter Korsgaard
  Cc: Jonathan Corbet, Mike Snitzer, Helen Koike, fabio.aiuto,
	dm-devel, michael

On Thu, Dec 01 2022 at  2:28P -0500,
Peter Korsgaard <peter@korsgaard.com> wrote:

> >>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:
> 
> Hi,
> 
>  > On Thu, Nov 24 2022 at  5:35P -0500,
>  > Peter Korsgaard <peter@korsgaard.com> wrote:
> 
>  >> > +	if (waitfor[0])
>  >> > +		DMINFO("all devices available");
>  >> > +
> 
>  > Why 20?  Also, why is waiting indefinitely OK?  Would really like to
>  > hear from other consumers of dm-init that this module param is useful
>  > and needed.
> 
> Mainly because of checkpatch.pl ;) prepare_namespace() uses msleep(5)
> but checkpatch complains about it, so I made it 20 instead. It doesn't
> really matter much, as long as it is small enough to not delay boots too
> much (E.G. this is typically used for embedded system where boot time is
> critical).
> 
> I would say that it DOES make sense to wait forever, similar to how it
> is done in prepare_namespace() when rootwait is passed. This waitfor
> bootargs is used for root-on-dm, so we cannot continue until the
> underlying devices are available.
> 
> As mentioned,
> https://lore.kernel.org/all/20220406154631.277107-1-fabio.aiuto@amarulasolutions.com/
> is an alternatively (less nice) approach to solve the same issue, so yes
> - It is useful and needed.

OK, so it should be easy for others to say so, right?

My hesitation is that it feels like something that could be papering
over device specific issues with their async initialization?  Even if
that is the case, it'd be nice to know _why_ this change is needed.
IMHO, the patch header stops short of offering compelling and informed
justification.

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
  2022-12-01 16:13       ` Mike Snitzer
@ 2022-12-01 16:38         ` Peter Korsgaard
  2022-12-02  6:44           ` Mike Snitzer
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Korsgaard @ 2022-12-01 16:38 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Jonathan Corbet, Mike Snitzer, Helen Koike, fabio.aiuto,
	dm-devel, michael

>>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:

Hi,

 >> As mentioned,
 >> https://lore.kernel.org/all/20220406154631.277107-1-fabio.aiuto@amarulasolutions.com/
 >> is an alternatively (less nice) approach to solve the same issue, so yes
 >> - It is useful and needed.

 > OK, so it should be easy for others to say so, right?

Yes. Fabio, you pointed me to the earlier patch, so you presumably need
something like this as well, right?


 > My hesitation is that it feels like something that could be papering
 > over device specific issues with their async initialization?  Even if
 > that is the case, it'd be nice to know _why_ this change is needed.
 > IMHO, the patch header stops short of offering compelling and informed
 > justification.

Well, it is exactly like the rootwait option we have for normal (non-dm)
root= handling, so it is for the same use cases / issues.

A lot of embedded / non-initramfs setups use rootwait today because of
the async initialization logic. If those setups want to use root-on-dm
(E.G. for dm-verity) then they need to use this dm_mod.waitfor= instead
of rootwait.

rootwait was added by:

commit cc1ed7542c8c26af0f501da8006b9fce03e9aaca
Author: Pierre Ossman <drzeus-list@drzeus.cx>
Date:   Sun Jul 15 23:40:35 2007 -0700

    init: wait for asynchronously scanned block devices

    Some buses (e.g.  USB and MMC) do their scanning of devices in the
    background, causing a race between them and prepare_namespace().  In order
    to be able to use these buses without an initrd, we now wait for the device
    specified in root= to actually show up.

    If the device never shows up than we will hang in an infinite loop.  In
    order to not mess with setups that reboot on panic, the feature must be
    turned on via the command line option "rootwait".

    [bunk@stusta.de: root_wait can become static]
    Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Cc: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Adrian Bunk <bunk@stusta.de>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

-- 
Bye, Peter Korsgaard

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
  2022-12-01 16:38         ` Peter Korsgaard
@ 2022-12-02  6:44           ` Mike Snitzer
  2022-12-02  6:51             ` Peter Korsgaard
  0 siblings, 1 reply; 8+ messages in thread
From: Mike Snitzer @ 2022-12-02  6:44 UTC (permalink / raw)
  To: Peter Korsgaard
  Cc: Jonathan Corbet, Mike Snitzer, Helen Koike, fabio.aiuto,
	dm-devel, michael

On Thu, Dec 01 2022 at 11:38P -0500,
Peter Korsgaard <peter@korsgaard.com> wrote:

> >>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:
> 
> Hi,
> 
>  >> As mentioned,
>  >> https://lore.kernel.org/all/20220406154631.277107-1-fabio.aiuto@amarulasolutions.com/
>  >> is an alternatively (less nice) approach to solve the same issue, so yes
>  >> - It is useful and needed.
> 
>  > OK, so it should be easy for others to say so, right?
> 
> Yes. Fabio, you pointed me to the earlier patch, so you presumably need
> something like this as well, right?
> 
> 
>  > My hesitation is that it feels like something that could be papering
>  > over device specific issues with their async initialization?  Even if
>  > that is the case, it'd be nice to know _why_ this change is needed.
>  > IMHO, the patch header stops short of offering compelling and informed
>  > justification.
> 
> Well, it is exactly like the rootwait option we have for normal (non-dm)
> root= handling, so it is for the same use cases / issues.
> 
> A lot of embedded / non-initramfs setups use rootwait today because of
> the async initialization logic. If those setups want to use root-on-dm
> (E.G. for dm-verity) then they need to use this dm_mod.waitfor= instead
> of rootwait.
> 
> rootwait was added by:
> 
> commit cc1ed7542c8c26af0f501da8006b9fce03e9aaca
> Author: Pierre Ossman <drzeus-list@drzeus.cx>
> Date:   Sun Jul 15 23:40:35 2007 -0700
> 
>     init: wait for asynchronously scanned block devices
> 
>     Some buses (e.g.  USB and MMC) do their scanning of devices in the
>     background, causing a race between them and prepare_namespace().  In order
>     to be able to use these buses without an initrd, we now wait for the device
>     specified in root= to actually show up.
> 
>     If the device never shows up than we will hang in an infinite loop.  In
>     order to not mess with setups that reboot on panic, the feature must be
>     turned on via the command line option "rootwait".
> 
>     [bunk@stusta.de: root_wait can become static]
>     Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
>     Cc: Al Viro <viro@zeniv.linux.org.uk>
>     Cc: Christoph Hellwig <hch@lst.de>
>     Signed-off-by: Adrian Bunk <bunk@stusta.de>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

OK, I'll accept your patch, but will tweak the DMINFO slightly to look
more like rootwait's pr_info (e.g. "Waiting for device %s..." and such).

I'll also change the msleep(20) to msleep(5) like was introduced with
commit 39a0e975c37de ("init: reduce rootwait polling interval time to
5ms") -- checkpatch be damned. ;)

Thanks,
Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices
  2022-12-02  6:44           ` Mike Snitzer
@ 2022-12-02  6:51             ` Peter Korsgaard
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Korsgaard @ 2022-12-02  6:51 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Jonathan Corbet, Mike Snitzer, dm-devel, fabio.aiuto,
	Helen Koike, michael

>>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:

Hi,

 > OK, I'll accept your patch, but will tweak the DMINFO slightly to look
 > more like rootwait's pr_info (e.g. "Waiting for device %s..." and such).

 > I'll also change the msleep(20) to msleep(5) like was introduced with
 > commit 39a0e975c37de ("init: reduce rootwait polling interval time to
 > 5ms") -- checkpatch be damned. ;)

Cool, thanks!

-- 
Bye, Peter Korsgaard

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

end of thread, other threads:[~2022-12-02  6:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-16  6:16 [dm-devel] [PATCH v2] dm init: add dm-mod.waitfor to wait for asynchronously probed block devices Peter Korsgaard
2022-11-24 10:35 ` Peter Korsgaard
2022-11-30 23:07   ` Mike Snitzer
2022-12-01  7:28     ` Peter Korsgaard
2022-12-01 16:13       ` Mike Snitzer
2022-12-01 16:38         ` Peter Korsgaard
2022-12-02  6:44           ` Mike Snitzer
2022-12-02  6:51             ` Peter Korsgaard

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