From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3604C2B9F7 for ; Tue, 25 May 2021 01:43:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8082A6141B for ; Tue, 25 May 2021 01:43:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229891AbhEYBog (ORCPT ); Mon, 24 May 2021 21:44:36 -0400 Received: from mail-pf1-f170.google.com ([209.85.210.170]:45987 "EHLO mail-pf1-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229550AbhEYBog (ORCPT ); Mon, 24 May 2021 21:44:36 -0400 Received: by mail-pf1-f170.google.com with SMTP id d16so22318290pfn.12; Mon, 24 May 2021 18:43:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=0oqSXpZzradp5Zo/ArJgl1DR73porgfQSQ9emcPhZCI=; b=U0iPb8uuqegZKN4c2f2NlNJFDZ+1ZbRpvjPEmiXG36Y6MOgfSieAxp25R572sBIlk4 MfL4j4CCM+/K3ms07lrYN0wf0GCoh8vx///N1iZE1l96NCf7Wvh4A63gOuTHnczYO33o Pdn9K6kQ24rslqKDvZ+eg+CYvYia1bFlihiH2Icx6W/sguK6VKk5OygNKoXrsi3Trs1B nxq7iervLKfjfayw1HXJiQIalftn8fwohEzHI2Da2FlesCpX+o9AESqLEFCg3FaE3uHz B9XABG6NC3XPT7tDk4CtdUAVU+QMV7yv4l+2MkSYY0eLQVaBxGGqrAkC6+isfQw2W5Bc YuNw== X-Gm-Message-State: AOAM530FG5Jh4kR1Y3EelzlGCj/ymWfHn8BgWk5fl4HHmGLY7ksQX0n0 SmUqxTQBkpBtqOTVZ8BiG3Q= X-Google-Smtp-Source: ABdhPJzJ58M4UJCtps9mLIJEzmfJMs1MGhvR1rBmd0Yt1mCHBRdWfKECHL8mOFrrSHSnXtaHjszqrQ== X-Received: by 2002:a05:6a00:bc2:b029:2df:93cc:371a with SMTP id x2-20020a056a000bc2b02902df93cc371amr27408187pfu.12.1621906986056; Mon, 24 May 2021 18:43:06 -0700 (PDT) Received: from 42.do-not-panic.com (42.do-not-panic.com. [157.230.128.187]) by smtp.gmail.com with ESMTPSA id y190sm12784817pgd.24.2021.05.24.18.43.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 May 2021 18:43:05 -0700 (PDT) Received: by 42.do-not-panic.com (Postfix, from userid 1000) id 413464025E; Tue, 25 May 2021 01:43:04 +0000 (UTC) Date: Tue, 25 May 2021 01:43:04 +0000 From: Luis Chamberlain To: Menglong Dong Cc: Jan Kara , Jens Axboe , hare@suse.de, gregkh@linuxfoundation.org, tj@kernel.org, Menglong Dong , song@kernel.org, NeilBrown , Andrew Morton , wangkefeng.wang@huawei.com, f.fainelli@gmail.com, arnd@arndb.de, Barret Rhoden , Rasmus Villemoes , mhiramat@kernel.org, Steven Rostedt , Kees Cook , vbabka@suse.cz, Alexander Potapenko , pmladek@suse.com, Chris Down , ebiederm@xmission.com, jojing64@gmail.com, LKML , palmerdabbelt@google.com, linux-fsdevel@vger.kernel.org, Alexander Viro Subject: Re: [PATCH RESEND] init/initramfs.c: make initramfs support pivot_root Message-ID: <20210525014304.GH4332@42.do-not-panic.com> References: <20210520154244.20209-1-dong.menglong@zte.com.cn> <20210520214111.GV4332@42.do-not-panic.com> <20210521155020.GW4332@42.do-not-panic.com> <20210524225827.GA4332@42.do-not-panic.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Tue, May 25, 2021 at 08:55:48AM +0800, Menglong Dong wrote: > On Tue, May 25, 2021 at 6:58 AM Luis Chamberlain wrote: > > > > 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. > > 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? > > > > > 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. > > > > > 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