[PATCH] use initmpfs even if there's root= cmdline

[PATCH] use initmpfs even if there's root= cmdline

[Dave Young]

Current code use ramfs instead of tmpfs for stub when root= defined.

But for real use case with initramfs, usually there's like cmdline like
root=UUID=$UUID the root dev is the real device. For that case we have
no way to use initmpfs, thus this patch removes the limitation so tmpfs
can benefit more people.

The logic become:
if CONFIG_TMPFS && rootfstype is not ramfs
    use tmpfs
else
    use ramfs

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 init/do_mounts.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 8e5addc..6fde471 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -622,8 +622,8 @@ int __init init_rootfs(void)
 	if (err)
 		return err;
 
-	if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
-		(!root_fs_names || strstr(root_fs_names, "tmpfs"))) {
+	if (IS_ENABLED(CONFIG_TMPFS) &&
+		(root_fs_names && !(strstr(root_fs_names, "ramfs")))) {
 		err = shmem_init();
 		is_tmpfs = true;
 	} else {

Re: [PATCH] use initmpfs even if there's root= cmdline

[Dave Young]

On Thu, Dec 12, 2013 at 05:25:42PM +0800, Dave Young wrote:
> 
> Current code use ramfs instead of tmpfs for stub when root= defined.
> 
> But for real use case with initramfs, usually there's like cmdline like
> root=UUID=$UUID the root dev is the real device. For that case we have
> no way to use initmpfs, thus this patch removes the limitation so tmpfs
> can benefit more people.
> 
> The logic become:
> if CONFIG_TMPFS && rootfstype is not ramfs
>     use tmpfs
> else
>     use ramfs
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  init/do_mounts.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/init/do_mounts.c b/init/do_mounts.c
> index 8e5addc..6fde471 100644
> --- a/init/do_mounts.c
> +++ b/init/do_mounts.c
> @@ -622,8 +622,8 @@ int __init init_rootfs(void)
>  	if (err)
>  		return err;
>  
> -	if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
> -		(!root_fs_names || strstr(root_fs_names, "tmpfs"))) {
> +	if (IS_ENABLED(CONFIG_TMPFS) &&
> +		(root_fs_names && !(strstr(root_fs_names, "ramfs")))) {

Oops, I probably mistakenly tested with "rootfstype=tmpfs". Rethinking about
it, below should be what I want:
	if (IS_ENABLED(CONFIG_TMPFS) &&
		(!root_fs_names || strstr(root_fs_names, "tmpfs"))) {

I will retest it tomorrow.

Qeustion to Rob Landley:
Rob, do you remember why you added the checking for root=? git log mentioned
below which is not clear:
use ramfs instead of tmpfs for stub when root= defined (for cosmetic
reasons).

--
Thanks
Dave

Re: [PATCH] use initmpfs even if there's root= cmdline

[Dave Young]

On 12/12/13 at 05:25pm, Dave Young wrote:
> 
> Current code use ramfs instead of tmpfs for stub when root= defined.
> 
> But for real use case with initramfs, usually there's like cmdline like
> root=UUID=$UUID the root dev is the real device. For that case we have
> no way to use initmpfs, thus this patch removes the limitation so tmpfs
> can benefit more people.
> 
> The logic become:
> if CONFIG_TMPFS && rootfstype is not ramfs
>     use tmpfs
> else
>     use ramfs
> 

Discussed with Vivek Goyal about the kdump use case, I missed one thing that
tmpfs has default size limit though we can tune it.

So I will think more about it, will address this later, please ignore this
patch.

Thanks
Dave

Re: [PATCH] use initmpfs even if there's root= cmdline

[Rob Landley]

On 12/12/13 20:38, Dave Young wrote:
> On 12/12/13 at 05:25pm, Dave Young wrote:
>>
>> Current code use ramfs instead of tmpfs for stub when root= defined.
>>
>> But for real use case with initramfs, usually there's like cmdline like
>> root=UUID=$UUID the root dev is the real device. For that case we have
>> no way to use initmpfs, thus this patch removes the limitation so tmpfs
>> can benefit more people.

The reason I did that was if you specify a root= then you don't want to 
_stay_ on rootfs. You specify root= so either the kernel does 
switch_root for you, or so rootfs does a swich_root at the end.

The point of initmpfs is that when rootfs _is_ the "real" root device, 
it can benefit from being tmpfs. When you're just goign to switch to a 
different root device, tmpfs doesn't make much difference.

> Discussed with Vivek Goyal about the kdump use case, I missed one thing that
> tmpfs has default size limit though we can tune it.
>
> So I will think more about it, will address this later, please ignore this
> patch.

I have a vague todo item of feeding rootflags= through to initmpfs, but 
that's really intended to specify flags for root=. There isn't really an 
existing command line option to specify initramfs flags because ramfs 
doesn't care.

It was one of those "only parse rootflags= for initmpfs when there's no 
root=" vs "create a new rdrootflags= ala rdinit= even though that's a 
subtly wrong name these days..." and it went on the todo list because 
neither approach was obviously superior.

Happy to take suggestions and whip up a patch if this is inconveniencing 
somebody. :)

> Thanks
> Dave

Rob

Re: [PATCH] use initmpfs even if there's root= cmdline

[Dave Young]

On Wed, Dec 18, 2013 at 11:51:30AM -0600, Rob Landley wrote:
> 
> 
> On 12/12/13 20:38, Dave Young wrote:
> >On 12/12/13 at 05:25pm, Dave Young wrote:
> >>
> >>Current code use ramfs instead of tmpfs for stub when root= defined.
> >>
> >>But for real use case with initramfs, usually there's like cmdline like
> >>root=UUID=$UUID the root dev is the real device. For that case we have
> >>no way to use initmpfs, thus this patch removes the limitation so tmpfs
> >>can benefit more people.
> 
> The reason I did that was if you specify a root= then you don't want
> to _stay_ on rootfs. You specify root= so either the kernel does
> switch_root for you, or so rootfs does a swich_root at the end.
> 
> The point of initmpfs is that when rootfs _is_ the "real" root
> device, it can benefit from being tmpfs. When you're just goign to
> switch to a different root device, tmpfs doesn't make much
> difference.

The reason make sense to most of users. Thanks for the info.
For Fedora kdump initramfs there's different requirement though, we do the
vmcore capturing in ramfs with root=, the root= is not necessay in most of
the cases because we will reboot immediately after vmcore capturing finish.
There's one potential exception is that we could switch to real root in
case of capturing failure. Another thing is we use dracut to create initramfs
and dracut has a limitation that root= is a must-have param.

> 
> >Discussed with Vivek Goyal about the kdump use case, I missed one thing that
> >tmpfs has default size limit though we can tune it.
> >
> >So I will think more about it, will address this later, please ignore this
> >patch.
> 
> I have a vague todo item of feeding rootflags= through to initmpfs,
> but that's really intended to specify flags for root=. There isn't
> really an existing command line option to specify initramfs flags
> because ramfs doesn't care.
> 
> It was one of those "only parse rootflags= for initmpfs when there's
> no root=" vs "create a new rdrootflags= ala rdinit= even though
> that's a subtly wrong name these days..." and it went on the todo
> list because neither approach was obviously superior.
> 
> Happy to take suggestions and whip up a patch if this is
> inconveniencing somebody. :)

It would be great that initmpfs can use the whole memory on demand by default at
the same time we can avoid the deadlock mentioned about OOM handler.

For this purpose no need for extra flags? For other flags maybe "initmpfsflags="?

Thanks
Dave

Re: [PATCH] use initmpfs even if there's root= cmdline

[Rob Landley]

On 12/18/13 20:35, Dave Young wrote:
> On Wed, Dec 18, 2013 at 11:51:30AM -0600, Rob Landley wrote:
>>
>>
>> On 12/12/13 20:38, Dave Young wrote:
>>> On 12/12/13 at 05:25pm, Dave Young wrote:
>>>>
>>>> Current code use ramfs instead of tmpfs for stub when root= defined.
>>>>
>>>> But for real use case with initramfs, usually there's like cmdline like
>>>> root=UUID=$UUID the root dev is the real device. For that case we have
>>>> no way to use initmpfs, thus this patch removes the limitation so tmpfs
>>>> can benefit more people.
>>
>> The reason I did that was if you specify a root= then you don't want
>> to _stay_ on rootfs. You specify root= so either the kernel does
>> switch_root for you, or so rootfs does a swich_root at the end.
>>
>> The point of initmpfs is that when rootfs _is_ the "real" root
>> device, it can benefit from being tmpfs. When you're just goign to
>> switch to a different root device, tmpfs doesn't make much
>> difference.
>
> The reason make sense to most of users. Thanks for the info.
> For Fedora kdump initramfs there's different requirement though, we do the
> vmcore capturing in ramfs with root=, the root= is not necessay in most of
> the cases because we will reboot immediately after vmcore capturing finish.
> There's one potential exception is that we could switch to real root in
> case of capturing failure. Another thing is we use dracut to create initramfs
> and dracut has a limitation that root= is a must-have param.

Interesting use case.

I can't do anything about dracut. (Otherwise userspace could look at 
ROOT= which is not parsed by existing kernel code to have a special 
meaning...)

>>> Discussed with Vivek Goyal about the kdump use case, I missed one thing that
>>> tmpfs has default size limit though we can tune it.
>>>
>>> So I will think more about it, will address this later, please ignore this
>>> patch.
>>
>> I have a vague todo item of feeding rootflags= through to initmpfs,
>> but that's really intended to specify flags for root=. There isn't
>> really an existing command line option to specify initramfs flags
>> because ramfs doesn't care.
>>
>> It was one of those "only parse rootflags= for initmpfs when there's
>> no root=" vs "create a new rdrootflags= ala rdinit= even though
>> that's a subtly wrong name these days..." and it went on the todo
>> list because neither approach was obviously superior.
>>
>> Happy to take suggestions and whip up a patch if this is
>> inconveniencing somebody. :)
>
> It would be great that initmpfs can use the whole memory on demand by default at
> the same time we can avoid the deadlock mentioned about OOM handler.

Except that if initmpfs size=100% uses all the memory, we're back to 
"fill up the filesystem and the kernel deadlocks" same as initramfs.

Possibly I could tell it to go to 100% during extract and then remount 
to 50% afterwards? (The cpio archive filling up memory is pilot error. I 
wonder what happens if tmpfs is told to remount using less space than 
the filesystem currently has, does it fail or does it adjust the amount 
to what's currently used, or...?)

If necessary I could query the filesystem size after extract and remount 
to that amount or 50% (whichever is greater), but this is getting more 
complicated than seems strictly necessary? (If rootflags= got passed 
through we'd then want to only do this if they _didn't_ specify a size, 
or else have the "actual size or size= the user passed"...)

Implementing behavior is easy. Determing what the correct behavior 
should be is hard.

> For this purpose no need for extra flags? For other flags maybe "initmpfsflags="?

You can already say rootfstype=ramfs if you want to drop back to a 
filesystem that doesn't care about size checks. I could trivially make 
it so rootfstype=tmpfs overrides the root= check, but then there's that 
50% memory limitation on the filesystem size. (Beyond which the disk 
cache reaping logic gets VERY UNHAPPY, by the way. The automatic load 
balancing stuff is... touchy. Better now than it used to be, but I still 
trust it about as far as I could comfortably spit out a rat. Then again 
I'm generally not using systems with half a terabyte of ram, which is 
what they all seem to be optimizing for these days...)

Adding initrdflags= to specify a size= other than 50% makes sense, but 
you might have to modify your dracut thing to do a du on the filesystem 
before packaging it and feed in the correct number of kilobytes.

If you want a different behavior than that, please explain what it 
should do in a little more detail?

> Thanks
> Dave

Rob

Re: [PATCH] use initmpfs even if there's root= cmdline

[Dave Young]

> I can't do anything about dracut. (Otherwise userspace could look at
> ROOT= which is not parsed by existing kernel code to have a special
> meaning...)

The root= requirement for dracut is not necessary just there's no one
is working on removing the limitation. I think that is not the key
problem here.

> 
> >>>Discussed with Vivek Goyal about the kdump use case, I missed one thing that
> >>>tmpfs has default size limit though we can tune it.
> >>>
> >>>So I will think more about it, will address this later, please ignore this
> >>>patch.
> >>
> >>I have a vague todo item of feeding rootflags= through to initmpfs,
> >>but that's really intended to specify flags for root=. There isn't
> >>really an existing command line option to specify initramfs flags
> >>because ramfs doesn't care.
> >>
> >>It was one of those "only parse rootflags= for initmpfs when there's
> >>no root=" vs "create a new rdrootflags= ala rdinit= even though
> >>that's a subtly wrong name these days..." and it went on the todo
> >>list because neither approach was obviously superior.
> >>
> >>Happy to take suggestions and whip up a patch if this is
> >>inconveniencing somebody. :)
> >
> >It would be great that initmpfs can use the whole memory on demand by default at
> >the same time we can avoid the deadlock mentioned about OOM handler.
> 
> Except that if initmpfs size=100% uses all the memory, we're back to
> "fill up the filesystem and the kernel deadlocks" same as initramfs.
> 
> Possibly I could tell it to go to 100% during extract and then
> remount to 50% afterwards? (The cpio archive filling up memory is
> pilot error. I wonder what happens if tmpfs is told to remount using
> less space than the filesystem currently has, does it fail or does
> it adjust the amount to what's currently used, or...?)

Tested that remounting to less space will success if there's enough free
space in the original mount. But I think it's hard to guess how much the
initrd will use while booting, it might need create other files dynamically. 

> 
> If necessary I could query the filesystem size after extract and
> remount to that amount or 50% (whichever is greater), but this is
> getting more complicated than seems strictly necessary? (If
> rootflags= got passed through we'd then want to only do this if they
> _didn't_ specify a size, or else have the "actual size or size= the
> user passed"...)
> 
> Implementing behavior is easy. Determing what the correct behavior
> should be is hard.
> 
> >For this purpose no need for extra flags? For other flags maybe "initmpfsflags="?
> 
> You can already say rootfstype=ramfs if you want to drop back to a
> filesystem that doesn't care about size checks. I could trivially
> make it so rootfstype=tmpfs overrides the root= check, but then
> there's that 50% memory limitation on the filesystem size. (Beyond
> which the disk cache reaping logic gets VERY UNHAPPY, by the way.
> The automatic load balancing stuff is... touchy. Better now than it
> used to be, but I still trust it about as far as I could comfortably
> spit out a rat. Then again I'm generally not using systems with half
> a terabyte of ram, which is what they all seem to be optimizing for
> these days...)

Yes, use rootfstype=ramfs is ok for dropping back to initramfs. But
rethinking about using tmpfs as default type, it might break current
users which using initramfs which does not have size limitation.
So I think for now keep the current logic is fine until we have a
better way to improve tmpfs.

> 
> Adding initrdflags= to specify a size= other than 50% makes sense,
> but you might have to modify your dracut thing to do a du on the
> filesystem before packaging it and feed in the correct number of
> kilobytes.

initrdflags looks more like for the original block device based initrd?
maybe tmpfs.default_size=?

> 
> If you want a different behavior than that, please explain what it
> should do in a little more detail?

I think I have given up the idea based on problems we have talked about..

Thanks
Dave