On Tue, May 25, 2021 at 08:55:48AM +0800, Menglong Dong wrote:

On Tue, May 25, 2021 at 6:58 AM Luis Chamberlain [off-list ref] wrote:

quoted

However, if you introduce it as a kconfig option so that users
who want to use this new feature can enable it, and then use it,
the its sold as a new feature.

Should this always be enabled, or done this way? Should we never have
the option to revert back to the old behaviour? If not, why not?

This change seems transparent to users, which don't change the behavior
of initramfs. 

Are we sure there nothing in the kernel that can regress with this
change? Are you sure? How sure?

However, it seems more reasonable to make it a kconfig option.
I'll do it in the v2 of the three patches I sended.

I'm actually quite convinced now this is a desirable default *other*
than the concern if this could regress. I recently saw some piece of
code fetching for the top most mount, I think it was on the
copy_user_ns() path or something like that, which made me just
consider possible regressions for heuristics we might have forgotten
about.

I however have't yet had time to review the path I was concerned for
yet.

quoted

What do you mean? init_mount_tree() is always called, and it has
statically:

static void __init init_mount_tree(void)
{
        struct vfsmount *mnt;
        ...
        mnt = vfs_kern_mount(&rootfs_fs_type, 0, "rootfs", NULL);
        ...
}

And as I noted, this is *always* called earlier than
do_populate_rootfs(). Your changes did not remove the init_mount_tree()
or modify it, and so why would the context of the above call always
be OK to be used now with a ramfs context now?

quoted

So it makes no sense to make the file system of the first mount selectable.

Why? I don't see why, nor is it explained, we're always caling
vfs_kern_mount(&rootfs_fs_type, ...) and you have not changed that
either.

quoted

To simplify the code here, I make it ramfs_init_fs_context directly. In fact,
it's fine to make it shmen_init_fs_context here too.

So indeed you're suggesting its arbitrary now.... I don't see why.

So the biggest problem now seems to be the first mount I changed, maybe I didn't
make it clear before.

Let's call the first mount which is created in init_mount_tree() the
'init_mount'.
If the 'root' is a block fs, initrd or nfs, the 'init_mount' will be a
ramfs, that seems
clear, it can be seen from the enable of tmpfs:

void __init init_rootfs(void)
{
    if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
       (!root_fs_names || strstr(root_fs_names, "tmpfs")))
       is_tmpfs = true;
}

Ah yes, I see now... Thanks!
 
  Luis