Re: [PATCH] arch_numa: fix common code printing of phys_addr_t

[Randy Dunlap]

On 2/1/21 7:36 PM, Palmer Dabbelt wrote:
> On Wed, 27 Jan 2021 19:55:33 PST (-0800), rdunlap@xxxxxxxxxxxxx wrote:
>> Fix build warnings in the arch_numa common code:
>>
>> ../include/linux/kern_levels.h:5:18: warning: format '%Lx' expects argument of type 'long long unsigned int', but argument 3 has type 'phys_addr_t' {aka 'unsigned int'} [-Wformat=]
>> ../drivers/base/arch_numa.c:360:56: note: format string is defined here
>>   360 |    pr_warn("Warning: invalid memblk node %d [mem %#010Lx-%#010Lx]\n",
>> ../drivers/base/arch_numa.c:435:39: note: format string is defined here
>>   435 |  pr_info("Faking a node at [mem %#018Lx-%#018Lx]\n", start, end - 1);
>>
>> Fixes: ae3c107cd8be ("numa: Move numa implementation to common code")
>> Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
>> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>> Cc: Atish Patra <atish.patra@xxxxxxx>
>> Cc: Palmer Dabbelt <palmerdabbelt@xxxxxxxxxx>
>> ---
>>  drivers/base/arch_numa.c |   13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> --- linux-next-20210125.orig/drivers/base/arch_numa.c
>> +++ linux-next-20210125/drivers/base/arch_numa.c
>> @@ -355,11 +355,12 @@ static int __init numa_register_nodes(vo
>>      /* Check that valid nid is set to memblks */
>>      for_each_mem_region(mblk) {
>>          int mblk_nid = memblock_get_region_node(mblk);
>> +        phys_addr_t start = mblk->base;
>> +        phys_addr_t end = mblk->base + mblk->size - 1;
>>
>>          if (mblk_nid == NUMA_NO_NODE || mblk_nid >= MAX_NUMNODES) {
>> -            pr_warn("Warning: invalid memblk node %d [mem %#010Lx-%#010Lx]\n",
>> -                mblk_nid, mblk->base,
>> -                mblk->base + mblk->size - 1);
>> +            pr_warn("Warning: invalid memblk node %d [mem %pap-%pap]\n",
>> +                mblk_nid, &start, &end);
>>              return -EINVAL;
>>          }
>>      }
>> @@ -427,14 +428,14 @@ out_free_distance:
>>  static int __init dummy_numa_init(void)
>>  {
>>      phys_addr_t start = memblock_start_of_DRAM();
>> -    phys_addr_t end = memblock_end_of_DRAM();
>> +    phys_addr_t end = memblock_end_of_DRAM() - 1;
>>      int ret;
>>
>>      if (numa_off)
>>          pr_info("NUMA disabled\n"); /* Forced off on command line. */
>> -    pr_info("Faking a node at [mem %#018Lx-%#018Lx]\n", start, end - 1);
>> +    pr_info("Faking a node at [mem %pap-%pap]\n", &start, &end);
>>
>> -    ret = numa_add_memblk(0, start, end);
>> +    ret = numa_add_memblk(0, start, end + 1);
>>      if (ret) {
>>          pr_err("NUMA init failed\n");
>>          return ret;
> 
> Thanks, this is on for-next.  Did you, by any chance, find %Lx documented
> anywhere?  It's not ISO C and the GCC source code says it's a GNU extension,
> but I couldn't find it in the documentation (or even where to add it, which I
> guess is how I forgot to send my version fo the patch).

'man sprintf' says this:

       As  a nonstandard extension, the GNU implementations treats ll and L as
       synonyms, so that one can, for example, write llg (as a synonym for the
       standards-compliant  Lg) and Ld (as a synonym for the standards compli-
       ant lld).  Such usage is nonportable.


and linux/lib/vsprintf.c has some handling for it:

	if (qualifier == 'L')
		spec->type = FORMAT_TYPE_LONG_LONG;

and

		case 'L':
			if (is_sign)
				*va_arg(args, long long *) = val.s;
			else
				*va_arg(args, unsigned long long *) = val.u;
			break;


Does that help?

-- 
~Randy