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 <[email protected]>
---
arch/x86/kernel/apic/x2apic_uv_x.c | 28 ++++------------------------
1 file changed, 4 insertions(+), 24 deletions(-)
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 */
--
2.20.0
On Thu, Dec 12, 2019 at 2:58 PM Arnd Bergmann <[email protected]> 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 <[email protected]>
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)
> {