Re: Patch 0727d35de ("Make initramfs honor CONFIG_DEVTMPFS_MOUNT") breaks boot

From: Petr Mladek <pmladek@suse.com>
To: Rob Landley <rob@landley.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
	Yury Norov <ynorov@caviumnetworks.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Prarit Bhargava <prarit@redhat.com>,
	Yang Shi <yang.shi@linaro.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Kees Cook <keescook@chromium.org>,
	Emese Revfy <re.emese@gmail.com>,
	Fabian Frederick <fabf@skynet.be>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: Patch 0727d35de ("Make initramfs honor CONFIG_DEVTMPFS_MOUNT") breaks boot
Date: Mon, 11 Sep 2017 13:45:35 +0200	[thread overview]
Message-ID: <20170911114535.GL8741@pathway.suse.cz> (raw)
In-Reply-To: <1fb53a8c-6c17-1de2-493c-0bff403c9ff9@landley.net>

On Sun 2017-09-10 18:43:24, Rob Landley wrote:
> On 05/25/2017 01:13 AM, Michael Ellerman wrote:
> > Hi Rob,
> > 
> > This is breaking a bunch of my powerpc boxes, for the exact same
> > reason, they use a config that has DEVTMPFS_MOUNT=y and that trips
> > up the initramfs.
> 
> I've continued to use this locally but should probably make another
> stab at submitting upstream. The obvious workaround until debian fixes
> its 100% obvious bug seems to be:
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index f8893dc..f57d5df 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2417,7 +2417,17 @@ static int do_add_mount(struct mount *newmnt, struct path *path, int mnt_flags)
>  	err = -EBUSY;
>  	if (path->mnt->mnt_sb == newmnt->mnt.mnt_sb &&
>  	    path->mnt->mnt_root == path->dentry)
> +	{
> +		if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT) &&
> +		    !strcmp(path->mnt->mnt_sb->s_type->name, "devtmpfs"))
> +		{
> +			printk(KERN_WARNING "Debian bug workaround for devtmpfs overmount.");
> +			printk(KERN_WARNING "This line doesn't output for some reason.");
> +
> +			err = 0;
> +		}
>  		goto unlock;
> +	}
>  
>  	err = -EINVAL;
>  	if (d_is_symlink(newmnt->mnt.mnt_root))
> 
> Except for the second printk line: If you boot with rdinit=/bin/hush
> then the first time you mount -t devtmpfs /dev /dev after boot (with
> CONFIG_DEVTMPFS_MOUNT already having mounted it), you get the 0 return
> value but the last printk() doesn't output? The second and later times
> you repeat it, both printk() lines are output.
> 
> What's up with printk?
> 
> (I added the second printk because the _first_ one wasn't outputting
> that first time. Something is happening to flush the printk() queue
> instead of writing it out?

You need to add "\n" at the end of the line. Otherwise, it expects
that the message would continue and puts it into a cont buffer.
The buffer is flushed only when another non-continuous message
is added.

This problem is more visible since the commit 5c2992ee7fd8a29d0412
("printk: remove console flushing special cases for partial buffered
lines").

Hmm, Linus wanted to add a timer that would always flush
the cont buffer in a reasonable time frame. But there was a risk
of deadlocks because of circular timer<->printk dependency,
so it never happened.

Maybe, we could setup the timer via an irq_work. We already use
this trick for flushing deferred printk and waking klogd.
It is not nice but it would be easier than the previous mess.

Best Regards,
Petr