lkml.org 
[lkml]   [2019]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] arch/x86/kernel/cpu/umwait.c - remove unused variable
Valdis,

On Thu, 8 Aug 2019, Valdis Klētnieks wrote:

I really appreciate your work, but can you please refrain from using file
names as prefixes?

git log $FILE gives you usually a pretty good hint what the proper prefix
is:

bd9a0c97e53c ("x86/umwait: Add sysfs interface to control umwait maximum time")
ff4b353f2ef9 ("x86/umwait: Add sysfs interface to control umwait C0.2 state")
bd688c69b7e6 ("x86/umwait: Initialize umwait control values")

See?

> We get a warning when building with W=1:

Please avoid 'We/I' in changelogs.

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

> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
>
> diff --git a/arch/x86/kernel/cpu/umwait.c b/arch/x86/kernel/cpu/umwait.c
> index 6a204e7336c1..3d1d3952774a 100644
> --- a/arch/x86/kernel/cpu/umwait.c
> +++ b/arch/x86/kernel/cpu/umwait.c
> @@ -180,12 +180,11 @@ static struct attribute_group umwait_attr_group = {
> static int __init umwait_init(void)
> {
> struct device *dev;
> - int ret;
>
> if (!boot_cpu_has(X86_FEATURE_WAITPKG))
> return -ENODEV;
>
> - ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "umwait:online",
> + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "umwait:online",
> umwait_cpu_online, NULL);

If that fails then umwait is broken. So instead of removing it, this should
actually check the return code and act accordingly. Fenghua?

> register_syscore_ops(&umwait_syscore_ops);

Thanks,

tglx
\
 
 \ /
  Last update: 2019-08-08 22:05    [W:0.217 / U:0.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site