Skip to contentSkip to conversation

likeliness accounting cleanup

PATCH In review Roel Kluin 3 participants 5 messages Mar 26, 2008 – Mar 28, 2008

Subsystems (2)
Library code Sparse checker
lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH v2 1/1 likeliness accounting cleanup

Store __builtin_return_address (caller) rather than __func__ in likeliness struct. 'line' and 'type' are combined in 'label'

+/- now denotes whether expectation fails in less than 5% of the tests - rather than whether more unexpected than expected were encountered. The function at the displayed filename & line and the caller are not necessarily the same. A few more Likely Profiling Results changes were made.

struct seq_operations becomes static, unsigned ints true and false (shadowed) are replaced by pos and neg.
---
New layout:

Likely Profiling Results
--------------------------------------------------------------------

[+- ]Type | # True  | # False | Function@Filename:Line
 unlikely |        0|    32082| fget+0xd0/0x1d0@include/asm/arch/atomic_32.h:235

Compiles and runs here. Thanks for previous comments.

 include/linux/compiler.h |   16 ++++++++--------
 lib/likely_prof.c        |   45 +++++++++++++++++++++++++--------------------
 2 files changed, 33 insertions(+), 28 deletions(-)
include/linux/compiler.h +8 -8
index d6d9efc..bd31d4c 100644
@@ -53,25 +53,25 @@ 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;
+	char *const file;
+	unsigned long caller;
 	unsigned int count[2];
 	struct likeliness *next;
+	unsigned int label;
 };
 
-extern int do_check_likely(struct likeliness *likeliness, int exp);
+extern int do_check_likely(struct likeliness *likeliness, unsigned int exp);
 
+#define LP_IS_EXPECTED	1
 #define LP_UNSEEN	4
+#define LP_LINE_SHIFT	3
 
 #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));			\
 	})
lib/likely_prof.c +25 -20

100 lines not on this page · raw message

author
#1 lore raw
Quoted 15 lines · Roel Kluin

Store __builtin_return_address (caller) rather than __func__ in likeliness struct. 'line' and 'type' are combined in 'label'

+/- now denotes whether expectation fails in less than 5% of the tests - rather than whether more unexpected than expected were encountered. The function at the displayed filename & line and the caller are not necessarily the same. A few more Likely Profiling Results changes were made.

struct seq_operations becomes static, unsigned ints true and false (shadowed) are replaced by pos and neg.
---
New layout:

Likely Profiling Results
--------------------------------------------------------------------

Quoted 9 lines
[+- ]Type | # True  | # False | Function@Filename:Line
 unlikely |        0|    32082| fget+0xd0/0x1d0@include/asm/arch/atomic_32.h:235

Compiles and runs here. Thanks for previous comments.

 include/linux/compiler.h |   16 ++++++++--------
 lib/likely_prof.c        |   45 +++++++++++++++++++++++++--------------------
 2 files changed, 33 insertions(+), 28 deletions(-)

This should be applied after the -mm patches:
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc5/2.6.25-rc5-mm1/broken-out/profile-likely-unlikely-macros.patch
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc5/2.6.25-rc5-mm1/broken-out/profile-likely-unlikely-macros-fix.patch

Also I forgot to add a signoff, so here it is:

Signed-off-by Roel Kluin <[email protected]>
#2 lore raw
In reply to Roel Kluin
Quoted 10 lines · Roel Kluin

Store __builtin_return_address (caller) rather than __func__ in likeliness struct. 'line' and 'type' are combined in 'label'

+/- now denotes whether expectation fails in less than 5% of the tests - rather than whether more unexpected than expected were encountered. The function at the displayed filename & line and the caller are not necessarily the same. A few more Likely Profiling Results changes were made.

struct seq_operations becomes static, unsigned ints true and false (shadowed) are replaced by pos and neg.

It's looks good to me .. You'll have to send it to Andrew to get it included tho ..

Daniel

author
#3 lore raw

PATCH v2 1/1 likeliness accounting change and cleanup

Quoted 4 lines · Daniel Walker

It's looks good to me .. You'll have to send it to Andrew to get it included tho ..

Daniel

Store __builtin_return_address (caller) rather than __func__ in likeliness struct. 'line' and 'type' are combined in 'label'

+/- now denotes whether expectation fails in less than 5% of the tests - rather than whether more unexpected than expected were encountered. The function at the displayed filename & line and the caller are not necessarily the same. A few more Likely Profiling Results changes were made.

struct seq_operations becomes static, unsigned ints true and false (shadowed) are replaced by pos and neg.

Signed-off-by Roel Kluin <[email protected]>

---
This should be applied after the -mm patches:
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc5/2.6.25-rc5-mm1/broken-out/profile-likely-unlikely-macros.patch
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc5/2.6.25-rc5-mm1/broken-out/profile-likely-unlikely-macros-fix.patch

New layout:

Likely Profiling Results
--------------------------------------------------------------------

[+- ]Type | # True  | # False | Function@Filename:Line
 unlikely |        0|    32082| fget+0xd0/0x1d0@include/asm/arch/atomic_32.h:235

Compiles and runs here. Thanks for previous comments.

 include/linux/compiler.h |   16 ++++++++--------
 lib/likely_prof.c        |   45 +++++++++++++++++++++++++--------------------
 2 files changed, 33 insertions(+), 28 deletions(-)
include/linux/compiler.h +8 -8
index d6d9efc..bd31d4c 100644
@@ -53,25 +53,25 @@ 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;
+	char *const file;
+	unsigned long caller;
 	unsigned int count[2];
 	struct likeliness *next;
+	unsigned int label;
 };
 
-extern int do_check_likely(struct likeliness *likeliness, int exp);
+extern int do_check_likely(struct likeliness *likeliness, unsigned int exp);
 
+#define LP_IS_EXPECTED	1
 #define LP_UNSEEN	4
+#define LP_LINE_SHIFT	3
 
 #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));			\
 	})
lib/likely_prof.c +25 -20

100 lines not on this page · raw message

Acked-by on v2 1/1
#4 lore raw

Re: [PATCH v2 -mm] likeliness accounting change and cleanup

Quoted 15 lines · Roel Kluin
Quoted 4 lines · Daniel Walker

It's looks good to me .. You'll have to send it to Andrew to get it included tho ..

Daniel

Store __builtin_return_address (caller) rather than __func__ in likeliness struct. 'line' and 'type' are combined in 'label'

+/- now denotes whether expectation fails in less than 5% of the tests - rather than whether more unexpected than expected were encountered. The function at the displayed filename & line and the caller are not necessarily the same. A few more Likely Profiling Results changes were made.

Quoted 4 lines

struct seq_operations becomes static, unsigned ints true and false (shadowed) are replaced by pos and neg.

Signed-off-by Roel Kluin <[email protected]>

Patch looks fine to me.

Acked-by Nick Piggin <[email protected]>

lib/likely_prof.c line 48

Quoted 12 lines
lib/likely_prof.c +4 -2
 		if (!test_and_set_bit(0, &likely_lock)) {
-			if (likeliness->type & LP_UNSEEN) {
-				likeliness->type &= (~LP_UNSEEN);
+			if (likeliness->label & LP_UNSEEN) {
+				likeliness->label &= (~LP_UNSEEN);
 				likeliness->next = likeliness_head;
 				likeliness_head = likeliness;
+				likeliness->caller = (unsigned long)
+						__builtin_return_address(0);
 			}
 			smp_mb__before_clear_bit();
 			clear_bit(0, &likely_lock);

While you're cleaning up this code, any chance you'd like to change this to test_and_set_bit_lock() / clear_bit_unlock() ? (in a 2nd patch).

The current usage is not wrong as such, but the _lock routines are faster and provide a better example to follow...