All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs
@ 2022-10-17 11:05 Alessandro Carminati
  2022-10-24 13:49 ` Richard Palethorpe
  0 siblings, 1 reply; 8+ messages in thread
From: Alessandro Carminati @ 2022-10-17 11:05 UTC (permalink / raw)
  To: ltp

mount_root() is the kernel function responsible for mounting the primary
rootfs.
A dynamic there, prevents the /dev/root device node in the not yet mounted
files system. For this reason, in the embedded system that starts without
an initramfs, or however a proper initscript, the /dev/root device appears
into the mount table in the / line.
The test tries to open this /dev/root and fails with a warning.
This patch aims to fix this situation.

Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>

typo fixes
---
 lib/tst_device.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/lib/tst_device.c b/lib/tst_device.c
index 8419b80c3..c3427eb31 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev)
 	unsigned int dev_major, dev_minor, line_mjr, line_mnr;
 	unsigned int len, best_match_len = 1;
 	char mnt_point[PATH_MAX];
+	char tmpbuf1[PATH_MAX];
+	char tmpbuf2[PATH_MAX];
 
 	if (stat(path, &buf) < 0)
 		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
@@ -562,6 +564,24 @@ void tst_find_backing_dev(const char *path, char *dev)
 	if (!*dev)
 		tst_brkm(TBROK, NULL, "Cannot find block device for %s", path);
 
+	if (stat(dev, &buf) < 0) {
+		if (strcmp("/dev/root", dev) != 0) {
+			tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
+		} else {
+			sprintf(tmpbuf1, "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor);
+			file = SAFE_FOPEN(NULL, tmpbuf1, "r");
+			while (fgets(line, sizeof(line), file)) {
+				if (sscanf(line, "%[^=]=%s", tmpbuf1, tmpbuf2) != 2)
+					continue;
+				if (strcmp("DEVNAME", tmpbuf1) == 0) {
+					sprintf(dev, "/dev/%s", tmpbuf2);
+					break;
+				}
+			}
+			SAFE_FCLOSE(NULL, file);
+		}
+	}
+
 	if (stat(dev, &buf) < 0)
 		tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
 
-- 
2.34.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* Re: [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs
  2022-10-17 11:05 [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs Alessandro Carminati
@ 2022-10-24 13:49 ` Richard Palethorpe
  2022-10-25  9:11   ` Alessandro Carminati
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Palethorpe @ 2022-10-24 13:49 UTC (permalink / raw)
  To: Alessandro Carminati; +Cc: ltp

Hello,

Alessandro Carminati <alessandro.carminati@gmail.com> writes:

> mount_root() is the kernel function responsible for mounting the primary
> rootfs.
> A dynamic there, prevents the /dev/root device node in the not yet mounted
> files system. For this reason, in the embedded system that starts without
> an initramfs, or however a proper initscript, the /dev/root device appears
> into the mount table in the / line.
> The test tries to open this /dev/root and fails with a warning.
> This patch aims to fix this situation.

Thanks I probably would have hit this issue sooner or later.

>
> Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>
>
> typo fixes
> ---
>  lib/tst_device.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 8419b80c3..c3427eb31 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev)
>  	unsigned int dev_major, dev_minor, line_mjr, line_mnr;
>  	unsigned int len, best_match_len = 1;
>  	char mnt_point[PATH_MAX];
> +	char tmpbuf1[PATH_MAX];
> +	char tmpbuf2[PATH_MAX];

It would be more readable to use three buffers and give them meaningful names.

>  
>  	if (stat(path, &buf) < 0)
>  		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
> @@ -562,6 +564,24 @@ void tst_find_backing_dev(const char *path, char *dev)
>  	if (!*dev)
>  		tst_brkm(TBROK, NULL, "Cannot find block device for %s", path);
>  
> +	if (stat(dev, &buf) < 0) {

As there is no harm in calling stat twice; could just do

if (stat(dev, &buf) < 0) && strcmp("/dev/root", dev) == 0) {

Or even replace with strcmp with errno == ENOENT and use this as a
general fallback.

> +		if (strcmp("/dev/root", dev) != 0) {
> +			tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
> +		} else {

Then remove this if statement

> +			sprintf(tmpbuf1, "/sys/dev/block/%d:%d/uevent",
> dev_major, dev_minor);
> +			file = SAFE_FOPEN(NULL, tmpbuf1, "r");
> +			while (fgets(line, sizeof(line), file)) {
> +				if (sscanf(line, "%[^=]=%s", tmpbuf1, tmpbuf2) != 2)
> +					continue;
> +				if (strcmp("DEVNAME", tmpbuf1) == 0) {
> +					sprintf(dev, "/dev/%s", tmpbuf2);
> +					break;
> +				}
> +			}
> +			SAFE_FCLOSE(NULL, file);
> +		}
> +	}
> +
>  	if (stat(dev, &buf) < 0)
>  		tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
>  
> -- 
> 2.34.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* Re: [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs
  2022-10-24 13:49 ` Richard Palethorpe
@ 2022-10-25  9:11   ` Alessandro Carminati
  2022-10-25 14:59     ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp
  0 siblings, 1 reply; 8+ messages in thread
From: Alessandro Carminati @ 2022-10-25  9:11 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: ltp

mount_root() is the kernel function responsible for mounting the primary
rootfs.
A dynamic there, prevents the /dev/root device node in the not yet mounted
files system. For this reason, in the embedded system that starts without
an initramfs, or however a proper initscript, the /dev/root device appears
into the mount table in the / line.
The test ioctl_loop05 tries to open this /dev/root and fails with a warning.
This patch aims to fix this situation.

Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>
---
 lib/tst_device.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lib/tst_device.c b/lib/tst_device.c
index 8419b80c3..c67328db3 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev)
        unsigned int dev_major, dev_minor, line_mjr, line_mnr;
        unsigned int len, best_match_len = 1;
        char mnt_point[PATH_MAX];
+       char devnum_info_fn[PATH_MAX];
+       char new_dev_fn_buf[PATH_MAX];

        if (stat(path, &buf) < 0)
                tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
@@ -562,6 +564,20 @@ void tst_find_backing_dev(const char *path, char *dev)
        if (!*dev)
                tst_brkm(TBROK, NULL, "Cannot find block device for %s", path);

+       if (stat(dev, &buf) < 0) && (strcmp("/dev/root", dev)==0) {
+               sprintf(devnum_info_fn, "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor);
+               file = SAFE_FOPEN(NULL, devnum_info_fn, "r");
+               while (fgets(line, sizeof(line), file)) {
+                       if (sscanf(line, "%[^=]=%s", devnum_info_fn, new_dev_fn_buf) != 2)
+                               continue;
+                       if (strcmp("DEVNAME", devnum_info_fn) == 0) {
+                               sprintf(dev, "/dev/%s", new_dev_fn_buf);
+                               break;
+                       }
+               }
+               SAFE_FCLOSE(NULL, file);
+       }
+
        if (stat(dev, &buf) < 0)
                tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);

--
2.34.1

On Mon, Oct 24, 2022 at 02:49:32PM +0100, Richard Palethorpe wrote:
> Hello,
> 
> Alessandro Carminati <alessandro.carminati@gmail.com> writes:
> 
> > mount_root() is the kernel function responsible for mounting the primary
> > rootfs.
> > A dynamic there, prevents the /dev/root device node in the not yet mounted
> > files system. For this reason, in the embedded system that starts without
> > an initramfs, or however a proper initscript, the /dev/root device appears
> > into the mount table in the / line.
> > The test tries to open this /dev/root and fails with a warning.
> > This patch aims to fix this situation.
> 
> Thanks I probably would have hit this issue sooner or later.
> 
> >
> > Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>
> >
> > typo fixes
> > ---
> >  lib/tst_device.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/lib/tst_device.c b/lib/tst_device.c
> > index 8419b80c3..c3427eb31 100644
> > --- a/lib/tst_device.c
> > +++ b/lib/tst_device.c
> > @@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev)
> >  	unsigned int dev_major, dev_minor, line_mjr, line_mnr;
> >  	unsigned int len, best_match_len = 1;
> >  	char mnt_point[PATH_MAX];
> > +	char tmpbuf1[PATH_MAX];
> > +	char tmpbuf2[PATH_MAX];
> 
> It would be more readable to use three buffers and give them meaningful names.
> 
> >  
> >  	if (stat(path, &buf) < 0)
> >  		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
> > @@ -562,6 +564,24 @@ void tst_find_backing_dev(const char *path, char *dev)
> >  	if (!*dev)
> >  		tst_brkm(TBROK, NULL, "Cannot find block device for %s", path);
> >  
> > +	if (stat(dev, &buf) < 0) {
> 
> As there is no harm in calling stat twice; could just do
> 
> if (stat(dev, &buf) < 0) && strcmp("/dev/root", dev) == 0) {
> 
> Or even replace with strcmp with errno == ENOENT and use this as a
> general fallback.
> 
> > +		if (strcmp("/dev/root", dev) != 0) {
> > +			tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
> > +		} else {
> 
> Then remove this if statement
> 
> > +			sprintf(tmpbuf1, "/sys/dev/block/%d:%d/uevent",
> > dev_major, dev_minor);
> > +			file = SAFE_FOPEN(NULL, tmpbuf1, "r");
> > +			while (fgets(line, sizeof(line), file)) {
> > +				if (sscanf(line, "%[^=]=%s", tmpbuf1, tmpbuf2) != 2)
> > +					continue;
> > +				if (strcmp("DEVNAME", tmpbuf1) == 0) {
> > +					sprintf(dev, "/dev/%s", tmpbuf2);
> > +					break;
> > +				}
> > +			}
> > +			SAFE_FCLOSE(NULL, file);
> > +		}
> > +	}
> > +
> >  	if (stat(dev, &buf) < 0)
> >  		tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
> >  
> > -- 
> > 2.34.1
> 
> 
> -- 
> Thank you,
> Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent
  2022-10-25  9:11   ` Alessandro Carminati
@ 2022-10-25 14:59     ` Richard Palethorpe via ltp
  2022-10-25 20:13       ` Alessandro Carminati
  2022-10-26 10:56       ` Jan Stancek
  0 siblings, 2 replies; 8+ messages in thread
From: Richard Palethorpe via ltp @ 2022-10-25 14:59 UTC (permalink / raw)
  To: ltp; +Cc: Alessandro Carminati, Richard Palethorpe

mountinfo doesn't always contain the correct device name. For example
/dev/root may be displayed, but not exist in devtmpfs[1].

The unevent file in sysfs is another way of finding the device name
from the major and minor numbers. Possibly it always displays the
proper device name.

One caveat is the sysfs can be disabled, so this commit does not
remove the mountinfo method altogether, but leaves it as a fallback.

Alessandro Carminati originally sent two patches[1] which added the
uevent file method as a fallback. However it seems like the better
method.

[1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com>
Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com>
---

Alessandro, it seems you tried to edit the last patch by hand? In any
case it did not apply and I ended up making some other changes. So I
took the liberty of submitting a new patch.

Thanks,
Richard.

 lib/tst_device.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/tst_device.c b/lib/tst_device.c
index 8419b80c3..676903fff 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -526,14 +526,30 @@ void tst_find_backing_dev(const char *path, char *dev)
 	unsigned int dev_major, dev_minor, line_mjr, line_mnr;
 	unsigned int len, best_match_len = 1;
 	char mnt_point[PATH_MAX];
+	char uevent_path[PATH_MAX];
+	char dev_name[NAME_MAX];
 
 	if (stat(path, &buf) < 0)
 		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
 
+	*dev = '\0';
 	dev_major = major(buf.st_dev);
 	dev_minor = minor(buf.st_dev);
+
+	sprintf(uevent_path,
+		"/sys/dev/block/%d:%d/uevent", dev_major, dev_minor);
+
+	if (!access(uevent_path, R_OK)) {
+		FILE_LINES_SCANF(NULL, uevent_path, "DEVNAME=%s", dev_name);
+
+		if (dev_name[0])
+			sprintf(dev, "/dev/%s", dev_name);
+	}
+
+	if (!stat(dev, &buf))
+		goto out;
+
 	file = SAFE_FOPEN(NULL, "/proc/self/mountinfo", "r");
-	*dev = '\0';
 
 	while (fgets(line, sizeof(line), file)) {
 		if (sscanf(line, "%*d %*d %d:%d %*s %s",
@@ -564,7 +580,7 @@ void tst_find_backing_dev(const char *path, char *dev)
 
 	if (stat(dev, &buf) < 0)
 		tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
-
+out:
 	if (S_ISBLK(buf.st_mode) != 1)
 		tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev);
 }
-- 
2.36.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* Re: [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent
  2022-10-25 14:59     ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp
@ 2022-10-25 20:13       ` Alessandro Carminati
  2022-10-26 10:56       ` Jan Stancek
  1 sibling, 0 replies; 8+ messages in thread
From: Alessandro Carminati @ 2022-10-25 20:13 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: ltp


[-- Attachment #1.1: Type: text/plain, Size: 3021 bytes --]

Hello,

Thank you this new version of the patch.
in this moment doc/c-test-api.txt is out of sync with the code, I can
handle its fix.

Regards
Alessandro

On Tue, Oct 25, 2022, 16:59 Richard Palethorpe <rpalethorpe@suse.com> wrote:

> mountinfo doesn't always contain the correct device name. For example
> /dev/root may be displayed, but not exist in devtmpfs[1].
>
> The unevent file in sysfs is another way of finding the device name
> from the major and minor numbers. Possibly it always displays the
> proper device name.
>
> One caveat is the sysfs can be disabled, so this commit does not
> remove the mountinfo method altogether, but leaves it as a fallback.
>
> Alessandro Carminati originally sent two patches[1] which added the
> uevent file method as a fallback. However it seems like the better
> method.
>
> [1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com>
> Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com>
> ---
>
> Alessandro, it seems you tried to edit the last patch by hand? In any
> case it did not apply and I ended up making some other changes. So I
> took the liberty of submitting a new patch.
>
> Thanks,
> Richard.
>
>  lib/tst_device.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 8419b80c3..676903fff 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -526,14 +526,30 @@ void tst_find_backing_dev(const char *path, char
> *dev)
>         unsigned int dev_major, dev_minor, line_mjr, line_mnr;
>         unsigned int len, best_match_len = 1;
>         char mnt_point[PATH_MAX];
> +       char uevent_path[PATH_MAX];
> +       char dev_name[NAME_MAX];
>
>         if (stat(path, &buf) < 0)
>                 tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
>
> +       *dev = '\0';
>         dev_major = major(buf.st_dev);
>         dev_minor = minor(buf.st_dev);
> +
> +       sprintf(uevent_path,
> +               "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor);
> +
> +       if (!access(uevent_path, R_OK)) {
> +               FILE_LINES_SCANF(NULL, uevent_path, "DEVNAME=%s",
> dev_name);
> +
> +               if (dev_name[0])
> +                       sprintf(dev, "/dev/%s", dev_name);
> +       }
> +
> +       if (!stat(dev, &buf))
> +               goto out;
> +
>         file = SAFE_FOPEN(NULL, "/proc/self/mountinfo", "r");
> -       *dev = '\0';
>
>         while (fgets(line, sizeof(line), file)) {
>                 if (sscanf(line, "%*d %*d %d:%d %*s %s",
> @@ -564,7 +580,7 @@ void tst_find_backing_dev(const char *path, char *dev)
>
>         if (stat(dev, &buf) < 0)
>                 tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
> -
> +out:
>         if (S_ISBLK(buf.st_mode) != 1)
>                 tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev);
>  }
> --
> 2.36.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 4357 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* Re: [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent
  2022-10-25 14:59     ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp
  2022-10-25 20:13       ` Alessandro Carminati
@ 2022-10-26 10:56       ` Jan Stancek
  2022-10-27  6:50         ` Alessandro Carminati
  1 sibling, 1 reply; 8+ messages in thread
From: Jan Stancek @ 2022-10-26 10:56 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: Alessandro Carminati, ltp

On Tue, Oct 25, 2022 at 4:59 PM Richard Palethorpe <rpalethorpe@suse.com> wrote:
>
> mountinfo doesn't always contain the correct device name. For example
> /dev/root may be displayed, but not exist in devtmpfs[1].
>
> The unevent file in sysfs is another way of finding the device name
         ^^ small typo here

> from the major and minor numbers. Possibly it always displays the
> proper device name.

major/minor numbers may still fail for btrfs, but that issue exists
outside of scope of this patch.

>
> One caveat is the sysfs can be disabled, so this commit does not
> remove the mountinfo method altogether, but leaves it as a fallback.

Could we add some TINFO message to output, so we know where dev_name
came from? (uevent vs mountinfo)

>
> Alessandro Carminati originally sent two patches[1] which added the
> uevent file method as a fallback. However it seems like the better
> method.
>
> [1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com>
> Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com>

Acked-by: Jan Stancek <jstancek@redhat.com>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* Re: [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent
  2022-10-26 10:56       ` Jan Stancek
@ 2022-10-27  6:50         ` Alessandro Carminati
  0 siblings, 0 replies; 8+ messages in thread
From: Alessandro Carminati @ 2022-10-27  6:50 UTC (permalink / raw)
  To: ltp


[-- Attachment #1.1: Type: text/plain, Size: 1551 bytes --]

Hello,

Just proposed a new patch version for the same topic
https://lore.kernel.org/ltp/20221026140408.471609-1-alessandro.carminati@gmail.com/

cheers
Alessandro Carminati


Il giorno mer 26 ott 2022 alle ore 12:56 Jan Stancek <jstancek@redhat.com>
ha scritto:

> On Tue, Oct 25, 2022 at 4:59 PM Richard Palethorpe <rpalethorpe@suse.com>
> wrote:
> >
> > mountinfo doesn't always contain the correct device name. For example
> > /dev/root may be displayed, but not exist in devtmpfs[1].
> >
> > The unevent file in sysfs is another way of finding the device name
>          ^^ small typo here
>
> > from the major and minor numbers. Possibly it always displays the
> > proper device name.
>
> major/minor numbers may still fail for btrfs, but that issue exists
> outside of scope of this patch.
>
> >
> > One caveat is the sysfs can be disabled, so this commit does not
> > remove the mountinfo method altogether, but leaves it as a fallback.
>
> Could we add some TINFO message to output, so we know where dev_name
> came from? (uevent vs mountinfo)
>
> >
> > Alessandro Carminati originally sent two patches[1] which added the
> > uevent file method as a fallback. However it seems like the better
> > method.
> >
> > [1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/
> >
> > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> > Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com>
> > Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com>
>
> Acked-by: Jan Stancek <jstancek@redhat.com>
>
>

[-- Attachment #1.2: Type: text/html, Size: 2793 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* Re: [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs
@ 2022-10-21 14:18 Alessandro Carminati
  0 siblings, 0 replies; 8+ messages in thread
From: Alessandro Carminati @ 2022-10-21 14:18 UTC (permalink / raw)
  To: ltp

After having discussed this patch content with Jan Stancek, I realized that 
my patch log is too minimal.
By this message, I want to substantiate the case.
I will start by saying the problem I want to fix happens in minimal systems: 
the kernel and minimal onit scripts.
This is the case of many embedded system powered by linux.
Here is the link of a minimal system I used to test the patch.
https://gitlab.com/acarmina/test-files/-/blob/main/testenv.tar.gz
In such a system, you have:

# cat /proc/cmdline 
console=ttyS0 root=/dev/sda rw   rootwait
# cat /proc/self/mountinfo 
14 1 8:0 / / rw,relatime - ext2 /dev/root rw
15 14 0:5 / /dev rw,relatime - devtmpfs devtmpfs rw,size=505412k,nr_inodes=126353,mode=755
16 14 0:13 / /proc rw,relatime - proc proc rw
17 15 0:14 / /dev/pts rw,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=666
18 15 0:15 / /dev/shm rw,relatime - tmpfs tmpfs rw,mode=777
19 14 0:16 / /tmp rw,relatime - tmpfs tmpfs rw
20 14 0:17 / /run rw,nosuid,nodev,relatime - tmpfs tmpfs rw,mode=755
21 14 0:18 / /sys rw,relatime - sysfs sysfs rw
# /usr/lib/ltp-testsuite/testcases/bin/ioctl_loop05
tst_test.c:1363: TINFO: Timeout per run is 0h 05m 00s
tst_test.c:1115: TINFO: ext2/ext3/ext4 is supported by the test
tst_device.c:88: TINFO: Found free device 0 '/dev/loop0'
loop0: detected capacity change from 0 to 2048
tst_device.c:566: TWARN: stat(/dev/root) failed: ENOENT (2)

Summary:
passed   0
failed   0
broken   0
skipped  0
warnings 1
#

/dev/root is simply not there, and the test fails with a warning.

In the systems where the /dev/root is there, it is typically a symbolic link 
to the actual device.

Digging on why this device is not there, despite the mount info reports the / 
to be mounted over it, I discovered something weird in the early kernel 
initialization:
https://elixir.bootlin.com/linux/v6.0.3/source/init/do_mounts.c#L566
In this recent kernel release it is evident how the kernel creates the 
/dev/root device node and after it mounts the new device on the / .

The problem is not widely known since in systems where the /dev/root is 
used, typically, the init scripts handle it.
Although it may look strange, this behaviour is considered the intended 
behaviour for the kernel.
An indirect proof is that as early as 2013 a patch aimed to fix this 
behaviour circulated in the kernel mailing lists 
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg378443.html
But it never made to the upstream

Assuming the current the intended kernel behaviour, the thesis I'm 
trying to push is that the test, meant to test the kernel, should not 
rely on work done in userspace init scripts.

In the patch I am pushing, my intent is to deal with this /dev/root 
peculiarity at the LTP level, by basically add a logic that replaces 
the actual device in case of stat failing on the /dev/root case.

I hope this make my patch context clear.

Cheers
Alessandro

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

end of thread, other threads:[~2022-10-27  6:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-17 11:05 [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs Alessandro Carminati
2022-10-24 13:49 ` Richard Palethorpe
2022-10-25  9:11   ` Alessandro Carminati
2022-10-25 14:59     ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp
2022-10-25 20:13       ` Alessandro Carminati
2022-10-26 10:56       ` Jan Stancek
2022-10-27  6:50         ` Alessandro Carminati
2022-10-21 14:18 [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs Alessandro Carminati

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.