Re: [PATCH] arch/x86/kernel/cpu/umwait.c - remove unused variable

[Thomas Gleixner]

On Thu, 8 Aug 2019, Fenghua Yu wrote:
> On Thu, Aug 08, 2019 at 10:32:38PM +0200, Thomas Gleixner wrote:
> > Valdis,
> > 
> > On Thu, 8 Aug 2019, Valdis KlÄtnieks wrote:
> > > On Thu, 08 Aug 2019 22:04:03 +0200, Thomas Gleixner said: It isn't 
> > > clear that whatever is doing the device_initcall()s will be able to 
> > > do any reasonable recovery if we return an error, so any error 
> > > recovery is going to have to happen before the function returns. It 
> > > might make sense to do an 'if (ret) return;' before going further in 
> > > the function, but given the comment a few lines further down about 
> > > ignoring errors, it was apparently considered more important to 
> > > struggle through and register stuff in sysfs even if umwait was 
> > > broken....
> > 
> > I missed that when going through it.
> > 
> > The right thing to do is to have a cpu_offline callback which clears 
> > the umwait MSR. That covers also the failure in the cpu hotplug setup. 
> > Then handling an error return makes sense and keeps everything in a 
> > workable shape.
> 
> When cpu is offline, the MSR won't be used. We don't need to clear it, right?

Groan. If soemthing goes wrong when registering the hotplug callback, what
undoes the MSR setup which might have happened and what takes care of it on
cpus coming online later? Exactly nothing. Then you have a non-consistent
behaviour.

Make stuff symmmetric and correct and not optimized for the sunshine case.

Thanks,

	tglx