linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Kees Cook <keescook@chromium.org>
Cc: Will Drewry <wad@chromium.org>, Chen Yu <yu.c.chen@intel.com>,
	Jonathan Corbet <corbet@lwn.net>,
	David Zeuthen <zeuthen@google.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	LKML <linux-kernel@vger.kernel.org>,
	Dan Ehrenberg <dehrenberg@chromium.org>,
	linux-raid@vger.kernel.org, dm-devel@redhat.com,
	Vishnu Pratap Singh <vishnu.ps@samsung.com>,
	Yaowei Bai <baiyaowei@cmss.chinamobile.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Shaohua Li <shli@kernel.org>, Alasdair Kergon <agk@redhat.com>
Subject: Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
Date: Fri, 26 Feb 2016 14:21:45 -0500	[thread overview]
Message-ID: <20160226192145.GA27326@redhat.com> (raw)
In-Reply-To: <CAGXu5j+JGPZ2sH3uAHwpJ3im_TBUL19U43EZw-Z2dZ6fQiC2zA@mail.gmail.com>

On Fri, Feb 26 2016 at  1:52pm -0500,
Kees Cook <keescook@chromium.org> wrote:

> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> > On Mon, Feb 22 2016 at  1:55pm -0500,
> > Kees Cook <keescook@chromium.org> wrote:
> >
> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@redhat.com> 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:
> >  <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
> > (inventing a special token to denote <newline>, 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.

  reply	other threads:[~2016-02-26 19:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-20 18:13 [PATCH v5 0/3] init: add support to directly boot to a mapped device Kees Cook
2016-02-20 18:13 ` [PATCH v5 1/3] dm: export a table+mapped device to the ioctl interface Kees Cook
2016-02-20 18:13 ` [PATCH v5 2/3] dm: make mapped_device locking functions available Kees Cook
2016-02-20 18:13 ` [PATCH v5 3/3] init: add support to directly boot to a mapped device Kees Cook
2016-02-21 22:08 ` [PATCH v5 0/3] " Alasdair G Kergon
2016-02-22 18:55   ` Kees Cook
2016-02-26 16:53     ` Mike Snitzer
2016-02-26 18:52       ` Kees Cook
2016-02-26 19:21         ` Mike Snitzer [this message]
2016-02-26 19:59           ` Kees Cook
2016-02-26 20:47             ` Mike Snitzer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160226192145.GA27326@redhat.com \
    --to=snitzer@redhat.com \
    --cc=agk@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=baiyaowei@cmss.chinamobile.com \
    --cc=corbet@lwn.net \
    --cc=dehrenberg@chromium.org \
    --cc=dm-devel@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=shli@kernel.org \
    --cc=vishnu.ps@samsung.com \
    --cc=wad@chromium.org \
    --cc=yu.c.chen@intel.com \
    --cc=zeuthen@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).