Re: [PATCH v2] scripts/gdb: fix lx-dmesg when CONFIG_PRINTK_CALLER is set
On 11.10.2019 16:02, Joel Colledge wrote: > On Fri, Oct 11, 2019 at 2:47 PM Leonard Crestez > wrote: >> This struct printk_log is quite small, I wonder if it's possible to do a >> single read for each log entry? This might make lx-dmesg faster because >> of fewer roundtrips to gdbserver and jtag (or whatever backend you're >> using). > > I think this is already covered. utils.read_memoryview uses > inferior.read_memory and I think that reads the entire log buffer at > once (at most 2 reads, one for each half). You're right, sorry
Re: [PATCH v2] scripts/gdb: fix lx-dmesg when CONFIG_PRINTK_CALLER is set
On Fri, Oct 11, 2019 at 2:47 PM Leonard Crestez wrote: > This struct printk_log is quite small, I wonder if it's possible to do a > single read for each log entry? This might make lx-dmesg faster because > of fewer roundtrips to gdbserver and jtag (or whatever backend you're > using). I think this is already covered. utils.read_memoryview uses inferior.read_memory and I think that reads the entire log buffer at once (at most 2 reads, one for each half).
Re: [PATCH v2] scripts/gdb: fix lx-dmesg when CONFIG_PRINTK_CALLER is set
On Fri, Oct 11, 2019 at 2:38 PM Jan Kiszka wrote: > Does bitpos really use a non-int type? Otherwise, plain '/' suffices. bitpos uses gdb.Field. When I use '/' I get an error: Error occurred in Python command: slice indices must be integers or None or have an __index__ method I'm guessing gdb.Field has some kind of override which causes it to be converted to float when using '/'; hence '//'. > Overlong line. > ... > Here as well. Better use some temp vars to break up the expressions. > Helps with readability. Will do.
Re: [PATCH v2] scripts/gdb: fix lx-dmesg when CONFIG_PRINTK_CALLER is set
On 11.10.2019 15:25, Joel Colledge wrote:
> When CONFIG_PRINTK_CALLER is set, struct printk_log contains an
> additional member caller_id. This affects the offset of the log text.
> Account for this by using the type information from gdb to determine all
> the offsets instead of using hardcoded values.
>
> This fixes following error:
>
>(gdb) lx-dmesg
>Python Exception embedded null character:
>Error occurred in Python command: embedded null character
>
> Signed-off-by: Joel Colledge
> ---
> Changes in v2:
> - use type information from gdb instead of hardcoded offsets
>
> Thanks for the idea about using the struct layout info from gdb, Leonard. I
> can't see any reason we shouldn't use that here, since most of the other
> commands use it. LxDmesg has used hardcoded offsets since scripts/gdb was
> introduced, so I assume it just ended up like that during the initial
> development of the tool. Here is a version of the fix using offsets from gdb.
This struct printk_log is quite small, I wonder if it's possible to do a
single read for each log entry? This might make lx-dmesg faster because
of fewer roundtrips to gdbserver and jtag (or whatever backend you're
using).
>
> scripts/gdb/linux/dmesg.py | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
> index 6d2e09a2ad2f..8f5d899029b7 100644
> --- a/scripts/gdb/linux/dmesg.py
> +++ b/scripts/gdb/linux/dmesg.py
> @@ -16,6 +16,8 @@ import sys
>
> from linux import utils
>
> +printk_log_type = utils.CachedType("struct printk_log")
> +
>
> class LxDmesg(gdb.Command):
> """Print Linux kernel log buffer."""
> @@ -42,9 +44,14 @@ class LxDmesg(gdb.Command):
> b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
> log_buf = a.tobytes() + b.tobytes()
>
> +length_offset = printk_log_type.get_type()['len'].bitpos // 8
> +text_len_offset = printk_log_type.get_type()['text_len'].bitpos // 8
> +time_stamp_offset = printk_log_type.get_type()['ts_nsec'].bitpos // 8
> +text_offset = printk_log_type.get_type().sizeof
> +
> pos = 0
> while pos < log_buf.__len__():
> -length = utils.read_u16(log_buf[pos + 8:pos + 10])
> +length = utils.read_u16(log_buf[pos + length_offset:pos +
> length_offset + 2])
> if length == 0:
> if log_buf_2nd_half == -1:
> gdb.write("Corrupted log buffer!\n")
> @@ -52,10 +59,11 @@ class LxDmesg(gdb.Command):
> pos = log_buf_2nd_half
> continue
>
> -text_len = utils.read_u16(log_buf[pos + 10:pos + 12])
> -text = log_buf[pos + 16:pos + 16 + text_len].decode(
> +text_len = utils.read_u16(log_buf[pos + text_len_offset:pos +
> text_len_offset + 2])
> +text = log_buf[pos + text_offset:pos + text_offset +
> text_len].decode(
> encoding='utf8', errors='replace')
> -time_stamp = utils.read_u64(log_buf[pos:pos + 8])
> +time_stamp = utils.read_u64(
> +log_buf[pos + time_stamp_offset:pos + time_stamp_offset + 8])
>
> for line in text.splitlines():
> msg = u"[{time:12.6f}] {line}\n".format(
>
Re: [PATCH v2] scripts/gdb: fix lx-dmesg when CONFIG_PRINTK_CALLER is set
On 11.10.19 14:24, Joel Colledge wrote:
When CONFIG_PRINTK_CALLER is set, struct printk_log contains an
additional member caller_id. This affects the offset of the log text.
Account for this by using the type information from gdb to determine all
the offsets instead of using hardcoded values.
This fixes following error:
(gdb) lx-dmesg
Python Exception embedded null character:
Error occurred in Python command: embedded null character
Signed-off-by: Joel Colledge
---
Changes in v2:
- use type information from gdb instead of hardcoded offsets
Thanks for the idea about using the struct layout info from gdb, Leonard. I
can't see any reason we shouldn't use that here, since most of the other
commands use it. LxDmesg has used hardcoded offsets since scripts/gdb was
introduced, so I assume it just ended up like that during the initial
development of the tool. Here is a version of the fix using offsets from gdb.
That's not unlikely, indeed. lx-dmesg was one of the very first features
I've implemented back then, and it definitely predated things like
CachedType.
scripts/gdb/linux/dmesg.py | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
index 6d2e09a2ad2f..8f5d899029b7 100644
--- a/scripts/gdb/linux/dmesg.py
+++ b/scripts/gdb/linux/dmesg.py
@@ -16,6 +16,8 @@ import sys
from linux import utils
+printk_log_type = utils.CachedType("struct printk_log")
+
class LxDmesg(gdb.Command):
"""Print Linux kernel log buffer."""
@@ -42,9 +44,14 @@ class LxDmesg(gdb.Command):
b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
log_buf = a.tobytes() + b.tobytes()
+length_offset = printk_log_type.get_type()['len'].bitpos // 8
Does bitpos really use a non-int type? Otherwise, plain '/' suffices.
+text_len_offset = printk_log_type.get_type()['text_len'].bitpos // 8
+time_stamp_offset = printk_log_type.get_type()['ts_nsec'].bitpos // 8
+text_offset = printk_log_type.get_type().sizeof
+
pos = 0
while pos < log_buf.__len__():
-length = utils.read_u16(log_buf[pos + 8:pos + 10])
+length = utils.read_u16(log_buf[pos + length_offset:pos +
length_offset + 2])
Overlong line.
if length == 0:
if log_buf_2nd_half == -1:
gdb.write("Corrupted log buffer!\n")
@@ -52,10 +59,11 @@ class LxDmesg(gdb.Command):
pos = log_buf_2nd_half
continue
-text_len = utils.read_u16(log_buf[pos + 10:pos + 12])
-text = log_buf[pos + 16:pos + 16 + text_len].decode(
+text_len = utils.read_u16(log_buf[pos + text_len_offset:pos +
text_len_offset + 2])
Here as well. Better use some temp vars to break up the expressions.
Helps with readability.
+text = log_buf[pos + text_offset:pos + text_offset +
text_len].decode(
encoding='utf8', errors='replace')
-time_stamp = utils.read_u64(log_buf[pos:pos + 8])
+time_stamp = utils.read_u64(
+log_buf[pos + time_stamp_offset:pos + time_stamp_offset + 8])
for line in text.splitlines():
msg = u"[{time:12.6f}] {line}\n".format(
Looks good otherwise.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
