On Fri, Jan 8, 2016 at 6:38 PM, Elliott, Robert (Persistent Memory) <elliott@xxxxxxx> wrote: >> -----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; > } No, no, I meant something slightly different. We already have a table of units. Needs to be shared (patch already cooked), second stage is to provide proper number and units. Something like units = string_units_2; units_index = __ffs64(value) / 10; value >>= units_index * 10; snprintf("%llu%s", value, units[units_index]); > > 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(), See above. > 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. -- With Best Regards, Andy Shevchenko -- 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
