[4/5] x86/platform/uv: Add deprecated messages to /proc info leaves
diff mbox series

Message ID 20201125172907.240996-5-mike.travis@hpe.com
State New, archived
Headers show
Series
  • x86/platform/uv: Move UV procfs leaves to sysfs
Related show

Commit Message

Mike Travis Nov. 25, 2020, 5:29 p.m. UTC
Add "deprecated" message to any access to old /proc/sgi_uv/* leaves.

Signed-off-by: Mike Travis <mike.travis@hpe.com>
Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
---
 arch/x86/kernel/apic/x2apic_uv_x.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Hans de Goede Nov. 26, 2020, 10:45 a.m. UTC | #1
Hi,

On 11/25/20 6:29 PM, Mike Travis wrote:
> Add "deprecated" message to any access to old /proc/sgi_uv/* leaves.
> 
> Signed-off-by: Mike Travis <mike.travis@hpe.com>
> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> ---
>  arch/x86/kernel/apic/x2apic_uv_x.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
> index 48746031b39a..bfd77a00c2a1 100644
> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -1615,21 +1615,45 @@ static void check_efi_reboot(void)
>  		reboot_type = BOOT_ACPI;
>  }
>  
> -/* Setup user proc fs files */
> +/*
> + * User proc fs file handling now deprecated.
> + * Recommend using /sys/firmware/sgi_uv/... instead.
> + */
> +static void proc_print_msg(int *flag, char *what, char *which)
> +{
> +	if (*flag)
> +		return;
> +
> +	pr_notice(
> +		"%s: using deprecated /proc/sgi_uv/%s, use /sys/firmware/sgi_uv/%s\n",
> +		current->comm, what, which ? which : what);
> +
> +	*flag = 1;
> +}
> +

You have just re-invented pr_notice_once, please just use pr_notice_once
directly in the _show functions.

Regards,

Hans




>  static int __maybe_unused proc_hubbed_show(struct seq_file *file, void *data)
>  {
> +	static int flag;
> +
> +	proc_print_msg(&flag, "hubbed", "hub_type");
>  	seq_printf(file, "0x%x\n", uv_hubbed_system);
>  	return 0;
>  }
>  
>  static int __maybe_unused proc_hubless_show(struct seq_file *file, void *data)
>  {
> +	static int flag;
> +
> +	proc_print_msg(&flag, "hubless", NULL);
>  	seq_printf(file, "0x%x\n", uv_hubless_system);
>  	return 0;
>  }
>  
>  static int __maybe_unused proc_archtype_show(struct seq_file *file, void *data)
>  {
> +	static int flag;
> +
> +	proc_print_msg(&flag, "archtype", NULL);
>  	seq_printf(file, "%s/%s\n", uv_archtype, oem_table_id);
>  	return 0;
>  }
>
Mike Travis Nov. 27, 2020, 2:58 p.m. UTC | #2
On 11/26/2020 2:45 AM, Hans de Goede wrote:
> Hi,
> 
> On 11/25/20 6:29 PM, Mike Travis wrote:
>> Add "deprecated" message to any access to old /proc/sgi_uv/* leaves.
>>
>> Signed-off-by: Mike Travis <mike.travis@hpe.com>
>> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
>> ---
>>   arch/x86/kernel/apic/x2apic_uv_x.c | 26 +++++++++++++++++++++++++-
>>   1 file changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
>> index 48746031b39a..bfd77a00c2a1 100644
>> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
>> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
>> @@ -1615,21 +1615,45 @@ static void check_efi_reboot(void)
>>   		reboot_type = BOOT_ACPI;
>>   }
>>   
>> -/* Setup user proc fs files */
>> +/*
>> + * User proc fs file handling now deprecated.
>> + * Recommend using /sys/firmware/sgi_uv/... instead.
>> + */
>> +static void proc_print_msg(int *flag, char *what, char *which)
>> +{
>> +	if (*flag)
>> +		return;
>> +
>> +	pr_notice(
>> +		"%s: using deprecated /proc/sgi_uv/%s, use /sys/firmware/sgi_uv/%s\n",
>> +		current->comm, what, which ? which : what);
>> +
>> +	*flag = 1;
>> +}
>> +
> 
> You have just re-invented pr_notice_once, please just use pr_notice_once
> directly in the _show functions.

I tried it both ways (actually with rate limiting as well).  The problem 
with using a static check in the error print function it will only print 
the first instance it encounters, not all of the references.

If I move it to the final output I need to replicate the verbiage of the 
format for every instance as you can't seem to combine the KERN_* level 
of printing and the pr_fmt reference of the format string.  I tried a 
few ways including just putting everything into a format character list. 
  But what used to work (indirect format pointer) doesn't any more.  Or 
I didn't hit on the correct combination of KERN_* level and indirect 
format string.

The last combination was no print limiting which caused of course the 
error message to be output on every occurrence.  (NASA has 35,000 
customers for their big systems, that's a lot of potential console 
messages.)  This really annoys them and we would get calls from those 
that don't have any means of changing this so they ask us.

So I just chose this method of accomplishing all goals, except of course 
using the higher level of print function (pr_notice_once).  But if you 
think method two ("use pr_notice_once directly in the _show function") 
is most favorable I will switch to that.  Thanks.

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
>>   static int __maybe_unused proc_hubbed_show(struct seq_file *file, void *data)
>>   {
>> +	static int flag;
>> +
>> +	proc_print_msg(&flag, "hubbed", "hub_type");
>>   	seq_printf(file, "0x%x\n", uv_hubbed_system);
>>   	return 0;
>>   }
>>   
>>   static int __maybe_unused proc_hubless_show(struct seq_file *file, void *data)
>>   {
>> +	static int flag;
>> +
>> +	proc_print_msg(&flag, "hubless", NULL);
>>   	seq_printf(file, "0x%x\n", uv_hubless_system);
>>   	return 0;
>>   }
>>   
>>   static int __maybe_unused proc_archtype_show(struct seq_file *file, void *data)
>>   {
>> +	static int flag;
>> +
>> +	proc_print_msg(&flag, "archtype", NULL);
>>   	seq_printf(file, "%s/%s\n", uv_archtype, oem_table_id);
>>   	return 0;
>>   }
>>
>
Hans de Goede Nov. 27, 2020, 6:06 p.m. UTC | #3
Hi,

On 11/27/20 3:58 PM, Mike Travis wrote:
> 
> 
> On 11/26/2020 2:45 AM, Hans de Goede wrote:
>> Hi,
>>
>> On 11/25/20 6:29 PM, Mike Travis wrote:
>>> Add "deprecated" message to any access to old /proc/sgi_uv/* leaves.
>>>
>>> Signed-off-by: Mike Travis <mike.travis@hpe.com>
>>> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
>>> ---
>>>   arch/x86/kernel/apic/x2apic_uv_x.c | 26 +++++++++++++++++++++++++-
>>>   1 file changed, 25 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
>>> index 48746031b39a..bfd77a00c2a1 100644
>>> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
>>> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
>>> @@ -1615,21 +1615,45 @@ static void check_efi_reboot(void)
>>>           reboot_type = BOOT_ACPI;
>>>   }
>>>   -/* Setup user proc fs files */
>>> +/*
>>> + * User proc fs file handling now deprecated.
>>> + * Recommend using /sys/firmware/sgi_uv/... instead.
>>> + */
>>> +static void proc_print_msg(int *flag, char *what, char *which)
>>> +{
>>> +    if (*flag)
>>> +        return;
>>> +
>>> +    pr_notice(
>>> +        "%s: using deprecated /proc/sgi_uv/%s, use /sys/firmware/sgi_uv/%s\n",
>>> +        current->comm, what, which ? which : what);
>>> +
>>> +    *flag = 1;
>>> +}
>>> +
>>
>> You have just re-invented pr_notice_once, please just use pr_notice_once
>> directly in the _show functions.
> 
> I tried it both ways (actually with rate limiting as well).  The problem with using a static check in the error print function it will only print the first instance it encounters, not all of the references.
> 
> If I move it to the final output I need to replicate the verbiage of the format for every instance as you can't seem to combine the KERN_* level of printing and the pr_fmt reference of the format string.  I tried a few ways including just putting everything into a format character list.  But what used to work (indirect format pointer) doesn't any more.  Or I didn't hit on the correct combination of KERN_* level and indirect format string.
> 
> The last combination was no print limiting which caused of course the error message to be output on every occurrence.  (NASA has 35,000 customers for their big systems, that's a lot of potential console messages.)  This really annoys them and we would get calls from those that don't have any means of changing this so they ask us.
> 
> So I just chose this method of accomplishing all goals, except of course using the higher level of print function (pr_notice_once).  But if you think method two ("use pr_notice_once directly in the _show function") is most favorable I will switch to that.

Yeah I think using that is much better then reinventing it, you can simply just write
out the 3 different messages which you are "formatting" now as static strings so you get:

	pr_notice_once("%s: using deprecated /proc/sgi_uv/hubbed, use /sys/firmware/sgi_uv/hub_type\n", current->comm);

	pr_notice_once("%s: using deprecated /proc/sgi_uv/hubless, use /sys/firmware/sgi_uv/hubless\n", current->comm);

	pr_notice_once("%s: using deprecated /proc/sgi_uv/archtype, use /sys/firmware/sgi_uv/archtype\n", current->comm);

Regards,

Hans




> 
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>>   static int __maybe_unused proc_hubbed_show(struct seq_file *file, void *data)
>>>   {
>>> +    static int flag;
>>> +
>>> +    proc_print_msg(&flag, "hubbed", "hub_type");
>>>       seq_printf(file, "0x%x\n", uv_hubbed_system);
>>>       return 0;
>>>   }
>>>     static int __maybe_unused proc_hubless_show(struct seq_file *file, void *data)
>>>   {
>>> +    static int flag;
>>> +
>>> +    proc_print_msg(&flag, "hubless", NULL);
>>>       seq_printf(file, "0x%x\n", uv_hubless_system);
>>>       return 0;
>>>   }
>>>     static int __maybe_unused proc_archtype_show(struct seq_file *file, void *data)
>>>   {
>>> +    static int flag;
>>> +
>>> +    proc_print_msg(&flag, "archtype", NULL);
>>>       seq_printf(file, "%s/%s\n", uv_archtype, oem_table_id);
>>>       return 0;
>>>   }
>>>
>>
>

Patch
diff mbox series

diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 48746031b39a..bfd77a00c2a1 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -1615,21 +1615,45 @@  static void check_efi_reboot(void)
 		reboot_type = BOOT_ACPI;
 }
 
-/* Setup user proc fs files */
+/*
+ * User proc fs file handling now deprecated.
+ * Recommend using /sys/firmware/sgi_uv/... instead.
+ */
+static void proc_print_msg(int *flag, char *what, char *which)
+{
+	if (*flag)
+		return;
+
+	pr_notice(
+		"%s: using deprecated /proc/sgi_uv/%s, use /sys/firmware/sgi_uv/%s\n",
+		current->comm, what, which ? which : what);
+
+	*flag = 1;
+}
+
 static int __maybe_unused proc_hubbed_show(struct seq_file *file, void *data)
 {
+	static int flag;
+
+	proc_print_msg(&flag, "hubbed", "hub_type");
 	seq_printf(file, "0x%x\n", uv_hubbed_system);
 	return 0;
 }
 
 static int __maybe_unused proc_hubless_show(struct seq_file *file, void *data)
 {
+	static int flag;
+
+	proc_print_msg(&flag, "hubless", NULL);
 	seq_printf(file, "0x%x\n", uv_hubless_system);
 	return 0;
 }
 
 static int __maybe_unused proc_archtype_show(struct seq_file *file, void *data)
 {
+	static int flag;
+
+	proc_print_msg(&flag, "archtype", NULL);
 	seq_printf(file, "%s/%s\n", uv_archtype, oem_table_id);
 	return 0;
 }