lkml.org 
[lkml]   [2011]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 2/2] CPU hotplug, Freezer: Synchronize CPU hotplug and Freezer
    On 10/28/2011 05:32 PM, Rafael J. Wysocki wrote:
    > On Friday, October 28, 2011, Rafael J. Wysocki wrote:
    >> On Friday, October 28, 2011, Srivatsa S. Bhat wrote:
    >>> On 10/28/2011 01:43 AM, Rafael J. Wysocki wrote:
    >>>> Hi,
    >>>>
    >>>> On Thursday, October 27, 2011, Srivatsa S. Bhat wrote:
    >>>>> Prevent CPU hotplug and the freezer from racing with each other, to ensure
    >>>>> that during the *entire duration* for which the callbacks for CPU hotplug
    >>>>> notifications such as CPU_ONLINE[_FROZEN], CPU_DEAD[_FROZEN] etc are being
    >>>>> executed, the state of the system (with respect to the tasks being frozen
    >>>>> or not) remains constant.
    >>>>>
    >>>>> This patches hooks the CPU hotplug infrastructure onto the freezer
    >>>>> notifications (PM_FREEZE_PREPARE and PM_POST_THAW) and thus synchronizes
    >>>>> with the freezer.
    >>>>>
    >>>>> Specifically,
    >>>>>
    >>>>> * Upon the PM_FREEZE_PREPARE notification, the CPU hotplug callback disables
    >>>>> future (regular) CPU hotplugging and also ensures that any currently running
    >>>>> CPU hotplug operation is completed before allowing the freezer to continue
    >>>>> any further.
    >>>>>
    >>>>> * Upon the PM_POST_THAW notification, the CPU hotplug callback re-enables
    >>>>> regular CPU hotplug.
    >>>>>
    >>>>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
    >>>>> ---
    >>>>>
    >>>>> kernel/cpu.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
    >>>>> 1 files changed, 76 insertions(+), 0 deletions(-)
    >>>>>
    >>>>> diff --git a/kernel/cpu.c b/kernel/cpu.c
    >>>>> index 12b7458..61985ce 100644
    >>>>> --- a/kernel/cpu.c
    >>>>> +++ b/kernel/cpu.c
    >>>>> @@ -15,6 +15,7 @@
    >>>>> #include <linux/stop_machine.h>
    >>>>> #include <linux/mutex.h>
    >>>>> #include <linux/gfp.h>
    >>>>> +#include <linux/suspend.h>
    >>>>>
    >>>>> #ifdef CONFIG_SMP
    >>>>> /* Serializes the updates to cpu_online_mask, cpu_present_mask */
    >>>>> @@ -478,6 +479,81 @@ static int alloc_frozen_cpus(void)
    >>>>> core_initcall(alloc_frozen_cpus);
    >>>>> #endif /* CONFIG_PM_SLEEP_SMP */
    >>>>>
    >>>>> +
    >>>>> +#ifdef CONFIG_FREEZER
    >>>>> +
    >>>>> +/*
    >>>>> + * Avoid CPU hotplug racing with the freezer subsystem, by disabling CPU
    >>>>> + * hotplug when tasks are about to be frozen.
    >>>>> + *
    >>>>> + * Also, don't allow the freezer subsystem to continue until any currently
    >>>>> + * running CPU hotplug operation gets completed.
    >>>>> + * To modify the 'cpu_hotplug_disabled' flag, we need to acquire the
    >>>>> + * 'cpu_add_remove_lock'. And this same lock is also taken by the regular
    >>>>> + * CPU hotplug path and released only after it is complete. Thus, we
    >>>>> + * (and hence the freezer) will block here until any currently running CPU
    >>>>> + * hotplug operation is completed.
    >>>>> + */
    >>>>> +static void cpu_hotplug_freezer_block_begin(void)
    >>>>> +{
    >>>>> + cpu_maps_update_begin();
    >>>>> + cpu_hotplug_disabled = 1;
    >>>>> + cpu_maps_update_done();
    >>>>> +}
    >>>>> +
    >>>>> +
    >>>>> +/*
    >>>>> + * When thawing of tasks is complete, re-enable CPU hotplug (which had been
    >>>>> + * disabled while beginning to freeze tasks).
    >>>>> + */
    >>>>> +static void cpu_hotplug_freezer_block_done(void)
    >>>>> +{
    >>>>> + cpu_maps_update_begin();
    >>>>> + cpu_hotplug_disabled = 0;
    >>>>> + cpu_maps_update_done();
    >>>>> +}
    >>>>> +
    >>>>
    >>>> I wonder if the new PM notifier events are really necessary?
    >>>>
    >>>> Why don't you just call cpu_hotplug_freezer_block_begin() (perhaps
    >>>> with a better name?) directly from freeze_processes()? And analogously
    >>>> for cpu_hotplug_freezer_block_done() and thaw_processes()?
    >>>>
    >>>
    >>> Yes, we can definitely do that.
    >>>
    >>> But the reason why I chose to introduce new notifiers was to make this
    >>> more extensible (because we know that at least 2 subsystems would benefit
    >>> from mutually excluding themselves from the freezer, namely CPU hotplug
    >>> and x86 microcode).
    >>> http://thread.gmane.org/gmane.linux.kernel/1198291/focus=1200591
    >>>
    >>> But now that I think of it, hooking onto the freezer notifiers wouldn't
    >>> solve the microcode cases since usermodehelper_disable() is called
    >>> _before_ freezing tasks... :(
    >>>
    >>> So we should probably call the functions directly like you suggested..
    >>>
    >>> But I really didn't want to clutter the freezer call path because of problems
    >>> elsewhere. So I felt freezer notifiers would be a cleaner way of dealing with
    >>> such things. Also, since freezer is a generic subsystem that could be used
    >>> for purposes other than S3/S4 as well (I have heard of attempts to use freezer
    >>> during tracing), wouldn't it be better to introduce new notifiers to
    >>> announce the begin and end of freezer activity to interested subsystems?
    >>> (and then use them to solve the CPU hotplug issue like this patch does...)
    >>>
    >>> Please let me know your suggestions.
    >>
    >> The freeze_processes() and thaw_processes() functions are only used for
    >> system suspend and hibernation, as far as I can tell, and I don't think there
    >> will be any other users in predictable future.
    >>
    >> Also, adding the calls directly to those functions will show exactly what
    >> the dependecies are, while doing that through a notifier kind of obfuscates
    >> things. So, please make direct calls from there.
    >
    > Alternatively, which I'd even prefer in fact, you can simply use the
    > PM_SUSPEND_PREPARE and PM_POST_SUSPEND notifier events (and analogously
    > for hibernation) to run that code. Which also might be useful for solving
    > the microcode case.
    >

    The primary reason I hadn't used those notifications for this purpose till now was
    because I thought freezer might have usecases other than S3/S4 and hence we needed
    synchronization at the freezer level. But now that you clarified that part, I'll
    go ahead and use the SUSPEND/HIBERNATE notifiers for this.
    Thank you very much.

    --
    Regards,
    Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
    Linux Technology Center,
    IBM India Systems and Technology Lab



    \
     
     \ /
      Last update: 2011-10-28 14:31    [W:0.035 / U:3.580 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site