> -----Original Message----- > From: Matt Fleming [mailto:matt@xxxxxxxxxxxxxxxxxxx] > Sent: Friday, January 8, 2016 6:19 AM > To: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > Cc: Elliott, Robert (Persistent Memory) <elliott@xxxxxxx>; Thomas Gleixner > <tglx@xxxxxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>; H. Peter Anvin > <hpa@xxxxxxxxx>; x86@xxxxxxxxxx; linux-efi@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 4/4] x86/efi: print size and base in binary units in > efi_print_memmap > > On Sun, 27 Dec, at 04:35:12PM, Andy Shevchenko wrote: > > On Mon, Dec 21, 2015 at 6:16 PM, Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx> > wrote: > > >> diff --git a/arch/x86/platform/efi/efi.c > b/arch/x86/platform/efi/efi.c > > >> index 635a955..030ba91 100644 > > >> --- a/arch/x86/platform/efi/efi.c > > >> +++ b/arch/x86/platform/efi/efi.c > > >> @@ -222,6 +222,25 @@ int __init efi_memblock_x86_reserve_range(void) > > >> return 0; > > >> } > > >> > > >> +char * __init efi_size_format(char *buf, size_t size, u64 bytes) > > >> +{ > > >> + if (!bytes || (bytes & 0x3ff)) > > >> + snprintf(buf, size, "%llu B", bytes); > > >> + else if (bytes & 0xfffff) > > >> + snprintf(buf, size, "%llu KiB", bytes >> 10); > > >> + else if (bytes & 0x3fffffff) > > >> + snprintf(buf, size, "%llu MiB", bytes >> 20); > > >> + else if (bytes & 0xffffffffff) > > >> + snprintf(buf, size, "%llu GiB", bytes >> 30); > > >> + else if (bytes & 0x3ffffffffffff) > > >> + snprintf(buf, size, "%llu TiB", bytes >> 40); > > >> + else if (bytes & 0xfffffffffffffff) > > >> + snprintf(buf, size, "%llu PiB", bytes >> 50); > > >> + else > > >> + snprintf(buf, size, "%llu EiB", bytes >> 60); > > >> + return buf; > > > > For me it looks like ffs with name in the table can be used. > > Could you provide a patch? I think this is functionally equivalent: #include <string.h> char * efi_size_format_ffsl(char *buf, size_t size, u64 bytes) { if (!bytes || ffsl(bytes) < 10) snprintf(buf, size, "%llu B", bytes); else if (ffsl(bytes) < 20) snprintf(buf, size, "%llu KiB", bytes >> 10); else if (ffsl(bytes) < 30) snprintf(buf, size, "%llu MiB", bytes >> 20); else if (ffsl(bytes) < 40) snprintf(buf, size, "%llu GiB", bytes >> 30); else if (ffsl(bytes) < 50) snprintf(buf, size, "%llu TiB", bytes >> 40); else if (ffsl(bytes) < 60) snprintf(buf, size, "%llu PiB", bytes >> 50); else snprintf(buf, size, "%llu EiB", bytes >> 60); return buf; } Compiled as a user program with gcc -O2, the original results in mov and testq instructions: movq %rdi, %rbx je .L2 testl $1023, %edx jne .L2 testl $1048575, %edx jne .L15 testl $1073741823, %edx jne .L16 movabsq $1099511627775, %rax testq %rax, %rdx jne .L17 movabsq $1125899906842623, %rax testq %rax, %rdx jne .L18 movabsq $1152921504606846975, %rax movq %rdx, %rcx testq %rax, %rdx jne .L19 while the ffs version uses bit scan forward (bsfq) and only needs cmpl instructions since the values are smaller: movq %rdi, %rbx je .L21 bsfq %rdx, %rcx addq $1, %rcx cmpl $9, %ecx jle .L21 cmpl $19, %ecx jle .L33 cmpl $29, %ecx jle .L34 cmpl $39, %ecx .p2align 4,,2 jle .L35 cmpl $49, %ecx .p2align 4,,2 jle .L36 cmpl $59, %ecx .p2align 4,,2 jle .L37 The kernel offers ffs(int x) but not ffsl(), and it uses inline assembly for one of these: bsfl bsfl, cmovzl bsfl, jnz, movl I don't know which code is the most efficient. --- Robert Elliott, HPE Persistent Memory -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
