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

[Valdis KlÄtnieks]

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..

> > We get a warning when building with W=1:
>
> Please avoid 'We/I' in changelogs.

OK..

> >   CC      arch/x86/kernel/cpu/umwait.o
> > arch/x86/kernel/cpu/umwait.c: In function 'umwait_init':
> > arch/x86/kernel/cpu/umwait.c:183:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
> >   183 |  int ret;
> >       |      ^~~
> >
> > 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....

Attachment: pgp9MMUBgRP8Y.pgp
Description: PGP signature