x86/platform/uv: avoid unused variable warning

Message ID 20191212135815.4176658-1-arnd@arndb.de
State Superseded
Headers show
Series
  • x86/platform/uv: avoid unused variable warning
Related show

Commit Message

Arnd Bergmann Dec. 12, 2019, 1:58 p.m.
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

Comments

Arnd Bergmann Dec. 12, 2019, 2:01 p.m. | #1
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
Christoph Hellwig Dec. 12, 2019, 2:54 p.m. | #2
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.
Mike Travis Dec. 12, 2019, 3:16 p.m. | #3
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)

>  {

Patch

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 */