Messages in this thread | | | Date | Thu, 8 Aug 2019 22:32:38 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH] arch/x86/kernel/cpu/umwait.c - remove unused variable |
| |
Valdis,
On Thu, 8 Aug 2019, Valdis Klētnieks wrote: > On Thu, 08 Aug 2019 22:04:03 +0200, Thomas Gleixner said: > > > I really appreciate your work, but can you please refrain from using file > > names as prefixes? > > OK, will do so going forward..
Care to resend the last few with fixed subject lines, so I don't have to clean them up?
> > > And indeed, we don't do anything with it, so clean it up. > > > > Well, the question is whether removing the variable is the right thing to > > do. > > > If that fails then umwait is broken. So instead of removing it, this should > > actually check the return code and act accordingly. Fenghua? > > [/usr/src/linux-next] git grep umwait_init > arch/x86/kernel/cpu/umwait.c:static int __init umwait_init(void) > arch/x86/kernel/cpu/umwait.c:device_initcall(umwait_init); > > 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.
Thanks,
tglx
| |