| Message ID | 20191212135815.4176658-1-arnd@arndb.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/platform/uv: avoid unused variable warning | expand |
On Thu, Dec 12, 2019 at 2:58 PM Arnd Bergmann <arnd@arndb.de> wrote: > > When CONFIG_PROC_FS is disabled, the compiler warns about an > unused variable: > > arch/x86/kernel/apic/x2apic_uv_x.c: In function 'uv_setup_proc_files': > arch/x86/kernel/apic/x2apic_uv_x.c:1546:8: error: unused variable 'name' [-Werror=unused-variable] > char *name = hubless ? "hubless" : "hubbed"; > > Simplify the code so this variable is no longer needed. > > Fixes: 8785968bce1c ("x86/platform/uv: Add UV Hubbed/Hubless Proc FS Files") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I noticed a second too late that this loses the oemid procfs file (annoyingly I did not get a warning because gcc ignores unused static const variables) Please wait for v2. Arnd
Instead of that maybe_unused mess please just use good old ifdefs. > if (hubless) > - proc_version_fops.open = proc_hubless_open; > + proc_create_single("hubless", 0, pde, proc_hubless_show); > else > - proc_version_fops.open = proc_hubbed_open; > + proc_create_single("hubbed", 0, pde, proc_hubbed_show); > } Or someone could figure out what happens if we turn the proc_create_single stub into an inline function instead of the define. That makes it used at a syntactic level, the big question is if the compiler is smart enough to optimize away the unused callback still.
On 12/12/2019 6:54 AM, Christoph Hellwig wrote: > Instead of that maybe_unused mess please just use good old ifdefs. > >> if (hubless) >> - proc_version_fops.open = proc_hubless_open; >> + proc_create_single("hubless", 0, pde, proc_hubless_show); >> else >> - proc_version_fops.open = proc_hubbed_open; >> + proc_create_single("hubbed", 0, pde, proc_hubbed_show); >> } > > Or someone could figure out what happens if we turn the > proc_create_single stub into an inline function instead of the > define. That makes it used at a syntactic level, the big question is > if the compiler is smart enough to optimize away the unused callback > still. > Yes, if CONFIG_PROC_FS is undefined, then this whole section can be removed since it's sole purpose is to set up the /proc/ interface. Something like this should suffice: > --- > arch/x86/kernel/apic/x2apic_uv_x.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > --- linux.orig/arch/x86/kernel/apic/x2apic_uv_x.c > +++ linux/arch/x86/kernel/apic/x2apic_uv_x.c > @@ -1533,6 +1533,7 @@ static void check_efi_reboot(void) > reboot_type = BOOT_ACPI; > } > > +#ifdef CONFIG_PROC_FS > /* Setup user proc fs files */ > static int proc_hubbed_show(struct seq_file *file, void *data) > { > @@ -1595,6 +1596,14 @@ static __init void uv_setup_proc_files(i > proc_version_fops.open = proc_hubbed_open; > } > > +#else /* !CONFIG_PROC_FS */ > + > +static __init void uv_setup_proc_files(int hubless) > +{ > +} > + > +#endif /* !CONFIG_PROC_FS */ > + > /* Initialize UV hubless systems */ > static __init int uv_system_init_hubless(void) > {
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c index d5b51a740524..1af416da1f87 100644 --- a/arch/x86/kernel/apic/x2apic_uv_x.c +++ b/arch/x86/kernel/apic/x2apic_uv_x.c @@ -1493,13 +1493,13 @@ static void check_efi_reboot(void) } /* Setup user proc fs files */ -static int proc_hubbed_show(struct seq_file *file, void *data) +static int __maybe_unused proc_hubbed_show(struct seq_file *file, void *data) { seq_printf(file, "0x%x\n", uv_hubbed_system); return 0; } -static int proc_hubless_show(struct seq_file *file, void *data) +static int __maybe_unused proc_hubless_show(struct seq_file *file, void *data) { seq_printf(file, "0x%x\n", uv_hubless_system); return 0; @@ -1511,28 +1511,11 @@ static int proc_oemid_show(struct seq_file *file, void *data) return 0; } -static int proc_hubbed_open(struct inode *inode, struct file *file) -{ - return single_open(file, proc_hubbed_show, (void *)NULL); -} - -static int proc_hubless_open(struct inode *inode, struct file *file) -{ - return single_open(file, proc_hubless_show, (void *)NULL); -} - static int proc_oemid_open(struct inode *inode, struct file *file) { return single_open(file, proc_oemid_show, (void *)NULL); } -/* (struct is "non-const" as open function is set at runtime) */ -static struct file_operations proc_version_fops = { - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - static const struct file_operations proc_oemid_fops = { .open = proc_oemid_open, .read = seq_read, @@ -1543,15 +1526,12 @@ static const struct file_operations proc_oemid_fops = { static __init void uv_setup_proc_files(int hubless) { struct proc_dir_entry *pde; - char *name = hubless ? "hubless" : "hubbed"; pde = proc_mkdir(UV_PROC_NODE, NULL); - proc_create("oemid", 0, pde, &proc_oemid_fops); - proc_create(name, 0, pde, &proc_version_fops); if (hubless) - proc_version_fops.open = proc_hubless_open; + proc_create_single("hubless", 0, pde, proc_hubless_show); else - proc_version_fops.open = proc_hubbed_open; + proc_create_single("hubbed", 0, pde, proc_hubbed_show); } /* Initialize UV hubless systems */
When CONFIG_PROC_FS is disabled, the compiler warns about an unused variable: arch/x86/kernel/apic/x2apic_uv_x.c: In function 'uv_setup_proc_files': arch/x86/kernel/apic/x2apic_uv_x.c:1546:8: error: unused variable 'name' [-Werror=unused-variable] char *name = hubless ? "hubless" : "hubbed"; Simplify the code so this variable is no longer needed. Fixes: 8785968bce1c ("x86/platform/uv: Add UV Hubbed/Hubless Proc FS Files") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/x86/kernel/apic/x2apic_uv_x.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) -- 2.20.0