Re: [PATCH v2] likeliness accounting cleanup

Previous thread: [TRIVIAL PATCH] Microcode: show results on success too plus fix typo by Ben Castricum on Wednesday, March 26, 2008 - 1:22 pm. (4 messages)

Next thread: Re: some minor issues with 2.6.25-rc6-git7-default by devzero on Wednesday, March 26, 2008 - 1:51 pm. (3 messages)
From: Roel Kluin
Date: Wednesday, March 26, 2008 - 1:46 pm

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(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index d6d9efc..bd31d4c 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -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 | ...
From: Roel Kluin
Date: Thursday, March 27, 2008 - 5:37 am

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-m...
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc5/2.6.25-rc5-m...

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

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
--

From: Daniel Walker
Date: Thursday, March 27, 2008 - 9:45 am

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

Daniel

--

From: Roel Kluin
Date: Thursday, March 27, 2008 - 1:25 pm

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 <12o3l@tiscali.nl>
---
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-m...
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc5/2.6.25-rc5-m...

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(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index d6d9efc..bd31d4c 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -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 ...
From: Nick Piggin
Date: Thursday, March 27, 2008 - 10:09 pm

Patch looks fine to me.


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...

--

Previous thread: [TRIVIAL PATCH] Microcode: show results on success too plus fix typo by Ben Castricum on Wednesday, March 26, 2008 - 1:22 pm. (4 messages)

Next thread: Re: some minor issues with 2.6.25-rc6-git7-default by devzero on Wednesday, March 26, 2008 - 1:51 pm. (3 messages)