public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Joel Stanley <joel.stanley@au1.ibm.com>,
	Jeremy Kerr <jeremy.kerr@au1.ibm.com>,
	Greg KH <gregkh@linuxfoundation.org>
Subject: kexec: device shutdown vs. remove
Date: Sun, 24 Jul 2016 06:51:52 +1000	[thread overview]
Message-ID: <1469307112.8568.224.camel@kernel.crashing.org> (raw)

Hi !

This is somewhat of a recurring issue, some of my previous attempts on
lkml, I suspect, were just drowned in the noise. Eric, we had a quick
discussion about this a while back but I don't think we reached a
conclusion.

A bit of context: On OpenPOWER machines, we have a Linux based
bootloader, so we rely heavily on kexec to boot distro kernels, and
this has been causing us grief, mostly in the device driver space.

Device drivers need to be quiesced before kexec. More specifically
the device *hardware* needs that, ie we want DMAs to stop and the
device to be put into a state where it can reliably be picked up by the
driver in the new kernel.

Today, kexec calls device_shutdown() to achieve that. I argue that this
is the wrong thing to do and instead we should do someting that causes
the various drivers ->remove() function to be called (whether that
implies actually unbinding the driver or not).

I believe we do this for historical reasons, as ->remove() used to
depend on CONFIG_HOTPLUG while ->shutdown() was always around but that
is no longer the case.

The most visible issue with ->shutdown() that we encouter is that a lot
of drivers simply don't implement it.

The *real* issue however is that it's the wrong thing to do anyway. It
is a call intended to be called when the machine will be shutdown, as
such not only it is very much optional (and rarely implemented), but it
can also (and will in some cases) power bits of hardware off which is
not what you want to do if a new driver will try to pick up the pieces.

Arguably, the most correct semantic is provided by ->remove() since
that corresponds to removing a driver and binding a new one to the
device. IE. the same flow as doing rmmod/insmod of a new driver.

In practice, we obseve that a lot more drivers implement ->remove(). A
few were "fixed" to have ->shutdown() for kexec stake over time, but in
many case it's a duplication of ->remove() (ugh...).

So I would like to discuss this or at least get feedback and an overall
agreement. I can provide patches to test fairly soon.

Cheers,
Ben.

             reply	other threads:[~2016-07-23 20:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-23 20:51 Benjamin Herrenschmidt [this message]
2016-07-24  5:18 ` kexec: device shutdown vs. remove Guenter Roeck
2016-07-24 13:13   ` Benjamin Herrenschmidt
2016-07-24 21:36     ` Eric W. Biederman
2016-07-25  0:28       ` Benjamin Herrenschmidt
2016-07-24  5:24 ` Eric W. Biederman
2016-07-24 13:15   ` Benjamin Herrenschmidt
2016-07-24 21:35     ` Eric W. Biederman

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=1469307112.8568.224.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jeremy.kerr@au1.ibm.com \
    --cc=joel.stanley@au1.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    /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