From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423171AbcBZTVv (ORCPT ); Fri, 26 Feb 2016 14:21:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59712 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1423022AbcBZTVt (ORCPT ); Fri, 26 Feb 2016 14:21:49 -0500 Date: Fri, 26 Feb 2016 14:21:45 -0500 From: Mike Snitzer To: Kees Cook Cc: Will Drewry , Chen Yu , Jonathan Corbet , David Zeuthen , "linux-doc@vger.kernel.org" , "Rafael J. Wysocki" , LKML , Dan Ehrenberg , linux-raid@vger.kernel.org, dm-devel@redhat.com, Vishnu Pratap Singh , Yaowei Bai , Andrew Morton , Shaohua Li , Alasdair Kergon Subject: Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device Message-ID: <20160226192145.GA27326@redhat.com> References: <1455992032-14594-1-git-send-email-keescook@chromium.org> <20160221220839.GD16166@agk-dp.fab.redhat.com> <20160226165358.GA26728@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 26 2016 at 1:52pm -0500, Kees Cook wrote: > On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer wrote: > > On Mon, Feb 22 2016 at 1:55pm -0500, > > Kees Cook wrote: > > > >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon wrote: > >> > On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote: > >> >> This is a resurrection of a patch series from a few years back, first > >> >> brought to the dm maintainers in 2010. It creates a way to define dm > >> >> devices on the kernel command line for systems that do not use an > >> >> initramfs, or otherwise need a dm running before init starts. > >> >> > >> >> This has been used by Chrome OS for several years, and now by Brillo > >> >> (and likely Android soon). > >> >> > >> >> The last version was v4: > >> >> https://patchwork.kernel.org/patch/104860/ > >> >> https://patchwork.kernel.org/patch/104861/ > >> > > >> > Inconsistencies in the terminology here can be sorted out during review, > >> > and I see that you've taken on board some of my review comments from > >> > 2010, but what are your responses to the rest of them? > >> > >> Ah, sorry, the threads I could find were incomplete, so I wasn't able > >> to find those comments that were made to Will's 2010 submission. In > >> some of the cleanups I did I was very confused about "target" vs > >> "table", and tried to fix that. Regardless, I'm open to fixing > >> whatever is needed. :) > >> > >> Thanks for looking at this again! > > > > This work isn't going to fly as is. I appreciate the effort and the > > goal (without understanding _why_) but: you're open-coding, duplicating > > and/or reinventing way too much in do_mounts_dm.c > > > > 1) You first need to answer: _why_ is using a proper initramfs not > > viable? A very simple initramfs that issues dmsetup commands, etc, > > isn't so daunting is it? Why is it so important for the kernel to > > natively provide a dmsetup interface? Chrome, Android, etc cannot use > > initramfs? > > That is correct: Chrome OS does not (and won't) use an initramfs. This > is mainly for reasons of boot speed, verified boot block size, and > maybe some other things I don't remember. Not sure what "verified boot block size" means but... Sorry I really don't buy that using a custom initramfs would be the source of slow boot. initramfs is _not_ this hugely inefficient mechanism you'd have us believe. And if that is the justification for this early boot dm= support then the Chrome OS project/team will have to continue to carry the hack locally. It has no place upstream. But I'm open to revisiting this if it can be implemented in a very cheap way. > > 2) If you are able to adequately justify the need for dm=: > > I'd much rather the dm= kernel commandline be a simple series of > > comma-delimited dmsetup-like commands. > > > > You'd handle each command with extremely basic parsing: > > [, ] > > (inventing a special token to denote , to support tables with > > multiple entries, rather than relying on commas and counts, etc) > > Sure, changing the syntax is fine by me. We'd need to plumb access to > the ioctl interface, though. I was hoping to avoid any extra hacks but yes... seems you'd need a new API to issue the equivalent of a DM ioctl programatically. Hopefully it'd be quite a small wrapper. > > and you'd then have do_mounts_dm.c open /dev/mapper/control directly and > > issue proper DM ioctls rather than adding all your shim code. This last > > bit of opening /dev/mapper/control from init needs more research -- not > > sure if doing such a thing from kernel is viable/safe/acceptable. > > Well, there's no /dev and no init since our dm is the root device > (dm-verity). We need everything up and running before we mount the > root filesystem, very similar to do_mount_md.c's purpose. Ah yes, microoptimization associated with no udev or normal Linux boot comes full circle and limits the use of existing standard interfaces.