Re: [PATCH 4/5] x86/platform/uv: Add deprecated messages to /proc info leaves

[Mike Travis <mike.travis@hpe.com>]

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;
>>   }
>>
>