linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
To: Tejun Heo <tj@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
	tigran@aivazian.fsnet.co.uk, tglx@linutronix.de, mingo@elte.hu,
	hpa@zytor.com, x86@kernel.org, linux-kernel@vger.kernel.org,
	Linux PM mailing list <linux-pm@lists.linux-foundation.org>
Subject: Re: [BUGFIX][PATCH] Freezer, CPU hotplug, x86 Microcode: Fix task freezing failures
Date: Mon, 03 Oct 2011 11:21:49 +0530	[thread overview]
Message-ID: <4E894D75.808@linux.vnet.ibm.com> (raw)
In-Reply-To: <20111003004051.GD31799@mtj.dyndns.org>

Hi,

First of all, thank you for your invaluable inputs.

On 10/03/2011 06:10 AM, Tejun Heo wrote:
> Hello,
> 
> On Mon, Oct 03, 2011 at 01:34:36AM +0530, Srivatsa S. Bhat wrote:
>> So when a CPU goes offline and comes back online, I don't see why the kernel
>> should not reuse the microcode that it already has. Anyhow the microcode will
>> not change. The same microcode would be requested from userspace again if the
>> kernel has freed its copy.
> 
> Can't one take a cpu offline, hot unplug it from the board and put in
> a new one and then bring it online?  That's usually what "hot
> [un]plug" stands for.  I don't think one would be able to put in a
> very different processor but maybe, say, revision difference is
> allowed and microcode should be looked up again?  Assuming that the
> same microcode can always be applied after the actual CPU is swapped
> could be dangerous.  Again, I'm not sure about how this is supposed to
> be managed so I could be wrong.
>

I see your point here. However it is also true that quite a lot of CPU hotplug
operations are done with use-cases other than physically plugging out and plugging in
CPUs. For example, the disabling and enabling of CPUs during suspend and resume,
the offlining of CPUs when the system is idle (in some architectures) to reduce
idle power consumption etc. Moreover physically hotplugging CPUs also demands that
the electrical wiring to the CPUs are specially designed in such a way as to support
physical CPU hotplugging - a feature which is supported by only very few hardware
manufacturers, if what I have heard is right.

Another related fact to consider would be how the disabling and enabling of non-boot
CPUs are done during system suspend and resume. Since the tasks are frozen at those
points, a different notification is sent i.e.,CPU_[ONLINE|DEAD]_FROZEN instead of the
usual CPU_[ONLINE|DEAD] notification. 
And upon the CPU_DEAD_FROZEN notification, the microcode core doesn't invalidate the
microcode! Hence, it won't have trouble applying the microcode to the CPU during the
phase of enabling the non-boot CPUs. But a valid point here would be that during suspend
and resume, nobody expects to plug out and plug in a new CPU!

But in any case, suppose we need a revised microcode (due to the usecase you have
described, for example), then I agree that it would definitely be not good to forcefully
apply the same old microcode that the kernel has.

> If I'm not wrong, can't we add synchronization between cpu hotplug and
> freezer so that they don't happen in parallel?
> 

I have posted another patch related to CPU hotplug and freezer here:
https://lkml.org/lkml/2011/10/2/163
That patch aims to sync up the CPU hotplug infrastructure with the activity of the 
freezer and hence ensure that the right notifications are sent. Maybe I could easily
modify that patch to disable CPU hotplugging when the freezer is active.

But still, that would not solve our problem due to the following possible scenario:

[Let us assume that we go ahead and invalidate the microcode upon a CPU_DEAD notification,
like you suggested.] Then, consider this scenario:

* Take a CPU offline (a pure CPU hotplug operation, not related to suspend).
  Let us call this CPU X.
  - This would involve a CPU_DEAD notification, upon which the microcode is invalidated.

* Start freezing tasks (due to a suspend operation in progress). Now we disable
  CPU hotplugging.
 
* Disable (offline) the non-boot CPUs as part of suspend operation. This sends the
CPU_DEAD_FROZEN notification, upon which the microcode is NOT invalidated for the other CPUs.

* Enable (online) the non-boot CPUs as part of resume operation. Please note that the tasks
are still frozen. This is where we hit the same issue again! When trying to bring that CPU X
back online, it finds that the microcode has been invalidated and hence tries to request the
userspace for the microcode, but alas, the userspace is still frozen at that moment.
This is the exact problem we were trying to solve earlier, which remains unsolved by disabling
CPU hotplug during freezer operations!


So my one-liner patch here tries to solve this problem by not invalidating the microcode at all,
to handle the corner case of a CPU hotplug going on in parallel with freezer operations.
But it creates issues with the usecase you described.

At the moment I can't think of any clean solution that solves both these problems - 
  a. The problem of requesting microcode from userspace when it is frozen (which can be individually
     solved by my patch).
  b. The possible need for a revised microcode during CPU online (due to the remote usecase of 
     physically plugging-out and plugging-in a new CPU), when the tasks are frozen.

To address both problems 'a' and 'b' in one shot, I can think of a rather ugly workaround such as:
* Go ahead and invalidate the microcode upon a CPU_DEAD notification.
* During CPU onlining, if we find that the userspace is frozen, defer applying microcode for now
  and register a callback function to be executed immediately when the userspace gets thawed.
  Also, prevent any task from executing on this CPU until the proper microcode is applied to it.
* When the userspace finally gets thawed, run the registered callback function, get the revised
  microcode from userspace, apply it to the CPU and then go ahead and run tasks on that CPU.

Any ideas?

Summary:
My patch solves the problem that is most likely hit, while there are some corner cases
which it doesn't handle, such as physical CPU hotplugging which might require revised microcode to be
loaded. The idea proposed above could solve both issues, but it doesn't seem very elegant, or does it?

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

  reply	other threads:[~2011-10-03  5:52 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-02 19:05 [BUGFIX][PATCH] Freezer, CPU hotplug, x86 Microcode: Fix task freezing failures Srivatsa S. Bhat
2011-10-02 19:36 ` Rafael J. Wysocki
2011-10-02 19:50 ` Tejun Heo
2011-10-02 20:04   ` Srivatsa S. Bhat
2011-10-03  0:40     ` Tejun Heo
2011-10-03  5:51       ` Srivatsa S. Bhat [this message]
2011-10-03  8:47         ` Borislav Petkov
2011-10-04  7:15           ` Tejun Heo
2011-10-04 13:15             ` Srivatsa S. Bhat
2011-10-04 13:46               ` Borislav Petkov
2011-10-04 17:14                 ` Borislav Petkov
2011-10-04 19:49                   ` Rafael J. Wysocki
2011-10-04 20:57                   ` Srivatsa S. Bhat
2011-10-05  7:21                     ` Borislav Petkov
2011-10-05  8:51                       ` Srivatsa S. Bhat
2011-10-05 20:26                         ` Rafael J. Wysocki
2011-10-05 21:15                           ` Srivatsa S. Bhat
2011-10-05 22:43                             ` Rafael J. Wysocki
2011-10-06  6:50                               ` Srivatsa S. Bhat
2011-10-06  8:34                                 ` Borislav Petkov
2011-10-06 15:47                                   ` Srivatsa S. Bhat
2011-10-06 18:11                                     ` Srivatsa S. Bhat
2011-10-06 20:35                                     ` [BUGFIX][PATCH RESEND] " Srivatsa S. Bhat
2011-10-06 22:13                                       ` Tejun Heo
2011-10-06 22:34                                         ` Borislav Petkov
2011-10-07 16:48                                           ` Srivatsa S. Bhat
2011-10-07 18:05                                             ` Borislav Petkov
2011-10-04 13:25             ` [BUGFIX][PATCH] " Borislav Petkov
2011-10-05  8:33         ` Srivatsa S. Bhat

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=4E894D75.808@linux.vnet.ibm.com \
    --to=srivatsa.bhat@linux.vnet.ibm.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=mingo@elte.hu \
    --cc=rjw@sisk.pl \
    --cc=tglx@linutronix.de \
    --cc=tigran@aivazian.fsnet.co.uk \
    --cc=tj@kernel.org \
    --cc=x86@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;
as well as URLs for NNTP newsgroup(s).