* [PATCH] scripts/faddr2line: show the code context
@ 2018-03-19 7:23 changbin.du
2018-05-29 16:03 ` Peter Zijlstra
0 siblings, 1 reply; 9+ messages in thread
From: changbin.du @ 2018-03-19 7:23 UTC (permalink / raw)
To: akpm, tglx, pombredanne, neilb; +Cc: linux-kernel, Changbin Du
From: Changbin Du <changbin.du@intel.com>
Inspired by gdb command 'list', show the code context of target lines.
Here is a example:
$ scripts/faddr2line vmlinux native_write_msr+0x6
native_write_msr+0x6/0x20:
arch_static_branch at arch/x86/include/asm/msr.h:105
100 return EAX_EDX_VAL(val, low, high);
101 }
102
103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
104 {
105 asm volatile("1: wrmsr\n"
106 "2:\n"
107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
108 : : "c" (msr), "a"(low), "d" (high) : "memory");
109 }
110
(inlined by) static_key_false at include/linux/jump_label.h:142
137 #define JUMP_TYPE_LINKED 2UL
138 #define JUMP_TYPE_MASK 3UL
139
140 static __always_inline bool static_key_false(struct static_key *key)
141 {
142 return arch_static_branch(key, false);
143 }
144
145 static __always_inline bool static_key_true(struct static_key *key)
146 {
147 return !arch_static_branch(key, true);
(inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
145 static inline void notrace
146 native_write_msr(unsigned int msr, u32 low, u32 high)
147 {
148 __wrmsr(msr, low, high);
149
150 if (msr_tracepoint_active(__tracepoint_write_msr))
151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
152 }
153
154 /* Can be uninlined because referenced by paravirt */
155 static inline int notrace
Signed-off-by: Changbin Du <changbin.du@intel.com>
---
scripts/faddr2line | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/faddr2line b/scripts/faddr2line
index 7721d5b..9e5735a 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -163,7 +163,17 @@ __faddr2line() {
# pass real address to addr2line
echo "$func+$offset/$sym_size:"
- ${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;"
+ local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
+ [[ -z $file_lines ]] && return
+
+ # show each line with context
+ echo "$file_lines" | while read -r line
+ do
+ echo $line
+ eval $(echo $line | awk -F "[ :]" '{printf("n1=%d;n2=%d;f=%s",$NF-5, $NF+5, $(NF-1))}')
+ awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
+ done
+
DONE=1
done < <(${NM} -n $objfile | awk -v fn=$func -v end=$file_end '$3 == fn { found=1; line=$0; start=$1; next } found == 1 { found=0; print line, "0x"$1 } END {if (found == 1) print line, end; }')
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-03-19 7:23 [PATCH] scripts/faddr2line: show the code context changbin.du
@ 2018-05-29 16:03 ` Peter Zijlstra
2018-05-29 16:26 ` Peter Zijlstra
2018-06-04 2:10 ` Du, Changbin
0 siblings, 2 replies; 9+ messages in thread
From: Peter Zijlstra @ 2018-05-29 16:03 UTC (permalink / raw)
To: changbin.du; +Cc: akpm, tglx, pombredanne, neilb, linux-kernel, jpoimboe
On Mon, Mar 19, 2018 at 03:23:25PM +0800, changbin.du@intel.com wrote:
> From: Changbin Du <changbin.du@intel.com>
>
> Inspired by gdb command 'list', show the code context of target lines.
> Here is a example:
>
> $ scripts/faddr2line vmlinux native_write_msr+0x6
> native_write_msr+0x6/0x20:
> arch_static_branch at arch/x86/include/asm/msr.h:105
> 100 return EAX_EDX_VAL(val, low, high);
> 101 }
> 102
> 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> 104 {
> 105 asm volatile("1: wrmsr\n"
> 106 "2:\n"
> 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> 109 }
> 110
> (inlined by) static_key_false at include/linux/jump_label.h:142
> 137 #define JUMP_TYPE_LINKED 2UL
> 138 #define JUMP_TYPE_MASK 3UL
> 139
> 140 static __always_inline bool static_key_false(struct static_key *key)
> 141 {
> 142 return arch_static_branch(key, false);
> 143 }
> 144
> 145 static __always_inline bool static_key_true(struct static_key *key)
> 146 {
> 147 return !arch_static_branch(key, true);
> (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> 145 static inline void notrace
> 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> 147 {
> 148 __wrmsr(msr, low, high);
> 149
> 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> 152 }
> 153
> 154 /* Can be uninlined because referenced by paravirt */
> 155 static inline int notrace
Not a fan of this :-/ And you didn't even make it optional. Nor did you
Cc the original author of the tool.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-05-29 16:03 ` Peter Zijlstra
@ 2018-05-29 16:26 ` Peter Zijlstra
2018-05-29 17:07 ` Josh Poimboeuf
2018-06-04 2:10 ` Du, Changbin
1 sibling, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2018-05-29 16:26 UTC (permalink / raw)
To: changbin.du; +Cc: akpm, tglx, pombredanne, neilb, linux-kernel, jpoimboe
On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 19, 2018 at 03:23:25PM +0800, changbin.du@intel.com wrote:
> > From: Changbin Du <changbin.du@intel.com>
> >
> > Inspired by gdb command 'list', show the code context of target lines.
> > Here is a example:
> >
> > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > native_write_msr+0x6/0x20:
> > arch_static_branch at arch/x86/include/asm/msr.h:105
> > 100 return EAX_EDX_VAL(val, low, high);
> > 101 }
> > 102
> > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> > 104 {
> > 105 asm volatile("1: wrmsr\n"
> > 106 "2:\n"
> > 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> > 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> > 109 }
> > 110
> > (inlined by) static_key_false at include/linux/jump_label.h:142
> > 137 #define JUMP_TYPE_LINKED 2UL
> > 138 #define JUMP_TYPE_MASK 3UL
> > 139
> > 140 static __always_inline bool static_key_false(struct static_key *key)
> > 141 {
> > 142 return arch_static_branch(key, false);
> > 143 }
> > 144
> > 145 static __always_inline bool static_key_true(struct static_key *key)
> > 146 {
> > 147 return !arch_static_branch(key, true);
> > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > 145 static inline void notrace
> > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > 147 {
> > 148 __wrmsr(msr, low, high);
> > 149
> > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > 152 }
> > 153
> > 154 /* Can be uninlined because referenced by paravirt */
> > 155 static inline int notrace
>
> Not a fan of this :-/ And you didn't even make it optional. Nor did you
> Cc the original author of the tool.
Something like so fixes it up and attempts to make the --list version
more readable.
---
scripts/faddr2line | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/scripts/faddr2line b/scripts/faddr2line
index 1876a741087c..a0149db00be7 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
usage() {
- echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
+ echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
exit 1
}
@@ -166,15 +166,25 @@ __faddr2line() {
local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
[[ -z $file_lines ]] && return
+ if [[ $LIST = 0 ]]; then
+ echo "$file_lines" | while read -r line
+ do
+ echo $line
+ done
+ DONE=1;
+ return
+ fi
+
# show each line with context
echo "$file_lines" | while read -r line
do
+ echo
echo $line
n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
n1=$[$n-5]
n2=$[$n+5]
f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
- awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
+ awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
done
DONE=1
@@ -185,6 +195,10 @@ __faddr2line() {
[[ $# -lt 2 ]] && usage
objfile=$1
+
+LIST=0
+[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
+
[[ ! -f $objfile ]] && die "can't find objfile $objfile"
shift
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-05-29 16:26 ` Peter Zijlstra
@ 2018-05-29 17:07 ` Josh Poimboeuf
2018-05-29 17:24 ` Peter Zijlstra
0 siblings, 1 reply; 9+ messages in thread
From: Josh Poimboeuf @ 2018-05-29 17:07 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: changbin.du, akpm, tglx, pombredanne, neilb, linux-kernel
On Tue, May 29, 2018 at 06:26:36PM +0200, Peter Zijlstra wrote:
> On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> > On Mon, Mar 19, 2018 at 03:23:25PM +0800, changbin.du@intel.com wrote:
> > > From: Changbin Du <changbin.du@intel.com>
> > >
> > > Inspired by gdb command 'list', show the code context of target lines.
> > > Here is a example:
> > >
> > > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > > native_write_msr+0x6/0x20:
> > > arch_static_branch at arch/x86/include/asm/msr.h:105
> > > 100 return EAX_EDX_VAL(val, low, high);
> > > 101 }
> > > 102
> > > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> > > 104 {
> > > 105 asm volatile("1: wrmsr\n"
> > > 106 "2:\n"
> > > 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> > > 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> > > 109 }
> > > 110
> > > (inlined by) static_key_false at include/linux/jump_label.h:142
> > > 137 #define JUMP_TYPE_LINKED 2UL
> > > 138 #define JUMP_TYPE_MASK 3UL
> > > 139
> > > 140 static __always_inline bool static_key_false(struct static_key *key)
> > > 141 {
> > > 142 return arch_static_branch(key, false);
> > > 143 }
> > > 144
> > > 145 static __always_inline bool static_key_true(struct static_key *key)
> > > 146 {
> > > 147 return !arch_static_branch(key, true);
> > > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > > 145 static inline void notrace
> > > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > > 147 {
> > > 148 __wrmsr(msr, low, high);
> > > 149
> > > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > > 152 }
> > > 153
> > > 154 /* Can be uninlined because referenced by paravirt */
> > > 155 static inline int notrace
> >
> > Not a fan of this :-/ And you didn't even make it optional. Nor did you
> > Cc the original author of the tool.
>
> Something like so fixes it up and attempts to make the --list version
> more readable.
>
> ---
> scripts/faddr2line | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 1876a741087c..a0149db00be7 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
>
> usage() {
> - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> exit 1
> }
>
> @@ -166,15 +166,25 @@ __faddr2line() {
> local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> [[ -z $file_lines ]] && return
>
> + if [[ $LIST = 0 ]]; then
> + echo "$file_lines" | while read -r line
> + do
> + echo $line
> + done
> + DONE=1;
> + return
> + fi
> +
> # show each line with context
> echo "$file_lines" | while read -r line
> do
> + echo
> echo $line
> n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> n1=$[$n-5]
> n2=$[$n+5]
> f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> done
>
> DONE=1
> @@ -185,6 +195,10 @@ __faddr2line() {
> [[ $# -lt 2 ]] && usage
>
> objfile=$1
> +
> +LIST=0
> +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> +
> [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> shift
Yeah, this change really should have been an optional arg. It hurt the
readability and compactness of the output. The above looks good to me.
Care to send a proper patch? If you send it to Linus he might apply it
directly as he did with my original patches.
--
Josh
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-05-29 17:07 ` Josh Poimboeuf
@ 2018-05-29 17:24 ` Peter Zijlstra
2018-05-29 17:25 ` Peter Zijlstra
2018-05-29 22:01 ` NeilBrown
0 siblings, 2 replies; 9+ messages in thread
From: Peter Zijlstra @ 2018-05-29 17:24 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: changbin.du, akpm, tglx, pombredanne, neilb, linux-kernel,
Linus Torvalds
On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
> Yeah, this change really should have been an optional arg. It hurt the
> readability and compactness of the output. The above looks good to me.
>
> Care to send a proper patch? If you send it to Linus he might apply it
> directly as he did with my original patches.
---
From: Peter Zijlstra (Intel) <peterz@infraded.org>
Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
radically altered the output format of the faddr2line tool. And while
the new list output format might have merrit it broke my vim usage and
was hard to read.
Make the new format optional; using a '--list' argument and attempt to
make the output slightly easier to read by adding a little whitespace to
separate the different files and explicitly mark the line in question.
Cc: Changbin Du <changbin.du@intel.com>
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
scripts/faddr2line | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/scripts/faddr2line b/scripts/faddr2line
index 1876a741087c..a0149db00be7 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
usage() {
- echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
+ echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
exit 1
}
@@ -166,15 +166,25 @@ __faddr2line() {
local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
[[ -z $file_lines ]] && return
+ if [[ $LIST = 0 ]]; then
+ echo "$file_lines" | while read -r line
+ do
+ echo $line
+ done
+ DONE=1;
+ return
+ fi
+
# show each line with context
echo "$file_lines" | while read -r line
do
+ echo
echo $line
n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
n1=$[$n-5]
n2=$[$n+5]
f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
- awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
+ awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
done
DONE=1
@@ -185,6 +195,10 @@ __faddr2line() {
[[ $# -lt 2 ]] && usage
objfile=$1
+
+LIST=0
+[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
+
[[ ! -f $objfile ]] && die "can't find objfile $objfile"
shift
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-05-29 17:24 ` Peter Zijlstra
@ 2018-05-29 17:25 ` Peter Zijlstra
2018-05-29 22:01 ` NeilBrown
1 sibling, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2018-05-29 17:25 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: changbin.du, akpm, tglx, pombredanne, neilb, linux-kernel,
Linus Torvalds
On Tue, May 29, 2018 at 07:24:30PM +0200, Peter Zijlstra wrote:
> From: Peter Zijlstra (Intel) <peterz@infraded.org>
Shees, you'd figure I could type my own email address by now..
From: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> radically altered the output format of the faddr2line tool. And while
> the new list output format might have merrit it broke my vim usage and
> was hard to read.
>
> Make the new format optional; using a '--list' argument and attempt to
> make the output slightly easier to read by adding a little whitespace to
> separate the different files and explicitly mark the line in question.
>
> Cc: Changbin Du <changbin.du@intel.com>
> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> scripts/faddr2line | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 1876a741087c..a0149db00be7 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
>
> usage() {
> - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> exit 1
> }
>
> @@ -166,15 +166,25 @@ __faddr2line() {
> local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> [[ -z $file_lines ]] && return
>
> + if [[ $LIST = 0 ]]; then
> + echo "$file_lines" | while read -r line
> + do
> + echo $line
> + done
> + DONE=1;
> + return
> + fi
> +
> # show each line with context
> echo "$file_lines" | while read -r line
> do
> + echo
> echo $line
> n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> n1=$[$n-5]
> n2=$[$n+5]
> f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> done
>
> DONE=1
> @@ -185,6 +195,10 @@ __faddr2line() {
> [[ $# -lt 2 ]] && usage
>
> objfile=$1
> +
> +LIST=0
> +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> +
> [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> shift
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-05-29 17:24 ` Peter Zijlstra
2018-05-29 17:25 ` Peter Zijlstra
@ 2018-05-29 22:01 ` NeilBrown
2018-06-04 2:06 ` Du, Changbin
1 sibling, 1 reply; 9+ messages in thread
From: NeilBrown @ 2018-05-29 22:01 UTC (permalink / raw)
To: Peter Zijlstra, Josh Poimboeuf
Cc: changbin.du, akpm, tglx, pombredanne, linux-kernel, Linus Torvalds
[-- Attachment #1: Type: text/plain, Size: 3601 bytes --]
On Tue, May 29 2018, Peter Zijlstra wrote:
> On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
>> Yeah, this change really should have been an optional arg. It hurt the
>> readability and compactness of the output. The above looks good to me.
>>
>> Care to send a proper patch? If you send it to Linus he might apply it
>> directly as he did with my original patches.
>
> ---
> From: Peter Zijlstra (Intel) <peterz@infraded.org>
>
> Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> radically altered the output format of the faddr2line tool. And while
> the new list output format might have merrit it broke my vim usage and
> was hard to read.
>
> Make the new format optional; using a '--list' argument and attempt to
> make the output slightly easier to read by adding a little whitespace to
> separate the different files and explicitly mark the line in question.
Not commenting on your code but on the original patch.....
I've recently noticed that ADDR2LINE sometimes outputs
(discriminator 2)
or similar at the end of the line. This messes up the parsing.
I hacked it to work so I could keep debugging with
- local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
+ local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed -e "s; $dir_prefix\(\./\)*; ;" -e "s/(discriminator [0-9]*)//")
but someone should probably find out exactly what sort of messages
ADDR2LINE produces, and make sure they are all parsed correctly.
(maybe that someone should be me, but not today).
Thanks,
NeilBrown
>
> Cc: Changbin Du <changbin.du@intel.com>
> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> scripts/faddr2line | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 1876a741087c..a0149db00be7 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
>
> usage() {
> - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> exit 1
> }
>
> @@ -166,15 +166,25 @@ __faddr2line() {
> local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> [[ -z $file_lines ]] && return
>
> + if [[ $LIST = 0 ]]; then
> + echo "$file_lines" | while read -r line
> + do
> + echo $line
> + done
> + DONE=1;
> + return
> + fi
> +
> # show each line with context
> echo "$file_lines" | while read -r line
> do
> + echo
> echo $line
> n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> n1=$[$n-5]
> n2=$[$n+5]
> f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> done
>
> DONE=1
> @@ -185,6 +195,10 @@ __faddr2line() {
> [[ $# -lt 2 ]] && usage
>
> objfile=$1
> +
> +LIST=0
> +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> +
> [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> shift
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-05-29 22:01 ` NeilBrown
@ 2018-06-04 2:06 ` Du, Changbin
0 siblings, 0 replies; 9+ messages in thread
From: Du, Changbin @ 2018-06-04 2:06 UTC (permalink / raw)
To: NeilBrown
Cc: Peter Zijlstra, Josh Poimboeuf, changbin.du, akpm, tglx,
pombredanne, linux-kernel, Linus Torvalds
On Wed, May 30, 2018 at 08:01:48AM +1000, NeilBrown wrote:
> On Tue, May 29 2018, Peter Zijlstra wrote:
>
> > On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
> >> Yeah, this change really should have been an optional arg. It hurt the
> >> readability and compactness of the output. The above looks good to me.
> >>
> >> Care to send a proper patch? If you send it to Linus he might apply it
> >> directly as he did with my original patches.
> >
> > ---
> > From: Peter Zijlstra (Intel) <peterz@infraded.org>
> >
> > Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > radically altered the output format of the faddr2line tool. And while
> > the new list output format might have merrit it broke my vim usage and
> > was hard to read.
> >
> > Make the new format optional; using a '--list' argument and attempt to
> > make the output slightly easier to read by adding a little whitespace to
> > separate the different files and explicitly mark the line in question.
>
> Not commenting on your code but on the original patch.....
> I've recently noticed that ADDR2LINE sometimes outputs
> (discriminator 2)
> or similar at the end of the line. This messes up the parsing.
>
> I hacked it to work so I could keep debugging with
>
> - local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> + local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed -e "s; $dir_prefix\(\./\)*; ;" -e "s/(discriminator [0-9]*)//")
>
> but someone should probably find out exactly what sort of messages
> ADDR2LINE produces, and make sure they are all parsed correctly.
> (maybe that someone should be me, but not today).
>
Hi, I have fixed it by commit 78eb0c6356 ("scripts/faddr2line: fix error when
addr2line output contains discriminator") and it is already in the mainline now.
Thank you!
> Thanks,
> NeilBrown
>
>
> >
> > Cc: Changbin Du <changbin.du@intel.com>
> > Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> > scripts/faddr2line | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/faddr2line b/scripts/faddr2line
> > index 1876a741087c..a0149db00be7 100755
> > --- a/scripts/faddr2line
> > +++ b/scripts/faddr2line
> > @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> > command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
> >
> > usage() {
> > - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> > + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> > exit 1
> > }
> >
> > @@ -166,15 +166,25 @@ __faddr2line() {
> > local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> > [[ -z $file_lines ]] && return
> >
> > + if [[ $LIST = 0 ]]; then
> > + echo "$file_lines" | while read -r line
> > + do
> > + echo $line
> > + done
> > + DONE=1;
> > + return
> > + fi
> > +
> > # show each line with context
> > echo "$file_lines" | while read -r line
> > do
> > + echo
> > echo $line
> > n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> > n1=$[$n-5]
> > n2=$[$n+5]
> > f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> > - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> > + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> > done
> >
> > DONE=1
> > @@ -185,6 +195,10 @@ __faddr2line() {
> > [[ $# -lt 2 ]] && usage
> >
> > objfile=$1
> > +
> > +LIST=0
> > +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> > +
> > [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> > shift
> >
--
Thanks,
Changbin Du
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts/faddr2line: show the code context
2018-05-29 16:03 ` Peter Zijlstra
2018-05-29 16:26 ` Peter Zijlstra
@ 2018-06-04 2:10 ` Du, Changbin
1 sibling, 0 replies; 9+ messages in thread
From: Du, Changbin @ 2018-06-04 2:10 UTC (permalink / raw)
To: Peter Zijlstra
Cc: changbin.du, akpm, tglx, pombredanne, neilb, linux-kernel, jpoimboe
On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 19, 2018 at 03:23:25PM +0800, changbin.du@intel.com wrote:
> > From: Changbin Du <changbin.du@intel.com>
> >
> > Inspired by gdb command 'list', show the code context of target lines.
> > Here is a example:
> >
> > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > native_write_msr+0x6/0x20:
> > arch_static_branch at arch/x86/include/asm/msr.h:105
> > 100 return EAX_EDX_VAL(val, low, high);
> > 101 }
> > 102
> > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> > 104 {
> > 105 asm volatile("1: wrmsr\n"
> > 106 "2:\n"
> > 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> > 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> > 109 }
> > 110
> > (inlined by) static_key_false at include/linux/jump_label.h:142
> > 137 #define JUMP_TYPE_LINKED 2UL
> > 138 #define JUMP_TYPE_MASK 3UL
> > 139
> > 140 static __always_inline bool static_key_false(struct static_key *key)
> > 141 {
> > 142 return arch_static_branch(key, false);
> > 143 }
> > 144
> > 145 static __always_inline bool static_key_true(struct static_key *key)
> > 146 {
> > 147 return !arch_static_branch(key, true);
> > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > 145 static inline void notrace
> > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > 147 {
> > 148 __wrmsr(msr, low, high);
> > 149
> > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > 152 }
> > 153
> > 154 /* Can be uninlined because referenced by paravirt */
> > 155 static inline int notrace
>
> Not a fan of this :-/ And you didn't even make it optional. Nor did you
> Cc the original author of the tool.
Yeah, understand your compatibility concern, and thanks for your improvment.
I only added people from 'scripts/get_maintainer.pl'.
--
Thanks,
Changbin Du
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-06-04 2:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-19 7:23 [PATCH] scripts/faddr2line: show the code context changbin.du
2018-05-29 16:03 ` Peter Zijlstra
2018-05-29 16:26 ` Peter Zijlstra
2018-05-29 17:07 ` Josh Poimboeuf
2018-05-29 17:24 ` Peter Zijlstra
2018-05-29 17:25 ` Peter Zijlstra
2018-05-29 22:01 ` NeilBrown
2018-06-04 2:06 ` Du, Changbin
2018-06-04 2:10 ` Du, Changbin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.