linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 0/7] kernel: Add support for restart notifier call chain
Date: Thu, 10 Jul 2014 17:15:49 -0700	[thread overview]
Message-ID: <53BF2CB5.5080602@roeck-us.net> (raw)
In-Reply-To: <20140710160907.da1024914366de947ebdf384@linux-foundation.org>

On 07/10/2014 04:09 PM, Andrew Morton wrote:
> On Tue,  8 Jul 2014 20:37:56 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
>
>> The existing mechanisms have a number of drawbacks. Typically only one scheme
>> to restart the system is supported (at least if arm_pm_restart is used).
>> At least in theory there can be mutliple means to restart the system, some of
>> which may be less desirable (for example one mechanism may only reset the CPU,
>> while another may reset the entire system).
>
> So the callbacks need to be prioritized.
>
>> Using arm_pm_restart can also be
>> racy if the function pointer is set from a driver, as the driver may be in
>> the process of being unloaded when arm_pm_restart is called.
>> Using the reboot notifier is always racy, as it is unknown if and when
>> other functions using the reboot notifier have completed execution
>> by the time the watchdog fires.
>>
>> To solve the problem, introduce a system restart notifier. This notifier
>> is expected to be called from the architecture specific machine_restart()
>> function. Drivers providing system restart functionality (such as the watchdog
>> drivers mentioned above) are expected to register with this notifier.
>
> It's worth mentioning here that the notifier_block priority scheme is
> used to address the problem which was identified in the previous
> paragraph.
>
Ok.

> If this scheme is to be successful we will need to set in place some
> protocol for specifying how the priorities are managed.  If someone
> sits down and writes a new restart handler, how is that person to
> decide how to prioritize it against other handlers, both present and
> future?
>
> Also, looking at the patches, you don't appear to have actually *used*
> the prioritization - everything is left at zero.  So we'll end up using
> the most-recently-registered handler to restart the system.  The
> patches don't actually solve the problem which was identified in the
> above paragraph?

The primary goal of this patch set was to provide a generic scheme for
registering restart handlers, and the ability to load and unload notifiers
without race conditions. Support for multiple restart handlers with
different priorities was a secondary objective. The conversions I did
so far are expected to be mutually exclusive, ie provide the one and
only means on a given architecture to restart the system. So I guess
you do have a point - the priorities for those notifiers should
probably be higher. Error on my part - I thought lower numbers would
have higher priority, but after looking into the code again that
is wrong.

To avoid making things too complicated, maybe it would make sense to
specify guidelines for notifier priorities, such as
0   - restart notifier of last resort, with least reset capabilities
128 - default; use if no other notifier is expected to be available
       and/or if restart functionality is acceptable
255 - highest priority notifier which _must_ be used

Would that make sense and be acceptable ? In this context, I would then
set the notifier priorities for the callers in the patch set to 128.

Thanks,
Guenter

  reply	other threads:[~2014-07-11  0:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-09  3:37 [PATCH v3 0/7] kernel: Add support for restart notifier call chain Guenter Roeck
2014-07-09  3:37 ` [PATCH v3 1/7] " Guenter Roeck
2014-07-09  3:37 ` [PATCH v3 2/7] arm64: Support restart through " Guenter Roeck
2014-07-09  3:37 ` [PATCH v3 3/7] arm: " Guenter Roeck
2014-07-09  3:38 ` [PATCH v3 4/7] power/restart: Call machine_restart instead of arm_pm_restart Guenter Roeck
2014-07-09  3:38 ` [PATCH v3 5/7] watchdog: moxart: Register restart handler with restart notifier Guenter Roeck
2014-07-09  3:38 ` [PATCH v3 6/7] watchdog: alim7101: " Guenter Roeck
2014-07-09  3:38 ` [PATCH v3 7/7] arm/arm64: Unexport restart handlers Guenter Roeck
2014-07-10 23:09 ` [PATCH v3 0/7] kernel: Add support for restart notifier call chain Andrew Morton
2014-07-11  0:15   ` Guenter Roeck [this message]
2014-07-11  0:44     ` Andrew Morton

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=53BF2CB5.5080602@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-arm-kernel@lists.infradead.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;
as well as URLs for NNTP newsgroup(s).