lkml.org 
[lkml]   [2011]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [RFC][PATCH] PM: Print a warning if firmware is requested when tasks are frozen
    Date
    On Tuesday, May 03, 2011, Valdis.Kletnieks@vt.edu wrote:
    > On Tue, 03 May 2011 00:44:51 +0200, "Rafael J. Wysocki" said:
    >
    > > + if (WARN_ON(usermodehelper_is_disabled()))
    > > + return -EBUSY;
    > > +
    >
    > Since this is a "no user serviceable parts inside" type of error, so I guess
    > WARN_ON rather than a printk(KERN_WARNING is a good idea so we get
    > a traceback pointing out the offending driver.
    >
    > I have to wonder 2 things though:
    >
    > 1) What percent of the time the missing firmware (or other issues, like the
    > display not being resumed yet) will prevent the WARN_ON output from making it
    > to the display *anyhow*, so the user *still* hits the power button to try again?

    Although the WARN_ON output will probably not make it to the user's screen
    immediately, the resume will continue without the delay, because error code
    will be returned as soon as the WARN_ON triggers.

    The user will likely notice the device not working after the resume and will
    look at dmesg output. :-)

    > 2) What percent of the time the WARN_ON output will itself make the user
    > think the resume has died rather than just being slow, causing them to power
    > cycle and hope for a clean boot?
    >
    > Maybe something like this instead?
    >
    > if (WARN_ON(usermodehelper_is_disable()))) {
    > printk(KERN_WARNING "Resume continuing, but firmware for %s not loaded", device);

    It's useful, but I'd rather do

    dev_info(device, "firmware: %s will not be loaded\n", name);

    > return -EBUSY;
    > }
    >
    > (or whatever that %s actually needs to work)
    >
    > All the same, it still looks better than what we're doing now.

    OK, updated patch is appended.

    Thanks,
    Rafael

    ---
    From: Rafael J. Wysocki <rjw@sisk.pl>
    Subject: PM: Print a warning if firmware is requested when tasks are frozen

    Some drivers erroneously use request_firmware() from their ->resume()
    (or ->thaw(), or ->restore()) callbacks, which is not going to work
    unless the firmware has been built in. This causes system resume to
    stall until the firmware-loading timeout expires, which makes users
    think that the resume has failed and reboot their machines
    unnecessarily. For this reason, make _request_firmware() print a
    warning and return immediately with error code if it has been called
    when tasks are frozen and it's impossible to start any new usermode
    helpers.

    Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
    ---
    drivers/base/firmware_class.c | 5 +++++
    include/linux/kmod.h | 1 +
    kernel/kmod.c | 8 ++++++++
    3 files changed, 14 insertions(+)

    Index: linux-2.6/include/linux/kmod.h
    ===================================================================
    --- linux-2.6.orig/include/linux/kmod.h
    +++ linux-2.6/include/linux/kmod.h
    @@ -113,5 +113,6 @@ extern void usermodehelper_init(void);

    extern int usermodehelper_disable(void);
    extern void usermodehelper_enable(void);
    +extern bool usermodehelper_is_disabled(void);

    #endif /* __LINUX_KMOD_H__ */
    Index: linux-2.6/kernel/kmod.c
    ===================================================================
    --- linux-2.6.orig/kernel/kmod.c
    +++ linux-2.6/kernel/kmod.c
    @@ -301,6 +301,14 @@ void usermodehelper_enable(void)
    usermodehelper_disabled = 0;
    }

    +/**
    + * usermodehelper_is_disabled - check if new helpers are allowed to be started
    + */
    +bool usermodehelper_is_disabled(void)
    +{
    + return usermodehelper_disabled;
    +}
    +
    static void helper_lock(void)
    {
    atomic_inc(&running_helpers);
    Index: linux-2.6/drivers/base/firmware_class.c
    ===================================================================
    --- linux-2.6.orig/drivers/base/firmware_class.c
    +++ linux-2.6/drivers/base/firmware_class.c
    @@ -521,6 +521,11 @@ static int _request_firmware(const struc
    if (!firmware_p)
    return -EINVAL;

    + if (WARN_ON(usermodehelper_is_disabled())) {
    + dev_info(device, "firmware: %s will not be loaded\n", name);
    + return -EBUSY;
    + }
    +
    *firmware_p = firmware = kzalloc(sizeof(*firmware), GFP_KERNEL);
    if (!firmware) {
    dev_err(device, "%s: kmalloc(struct firmware) failed\n",

    \
     
     \ /
      Last update: 2011-05-03 13:03    [W:0.026 / U:92.072 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site