| Message ID | d5f99997-1156-f7d3-872b-8b1f360f25e8@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 29/06/2016 12:11, Josua Mayer wrote: > Hi John, > > thansk for taking a look. I actually sent it this way by intention, > looking to get in touch with someone who had the original bug. > > Am 29.06.2016 um 08:30 schrieb John Crispin: >> Hi, >> >> the patch is an attachement making inline commenting impossible. please >> send patches inline > From 59a8fa6d7490ba3da76aec710a1beb241ffaa089 Mon Sep 17 00:00:00 2001 > From: Josua Mayer <josua.mayer97@gmail.com> > Date: Thu, 16 Jun 2016 18:35:30 +0200 > Subject: [PATCH 2/4] mount_root: Don't mount ext4 rootfs twice > > When there is a) no rootfs_data overlay partition, > and b) /dev/root points to an ext4 partition > the partition would be mounted twice, once as / and then as /overlay. > The essence of this change is to return before mounting /overlay, > if /dev/root has been mounted as /. >> > Signed-off-by: Josua Mayer <josua.mayer97@gmail.com> > --- > mount_root.c | 40 ++++++++++++++++++++++++++++++---------- > 1 file changed, 30 insertions(+), 10 deletions(-) > > diff --git a/mount_root.c b/mount_root.c > index 608ce5d..13e5772 100644 > --- a/mount_root.c > +++ b/mount_root.c > @@ -37,25 +37,45 @@ start(int argc, char *argv[1]) > if (!getenv("PREINIT") && stat("/tmp/.preinit", &s)) > return -1; > > + /* > + * When the default overlay partition name rootfs_data can not be found, > + * fall back to the special /dev/root device. > + */ > if (!data) { > root = volume_find("rootfs"); > volume_init(root); > + > + // mount /dev/root at / > ULOG_NOTE("mounting /dev/root\n"); > mount("/dev/root", "/", NULL, MS_NOATIME | MS_REMOUNT, 0); > - } > > - /* > - * Before trying to mount and use "rootfs_data" let's check if there is > - * extroot configured. Following call will handle reading config from > - * the "rootfs_data" on its own. > - */ > - extroot_prefix = ""; > - if (!mount_extroot()) { > - ULOG_NOTE("switched to extroot\n"); > + /* > + * Now that / has been mounted, and there is no overlay device, > + * see if extroot is configured. > + * > + * The following call will handle reading configuration from > + * rootfs on its own. > + */ > + extroot_prefix = ""; > + if (!mount_extroot()) { > + ULOG_NOTE("switched to extroot\n"); > + /* > + * extroot succeeded mounting an overlay partition, return. > + */ > + return 0; > + } > + > + /* > + * Even if extroot was not configured, considering that no overlay > + * partition was found, and / was mounted, return now. > + */ > return 0; > } > > - /* There isn't extroot, so just try to mount "rootfs_data" */ > + /* > + * neither /dev/root nor extroot were used. > + * Attempt to mount the overlay partition. > + */ > switch (volume_identify(data)) { > case FS_NONE: > ULOG_WARN("no usable overlay filesystem found, using tmpfs overlay\n"); > derived from looking at the patch. as the patch is not inline i am not able to actually import it locally so i can only judge it by looking at it. after applying your patch, mount_extroot() would be hidden behind a if(!data) {} which means that it will only ever be called if there is no rootfs_data John
Am 29.06.2016 um 12:21 schrieb John Crispin: > > > On 29/06/2016 12:11, Josua Mayer wrote: >> Hi John, >> >> thansk for taking a look. I actually sent it this way by intention, >> looking to get in touch with someone who had the original bug. >> >> Am 29.06.2016 um 08:30 schrieb John Crispin: >>> Hi, >>> >>> the patch is an attachement making inline commenting impossible. please >>> send patches inline >> From 59a8fa6d7490ba3da76aec710a1beb241ffaa089 Mon Sep 17 00:00:00 2001 >> From: Josua Mayer <josua.mayer97@gmail.com> >> Date: Thu, 16 Jun 2016 18:35:30 +0200 >> Subject: [PATCH 2/4] mount_root: Don't mount ext4 rootfs twice >> >> When there is a) no rootfs_data overlay partition, >> and b) /dev/root points to an ext4 partition >> the partition would be mounted twice, once as / and then as /overlay. >> The essence of this change is to return before mounting /overlay, >> if /dev/root has been mounted as /. >>> >> Signed-off-by: Josua Mayer <josua.mayer97@gmail.com> >> --- >> mount_root.c | 40 ++++++++++++++++++++++++++++++---------- >> 1 file changed, 30 insertions(+), 10 deletions(-) >> >> diff --git a/mount_root.c b/mount_root.c >> index 608ce5d..13e5772 100644 >> --- a/mount_root.c >> +++ b/mount_root.c >> @@ -37,25 +37,45 @@ start(int argc, char *argv[1]) >> if (!getenv("PREINIT") && stat("/tmp/.preinit", &s)) >> return -1; >> >> + /* >> + * When the default overlay partition name rootfs_data can not be found, >> + * fall back to the special /dev/root device. >> + */ >> if (!data) { >> root = volume_find("rootfs"); >> volume_init(root); >> + >> + // mount /dev/root at / >> ULOG_NOTE("mounting /dev/root\n"); >> mount("/dev/root", "/", NULL, MS_NOATIME | MS_REMOUNT, 0); >> - } >> >> - /* >> - * Before trying to mount and use "rootfs_data" let's check if there is >> - * extroot configured. Following call will handle reading config from >> - * the "rootfs_data" on its own. >> - */ >> - extroot_prefix = ""; >> - if (!mount_extroot()) { >> - ULOG_NOTE("switched to extroot\n"); >> + /* >> + * Now that / has been mounted, and there is no overlay device, >> + * see if extroot is configured. >> + * >> + * The following call will handle reading configuration from >> + * rootfs on its own. >> + */ >> + extroot_prefix = ""; >> + if (!mount_extroot()) { >> + ULOG_NOTE("switched to extroot\n"); >> + /* >> + * extroot succeeded mounting an overlay partition, return. >> + */ >> + return 0; >> + } >> + >> + /* >> + * Even if extroot was not configured, considering that no overlay >> + * partition was found, and / was mounted, return now. >> + */ >> return 0; >> } >> >> - /* There isn't extroot, so just try to mount "rootfs_data" */ >> + /* >> + * neither /dev/root nor extroot were used. >> + * Attempt to mount the overlay partition. >> + */ >> switch (volume_identify(data)) { >> case FS_NONE: >> ULOG_WARN("no usable overlay filesystem found, using tmpfs overlay\n"); >> > > derived from looking at the patch. as the patch is not inline i am not > able to actually import it locally so i can only judge it by looking at it. > > after applying your patch, mount_extroot() would be hidden behind a > if(!data) {} which means that it will only ever be called if there is no > rootfs_data That is only partially right. Like I pointed out, it is called inside mount_overlay(). If there is a way to fix this, I need to understand what should happen: code path before my patch: data = volume_find("rootfs_data"); if (!data) {/* mount /dev/root as / */} mount_extroot() switch (volume_identify(data)) From this artificial trace, I extracted these runtime conditions for extroot: 1. rootfs_data exists but not mounted yet 2. rootfs_data does not exist and /dev/root mounted as / I then guessed that mount_extroot only makes sense when there is an overlay partition, or the read-write rootfs mounted in order to have its configuration available. So my guess was: if overlay isnt mounted yet but will be later, no point in executing mount_extroot. Especially considering it will be inherently called from the call to mount_overlay thats occuring later on. > > John >
On 29/06/2016 12:34, Josua Mayer wrote: > > > Am 29.06.2016 um 12:21 schrieb John Crispin: >> >> >> On 29/06/2016 12:11, Josua Mayer wrote: >>> Hi John, >>> >>> thansk for taking a look. I actually sent it this way by intention, >>> looking to get in touch with someone who had the original bug. >>> >>> Am 29.06.2016 um 08:30 schrieb John Crispin: >>>> Hi, >>>> >>>> the patch is an attachement making inline commenting impossible. please >>>> send patches inline >>> From 59a8fa6d7490ba3da76aec710a1beb241ffaa089 Mon Sep 17 00:00:00 2001 >>> From: Josua Mayer <josua.mayer97@gmail.com> >>> Date: Thu, 16 Jun 2016 18:35:30 +0200 >>> Subject: [PATCH 2/4] mount_root: Don't mount ext4 rootfs twice >>> >>> When there is a) no rootfs_data overlay partition, >>> and b) /dev/root points to an ext4 partition >>> the partition would be mounted twice, once as / and then as /overlay. >>> The essence of this change is to return before mounting /overlay, >>> if /dev/root has been mounted as /. >>>> >>> Signed-off-by: Josua Mayer <josua.mayer97@gmail.com> >>> --- >>> mount_root.c | 40 ++++++++++++++++++++++++++++++---------- >>> 1 file changed, 30 insertions(+), 10 deletions(-) >>> >>> diff --git a/mount_root.c b/mount_root.c >>> index 608ce5d..13e5772 100644 >>> --- a/mount_root.c >>> +++ b/mount_root.c >>> @@ -37,25 +37,45 @@ start(int argc, char *argv[1]) >>> if (!getenv("PREINIT") && stat("/tmp/.preinit", &s)) >>> return -1; >>> >>> + /* >>> + * When the default overlay partition name rootfs_data can not be found, >>> + * fall back to the special /dev/root device. >>> + */ >>> if (!data) { >>> root = volume_find("rootfs"); >>> volume_init(root); >>> + >>> + // mount /dev/root at / >>> ULOG_NOTE("mounting /dev/root\n"); >>> mount("/dev/root", "/", NULL, MS_NOATIME | MS_REMOUNT, 0); >>> - } >>> >>> - /* >>> - * Before trying to mount and use "rootfs_data" let's check if there is >>> - * extroot configured. Following call will handle reading config from >>> - * the "rootfs_data" on its own. >>> - */ >>> - extroot_prefix = ""; >>> - if (!mount_extroot()) { >>> - ULOG_NOTE("switched to extroot\n"); >>> + /* >>> + * Now that / has been mounted, and there is no overlay device, >>> + * see if extroot is configured. >>> + * >>> + * The following call will handle reading configuration from >>> + * rootfs on its own. >>> + */ >>> + extroot_prefix = ""; >>> + if (!mount_extroot()) { >>> + ULOG_NOTE("switched to extroot\n"); >>> + /* >>> + * extroot succeeded mounting an overlay partition, return. >>> + */ >>> + return 0; >>> + } >>> + >>> + /* >>> + * Even if extroot was not configured, considering that no overlay >>> + * partition was found, and / was mounted, return now. >>> + */ >>> return 0; >>> } >>> >>> - /* There isn't extroot, so just try to mount "rootfs_data" */ >>> + /* >>> + * neither /dev/root nor extroot were used. >>> + * Attempt to mount the overlay partition. >>> + */ >>> switch (volume_identify(data)) { >>> case FS_NONE: >>> ULOG_WARN("no usable overlay filesystem found, using tmpfs overlay\n"); >>> >> >> derived from looking at the patch. as the patch is not inline i am not >> able to actually import it locally so i can only judge it by looking at it. >> >> after applying your patch, mount_extroot() would be hidden behind a >> if(!data) {} which means that it will only ever be called if there is no >> rootfs_data > That is only partially right. Like I pointed out, it is called inside > mount_overlay(). > If there is a way to fix this, I need to understand what should happen: > code path before my patch: > data = volume_find("rootfs_data"); > if (!data) {/* mount /dev/root as / */} > mount_extroot() > switch (volume_identify(data)) > > From this artificial trace, I extracted these runtime conditions for > extroot: > 1. rootfs_data exists but not mounted yet > 2. rootfs_data does not exist and /dev/root mounted as / > I then guessed that mount_extroot only makes sense when there is an > overlay partition, or the read-write rootfs mounted in order to have its > configuration available. > So my guess was: if overlay isnt mounted yet but will be later, no point > in executing mount_extroot. Especially considering it will be inherently > called from the call to mount_overlay thats occuring later on. >> >> John >> > > _______________________________________________ > Lede-dev mailing list > Lede-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/lede-dev > send a proper inline patch so i can test this on device please. John
diff --git a/mount_root.c b/mount_root.c index 608ce5d..13e5772 100644 --- a/mount_root.c +++ b/mount_root.c @@ -37,25 +37,45 @@ start(int argc, char *argv[1]) if (!getenv("PREINIT") && stat("/tmp/.preinit", &s)) return -1; + /* + * When the default overlay partition name rootfs_data can not be found, + * fall back to the special /dev/root device. + */ if (!data) { root = volume_find("rootfs"); volume_init(root); + + // mount /dev/root at / ULOG_NOTE("mounting /dev/root\n"); mount("/dev/root", "/", NULL, MS_NOATIME | MS_REMOUNT, 0); - } - /* - * Before trying to mount and use "rootfs_data" let's check if there is - * extroot configured. Following call will handle reading config from - * the "rootfs_data" on its own. - */ - extroot_prefix = ""; - if (!mount_extroot()) { - ULOG_NOTE("switched to extroot\n"); + /* + * Now that / has been mounted, and there is no overlay device, + * see if extroot is configured. + * + * The following call will handle reading configuration from + * rootfs on its own. + */ + extroot_prefix = ""; + if (!mount_extroot()) { + ULOG_NOTE("switched to extroot\n"); + /* + * extroot succeeded mounting an overlay partition, return. + */ + return 0; + } + + /* + * Even if extroot was not configured, considering that no overlay + * partition was found, and / was mounted, return now. + */ return 0; } - /* There isn't extroot, so just try to mount "rootfs_data" */ + /* + * neither /dev/root nor extroot were used. + * Attempt to mount the overlay partition. + */ switch (volume_identify(data)) { case FS_NONE: ULOG_WARN("no usable overlay filesystem found, using tmpfs overlay\n");