change likeliness accounting
Subsystems (2)
PATCH 1/1 change likeliness accounting
In the patch below I propose some changes to the likely/unlikely accounting.
---
In struct likeliness: store __builtin_return_address in 'caller', rather than __FILE__ and __func__, and combine 'type' and 'line' into one 'label'.
The __check_likely macro performs some functionality originally in do_check_likely(): The first check for LP_UNSEEN bit, the increment of the experienced likely-count. do_check_likely() looses its parameter and return value. I added a __builtin_expect() to make use of the expectation.
The likely diplay changes: now +/- denotes whether expectation fails in less than 0.01% of the tests rather than whether more unexpected than expected were encountered. __FILE__ is no longer displayed in output.
struct seq_operations becomes static and corrects a typo in comment.
---
include/linux/compiler.h +15 -11
@@ -53,27 +53,31 @@ extern void __chk_io_ptr(const volatile void __iomem *);#if defined(CONFIG_PROFILE_LIKELY ) && !(defined(CONFIG_MODULE_UNLOAD ) && defined(MODULE ))struct likeliness { -const char *func; -char *file; -int line; -int type; +unsigned long caller; +unsigned int label;unsigned int count[2 ];struct likeliness *next; }; -extern int do_check_likely (struct likeliness *likeliness,int exp); +extern void do_check_likely (struct likeliness *likeliness); -#define LP_UNSEEN 4 +#define LP_IS_EXPECTED 1 +#define LP_UNSEEN 2 +#define LP_LINE_SHIFT 2#define __check_likely (exp, is_likely) \ ({ \ static struct likeliness likeliness = { \ - .func = __func__, \ - .file = __FILE__, \ - .line = __LINE__, \ - .type = is_likely | LP_UNSEEN, \ + .label = __LINE__ << LP_LINE_SHIFT | \ + LP_UNSEEN | is_likely, \ }; \ - do_check_likely(&likeliness, !!(exp)); \ + \ + if (likeliness.label & LP_UNSEEN) \ + do_check_likely(&likeliness); \ + \ + __builtin_expect(!!(exp), is_likely) ? \ + likeliness.count[1]++ || 1 : \ + likeliness.count[0]++ && 0; \ })/*
lib/likely_prof.c +31 -33
105 lines not on this page · raw message
lib/likely_prof.c static int lp_seq_show(struct seq_file *out, void *p) · line 107
Quoted 5 lines · Roel Kluin
lib/likely_prof.c +3
-2-seq_printf (out,"|%9u|%9u\t%s()@:%s@%d\n" , true, false, - entry->func, entry->file, entry->line); +sprint_symbol (function, entry->caller); +seq_printf (out,"|%9u|%9u\t%s()@:%u\n" , pos, neg, function, + entry->label >>LP_LINE_SHIFT );
Looks like we're missing a "|" after the second %9u above .. You could add that as long as your modifying this area.. Also your tabbing is a little too heavy ..
I wonder if you think the change to saving the caller EIP is better than saving the file name? The size saving in the likeliness struct is nice, but since this is for debugging is not at all required ..
Overall it looks like a nice update/cleanup ..
(CC'd Andrew and Hua..)
Daniel
Quoted 4 lines · Roel Kluin
#define
__check_likely(exp, is_likely)\include/linux/compiler.h +0
-1({ \ static struct likeliness likeliness = { \ - .func = __func__, \
include/linux/compiler.h extern void __chk_io_ptr(const volatile void __iomem *); · line 76
Quoted 10 lines
include/linux/compiler.h +5
-4- .file = __FILE__, \ - .line = __LINE__, \ - .type = is_likely | LP_UNSEEN, \ + .label = __LINE__ << LP_LINE_SHIFT | \ + LP_UNSEEN | is_likely, \ }; \ -do_check_likely (&likeliness, !!(exp)); \ + \ +if (likeliness.label &LP_UNSEEN ) \ +do_check_likely (&likeliness); \
On second look, your actually break even on size (if not a little worse) cause you adding some addition code into the macro which effectively inlines it at all the likely/unlikely locations .. I'm not sure that's really necessary ..
Also by not including the file name you have situation like the following,
+unlikely | 208324| 4010128| do_sys_open+0x2a/0xb2()@:34
Where do_sys_open is on line 1076 in file fs/open.c , but the likely location is actual on like 34 in file include/linux/err.h ..
So I think saving of the EIP does help in finding the actually caller, but I do think we loose something by removing the file name..
Daniel
Quoted 5 lines · Roel Kluin
In the patch below I propose some changes to the likely/unlikely accounting. ---
In struct likeliness: store__builtin_return_addressin 'caller', rather than__FILE__and__func__, and combine 'type' and 'line' into one 'label'.
Quoted 8 lines
The
__check_likelymacro performs some functionality originally indo_check_likely(): The first check forLP_UNSEENbit, the increment of the experienced likely-count.do_check_likely()looses its parameter and return value. I added a__builtin_expect()to make use of the expectation.The likely diplay changes: now +/- denotes whether expectation fails in less than 0.01% of the tests rather than whether more unexpected than expected were encountered.
__FILE__is no longer displayed in output.
0.01%, so 1 time every 10 000 tests.
That seems pretty excessive. If the 9 999 tests which were taken correctly each saved only 0.1 cycle due to the likely annotation, then that would be implying the same likely annotation adds 1 000 cycles to the cost of the branch when taken that way.
In practice, I'd say it is probably just going to cost an extra icache miss, another jump, and maybe a cycle here or there... 100 cycles at worst. And the benefit might also be larger than 0.1 cycle because it could save some icache and reduce cold branch mispredicts. It will also make the next gcc use jumps instead of cmov, which reduces hazards to the out of order engine for example.
I think 1-5% is more reasonable a number if I were to pull one out of my hat.