Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

From: Rob Landley
Date: Wed May 17 2017 - 17:16:22 EST




On 05/16/2017 10:58 PM, Michael Ellerman wrote:
> Rob Landley <rob@xxxxxxxxxxx> writes:
>
>> diff --git a/init/main.c b/init/main.c
>> index f866510..9ec09ff 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -1055,8 +1049,17 @@ static noinline void __init kernel_init_freeable(void)
>> if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
>> ramdisk_execute_command = NULL;
>> prepare_namespace();
>> + } else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
>> + sys_mkdir("/dev", 0755);
>> + devtmpfs_mount("/dev");
>> }
>>
>> + /* Open the /dev/console on the rootfs, this should never fail */
>> + if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
>
> Sorry to pile on,

The correct phrase is "bikeshed". (It's a verb now.)

> but while you're moving it do you want

_I_ don't, no. I intentionally moved it unmodified. If you want to
submit a patch on top of mine, be my guest.

> to update this fairly misleading comment.

Define "should". (I'll get the popcorn.)

> It definitely can fail, eg. if /dev/console doesn't exist, or if no
> console driver is registered.

/dev/console not existing in an initramfs created by pointing
CONFIG_INITRAMFS_SOURCE at a directory created by a normal user was
pretty much my initial motivation for poking at this area, yes.

That said, /dev/console should always exist. My patch was just finding a
different way for it to exist so the condition was satisfied. Meaning
the comment isn't exactly _wrong_, just really terse. Feel free to
submit a patch rephrasing it.

Rob