[PATCH 0/2] hexdump: minimize the output width of address and offset

[PATCH 0/2] hexdump: minimize the output width of address and offset

[thunder.leizhen]

From: Zhen Lei <thunder.leizhen@huawei.com>

The dump prefix is added to facilitate the reading of the dumped memory.
However, if the prefix content is too repeated or redundant, the readability
is reduced, and the ring buffer becomes full quickly and other prints are
overwritten.

For example: (DUMP_PREFIX_OFFSET)
Before:
dump_size=36:
00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
00000020: 80 ca 2f 98

After:
dump_size=36:
00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
20: 40 9e 29 40


Zhen Lei (2):
  hexdump: minimize the output width of the offset
  hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

 include/linux/printk.h |  1 +
 lib/hexdump.c          | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/2] hexdump: minimize the output width of the offset

[thunder.leizhen]

From: Zhen Lei <thunder.leizhen@huawei.com>

The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
the output width is fixed to 8. But we usually dump only tens or hundreds
of bytes, occasionally thousands of bytes. Therefore, the output offset
value always has a number of leading zeros, which increases the number of
bytes printed and reduces readability. Let's minimize the output width of
the offset based on the number of significant bits of its maximum value.

Before:
dump_size=36:
00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
00000020: 80 ca 2f 98

After:
dump_size=8:
0: c0 ba 89 80 00 80 ff ff

dump_size=36:
00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
20: 40 9e 29 40

dump_size=300:
000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
... ...
110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
120: 00 08 12 01 0c 40 ff ff 00 00 01 00

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 lib/hexdump.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/lib/hexdump.c b/lib/hexdump.c
index 06833d404398d74..86cb4cc3eec485a 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 		    const void *buf, size_t len, bool ascii)
 {
 	const u8 *ptr = buf;
-	int i, linelen, remaining = len;
+	int i, linelen, width = 0, remaining = len;
 	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
 
 	if (rowsize != 16 && rowsize != 32)
 		rowsize = 16;
 
+	if (prefix_type == DUMP_PREFIX_OFFSET) {
+		unsigned long tmp = len - 1;	/* offset start from 0, so minus 1 */
+
+		do {
+			width++;
+			tmp >>= 4;
+		} while (tmp);
+	}
+
 	for (i = 0; i < len; i += rowsize) {
 		linelen = min(remaining, rowsize);
 		remaining -= rowsize;
@@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			       level, prefix_str, ptr + i, linebuf);
 			break;
 		case DUMP_PREFIX_OFFSET:
-			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
+			printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
 			break;
 		default:
 			printk("%s%s%s\n", level, prefix_str, linebuf);
-- 
2.34.1

Re: [PATCH 1/2] hexdump: minimize the output width of the offset

[Randy Dunlap]

Hi--

On 8/5/23 00:21, thunder.leizhen@huaweicloud.com wrote:
> From: Zhen Lei <thunder.leizhen@huawei.com>
> 
> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
> the output width is fixed to 8. But we usually dump only tens or hundreds
> of bytes, occasionally thousands of bytes. Therefore, the output offset
> value always has a number of leading zeros, which increases the number of
> bytes printed and reduces readability. Let's minimize the output width of
> the offset based on the number of significant bits of its maximum value.
> 
> Before:
> dump_size=36:
> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
> 00000020: 80 ca 2f 98
> 
> After:
> dump_size=8:
> 0: c0 ba 89 80 00 80 ff ff
> 
> dump_size=36:
> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
> 20: 40 9e 29 40
> 
> dump_size=300:
> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
> ... ...
> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  lib/hexdump.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 06833d404398d74..86cb4cc3eec485a 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>  		    const void *buf, size_t len, bool ascii)
>  {
>  	const u8 *ptr = buf;
> -	int i, linelen, remaining = len;
> +	int i, linelen, width = 0, remaining = len;
>  	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>  
>  	if (rowsize != 16 && rowsize != 32)
>  		rowsize = 16;
>  
> +	if (prefix_type == DUMP_PREFIX_OFFSET) {
> +		unsigned long tmp = len - 1;	/* offset start from 0, so minus 1 */
> +
> +		do {
> +			width++;
> +			tmp >>= 4;
> +		} while (tmp);
> +	}
> +

You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.
Otherwise LGTM.

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.

>  	for (i = 0; i < len; i += rowsize) {
>  		linelen = min(remaining, rowsize);
>  		remaining -= rowsize;
> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>  			       level, prefix_str, ptr + i, linebuf);
>  			break;
>  		case DUMP_PREFIX_OFFSET:
> -			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
> +			printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>  			break;
>  		default:
>  			printk("%s%s%s\n", level, prefix_str, linebuf);

-- 
~Randy

Re: [PATCH 1/2] hexdump: minimize the output width of the offset

[Leizhen (ThunderTown)]

On 2023/8/8 6:37, Randy Dunlap wrote:
> Hi--
> 
> On 8/5/23 00:21, thunder.leizhen@huaweicloud.com wrote:
>> From: Zhen Lei <thunder.leizhen@huawei.com>
>>
>> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
>> the output width is fixed to 8. But we usually dump only tens or hundreds
>> of bytes, occasionally thousands of bytes. Therefore, the output offset
>> value always has a number of leading zeros, which increases the number of
>> bytes printed and reduces readability. Let's minimize the output width of
>> the offset based on the number of significant bits of its maximum value.
>>
>> Before:
>> dump_size=36:
>> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
>> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
>> 00000020: 80 ca 2f 98
>>
>> After:
>> dump_size=8:
>> 0: c0 ba 89 80 00 80 ff ff
>>
>> dump_size=36:
>> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
>> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
>> 20: 40 9e 29 40
>>
>> dump_size=300:
>> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
>> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
>> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
>> ... ...
>> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  lib/hexdump.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>> index 06833d404398d74..86cb4cc3eec485a 100644
>> --- a/lib/hexdump.c
>> +++ b/lib/hexdump.c
>> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>  		    const void *buf, size_t len, bool ascii)
>>  {
>>  	const u8 *ptr = buf;
>> -	int i, linelen, remaining = len;
>> +	int i, linelen, width = 0, remaining = len;
>>  	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>>  
>>  	if (rowsize != 16 && rowsize != 32)
>>  		rowsize = 16;
>>  
>> +	if (prefix_type == DUMP_PREFIX_OFFSET) {
>> +		unsigned long tmp = len - 1;	/* offset start from 0, so minus 1 */
>> +
>> +		do {
>> +			width++;
>> +			tmp >>= 4;
>> +		} while (tmp);
>> +	}
>> +
> 
> You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.

for (i = 0; i < len; i += rowsize) {

"case DUMP_PREFIX_OFFSET" is in the loop, and moving the code above
to the case DUMP_PREFIX_OFFSET will be calculate multiple times. But
following your prompt, I thought again, I can control it with the
local variable width. I will post v2 right away.

> Otherwise LGTM.
> 
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> Thanks.
> 
>>  	for (i = 0; i < len; i += rowsize) {
>>  		linelen = min(remaining, rowsize);
>>  		remaining -= rowsize;
>> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>  			       level, prefix_str, ptr + i, linebuf);
>>  			break;
>>  		case DUMP_PREFIX_OFFSET:
>> -			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
>> +			printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>>  			break;
>>  		default:
>>  			printk("%s%s%s\n", level, prefix_str, linebuf);
> 

-- 
Regards,
  Zhen Lei

Re: [PATCH 1/2] hexdump: minimize the output width of the offset

[Randy Dunlap]

On 8/7/23 18:10, Leizhen (ThunderTown) wrote:
> 
> 
> On 2023/8/8 6:37, Randy Dunlap wrote:
>> Hi--
>>
>> On 8/5/23 00:21, thunder.leizhen@huaweicloud.com wrote:
>>> From: Zhen Lei <thunder.leizhen@huawei.com>
>>>
>>> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
>>> the output width is fixed to 8. But we usually dump only tens or hundreds
>>> of bytes, occasionally thousands of bytes. Therefore, the output offset
>>> value always has a number of leading zeros, which increases the number of
>>> bytes printed and reduces readability. Let's minimize the output width of
>>> the offset based on the number of significant bits of its maximum value.
>>>
>>> Before:
>>> dump_size=36:
>>> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
>>> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
>>> 00000020: 80 ca 2f 98
>>>
>>> After:
>>> dump_size=8:
>>> 0: c0 ba 89 80 00 80 ff ff
>>>
>>> dump_size=36:
>>> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
>>> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
>>> 20: 40 9e 29 40
>>>
>>> dump_size=300:
>>> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
>>> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
>>> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
>>> ... ...
>>> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
>>>
>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>> ---
>>>  lib/hexdump.c | 13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>>> index 06833d404398d74..86cb4cc3eec485a 100644
>>> --- a/lib/hexdump.c
>>> +++ b/lib/hexdump.c
>>> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>>  		    const void *buf, size_t len, bool ascii)
>>>  {
>>>  	const u8 *ptr = buf;
>>> -	int i, linelen, remaining = len;
>>> +	int i, linelen, width = 0, remaining = len;
>>>  	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>>>  
>>>  	if (rowsize != 16 && rowsize != 32)
>>>  		rowsize = 16;
>>>  
>>> +	if (prefix_type == DUMP_PREFIX_OFFSET) {
>>> +		unsigned long tmp = len - 1;	/* offset start from 0, so minus 1 */
>>> +
>>> +		do {
>>> +			width++;
>>> +			tmp >>= 4;
>>> +		} while (tmp);
>>> +	}
>>> +
>>
>> You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.
> 
> for (i = 0; i < len; i += rowsize) {
> 
> "case DUMP_PREFIX_OFFSET" is in the loop, and moving the code above
> to the case DUMP_PREFIX_OFFSET will be calculate multiple times. But
> following your prompt, I thought again, I can control it with the
> local variable width. I will post v2 right away.

Ah I see. My apologies.

>> Otherwise LGTM.
>>
>> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
>> Thanks.
>>
>>>  	for (i = 0; i < len; i += rowsize) {
>>>  		linelen = min(remaining, rowsize);
>>>  		remaining -= rowsize;
>>> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>>  			       level, prefix_str, ptr + i, linebuf);
>>>  			break;
>>>  		case DUMP_PREFIX_OFFSET:
>>> -			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
>>> +			printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>>>  			break;
>>>  		default:
>>>  			printk("%s%s%s\n", level, prefix_str, linebuf);
>>
> 

-- 
~Randy

Re: [PATCH 1/2] hexdump: minimize the output width of the offset

[Leizhen (ThunderTown)]

On 2023/8/8 10:42, Randy Dunlap wrote:
> 
> 
> On 8/7/23 18:10, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2023/8/8 6:37, Randy Dunlap wrote:
>>> Hi--
>>>
>>> On 8/5/23 00:21, thunder.leizhen@huaweicloud.com wrote:
>>>> From: Zhen Lei <thunder.leizhen@huawei.com>
>>>>
>>>> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
>>>> the output width is fixed to 8. But we usually dump only tens or hundreds
>>>> of bytes, occasionally thousands of bytes. Therefore, the output offset
>>>> value always has a number of leading zeros, which increases the number of
>>>> bytes printed and reduces readability. Let's minimize the output width of
>>>> the offset based on the number of significant bits of its maximum value.
>>>>
>>>> Before:
>>>> dump_size=36:
>>>> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
>>>> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
>>>> 00000020: 80 ca 2f 98
>>>>
>>>> After:
>>>> dump_size=8:
>>>> 0: c0 ba 89 80 00 80 ff ff
>>>>
>>>> dump_size=36:
>>>> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
>>>> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
>>>> 20: 40 9e 29 40
>>>>
>>>> dump_size=300:
>>>> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
>>>> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
>>>> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
>>>> ... ...
>>>> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
>>>>
>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>> ---
>>>>  lib/hexdump.c | 13 +++++++++++--
>>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>>>> index 06833d404398d74..86cb4cc3eec485a 100644
>>>> --- a/lib/hexdump.c
>>>> +++ b/lib/hexdump.c
>>>> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>>>  		    const void *buf, size_t len, bool ascii)
>>>>  {
>>>>  	const u8 *ptr = buf;
>>>> -	int i, linelen, remaining = len;
>>>> +	int i, linelen, width = 0, remaining = len;
>>>>  	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>>>>  
>>>>  	if (rowsize != 16 && rowsize != 32)
>>>>  		rowsize = 16;
>>>>  
>>>> +	if (prefix_type == DUMP_PREFIX_OFFSET) {
>>>> +		unsigned long tmp = len - 1;	/* offset start from 0, so minus 1 */
>>>> +
>>>> +		do {
>>>> +			width++;
>>>> +			tmp >>= 4;
>>>> +		} while (tmp);
>>>> +	}
>>>> +
>>>
>>> You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.
>>
>> for (i = 0; i < len; i += rowsize) {
>>
>> "case DUMP_PREFIX_OFFSET" is in the loop, and moving the code above
>> to the case DUMP_PREFIX_OFFSET will be calculate multiple times. But
>> following your prompt, I thought again, I can control it with the
>> local variable width. I will post v2 right away.
> 
> Ah I see. My apologies.

It should be okay to move it into the case DUMP_PREFIX_OFFSET. The
compiler can optimize it.

> 
>>> Otherwise LGTM.
>>>
>>> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
>>> Thanks.
>>>
>>>>  	for (i = 0; i < len; i += rowsize) {
>>>>  		linelen = min(remaining, rowsize);
>>>>  		remaining -= rowsize;
>>>> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>>>  			       level, prefix_str, ptr + i, linebuf);
>>>>  			break;
>>>>  		case DUMP_PREFIX_OFFSET:
>>>> -			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
>>>> +			printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>>>>  			break;
>>>>  		default:
>>>>  			printk("%s%s%s\n", level, prefix_str, linebuf);
>>>
>>
> 

-- 
Regards,
  Zhen Lei

[PATCH 2/2] hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

[thunder.leizhen]

From: Zhen Lei <thunder.leizhen@huawei.com>

Currently, function print_hex_dump() supports three dump prefixes:
DUMP_PREFIX_NONE, DUMP_PREFIX_ADDRESS and DUMP_PREFIX_OFFSET. But for some
usage scenarios, they don't work perfectly. For example, dump the content
of one task's stack. In order to quickly identify a stack frame,
DUMP_PREFIX_ADDRESS is preferred. But printing multiple 64-bit addresses
is a bit unwise when the 'sp' value is already printed. It is redundant
and unintuitive.

For example:
dump memory at sp=ffff800080883a90:
ffff800080883a90: 80883ac0 ffff8000 3d8e936c ffffbd5b
ffff800080883aa0: 5833f000 ffff3580 00000001 00000000
ffff800080883ab0: 40299840 ffff3580 590dfa00 ffff3580
ffff800080883ac0: 80883b30 ffff8000 3d938b28 ffffbd5b
ffff800080883ad0: 40877180 ffff3580 590dfa00 ffff3580
ffff800080883ae0: 4090f600 ffff3580 80883cb0 ffff8000
ffff800080883af0: 00000010 00000000 00000000 00000000
ffff800080883b00: 4090f700 ffff3580 00000001 00000000

Generally, we do not dump more than 64 KB memory. It is sufficient to
print only the lower 16 bits of the address.

dump memory at sp=ffff800080883a90:
3a90: 80883ac0 ffff8000 3d8e936c ffffbd5b
3aa0: 5833f000 ffff3580 00000001 00000000
3ab0: 40299840 ffff3580 590dfa00 ffff3580
3ac0: 80883b30 ffff8000 3d938b28 ffffbd5b
3ad0: 40877180 ffff3580 590dfa00 ffff3580
3ae0: 4090f600 ffff3580 80883cb0 ffff8000
3af0: 00000010 00000000 00000000 00000000
3b00: 4090f700 ffff3580 00000001 00000000

Another benefit of adding DUMP_PREFIX_ADDRESS_LOW16 is that we don't have
to worry about %p outputting address as hashed value.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 include/linux/printk.h | 1 +
 lib/hexdump.c          | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 8ef499ab3c1ed2e..ccad9e8eaaf0c31 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -704,6 +704,7 @@ extern const struct file_operations kmsg_fops;
 enum {
 	DUMP_PREFIX_NONE,
 	DUMP_PREFIX_ADDRESS,
+	DUMP_PREFIX_ADDRESS_LOW16,
 	DUMP_PREFIX_OFFSET
 };
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 86cb4cc3eec485a..eb33e477bc96df1 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -290,6 +290,10 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			printk("%s%s%p: %s\n",
 			       level, prefix_str, ptr + i, linebuf);
 			break;
+		case DUMP_PREFIX_ADDRESS_LOW16:
+			printk("%s%s%04lx: %s\n", level,
+			       prefix_str, 0xffff & (unsigned long)(ptr + i), linebuf);
+			break;
 		case DUMP_PREFIX_OFFSET:
 			printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
 			break;
-- 
2.34.1

Re: [PATCH 2/2] hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

[Randy Dunlap]

On 8/5/23 00:21, thunder.leizhen@huaweicloud.com wrote:
> From: Zhen Lei <thunder.leizhen@huawei.com>
> 
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>


Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.

> ---
>  include/linux/printk.h | 1 +
>  lib/hexdump.c          | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 8ef499ab3c1ed2e..ccad9e8eaaf0c31 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -704,6 +704,7 @@ extern const struct file_operations kmsg_fops;
>  enum {
>  	DUMP_PREFIX_NONE,
>  	DUMP_PREFIX_ADDRESS,
> +	DUMP_PREFIX_ADDRESS_LOW16,
>  	DUMP_PREFIX_OFFSET
>  };
>  extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 86cb4cc3eec485a..eb33e477bc96df1 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -290,6 +290,10 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>  			printk("%s%s%p: %s\n",
>  			       level, prefix_str, ptr + i, linebuf);
>  			break;
> +		case DUMP_PREFIX_ADDRESS_LOW16:
> +			printk("%s%s%04lx: %s\n", level,
> +			       prefix_str, 0xffff & (unsigned long)(ptr + i), linebuf);
> +			break;
>  		case DUMP_PREFIX_OFFSET:
>  			printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>  			break;

-- 
~Randy

Re: [PATCH 2/2] hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

[Steven Rostedt]

On Sat,  5 Aug 2023 15:21:16 +0800
thunder.leizhen@huaweicloud.com wrote:

> For example:
> dump memory at sp=ffff800080883a90:
> ffff800080883a90: 80883ac0 ffff8000 3d8e936c ffffbd5b
> ffff800080883aa0: 5833f000 ffff3580 00000001 00000000
> ffff800080883ab0: 40299840 ffff3580 590dfa00 ffff3580
> ffff800080883ac0: 80883b30 ffff8000 3d938b28 ffffbd5b
> ffff800080883ad0: 40877180 ffff3580 590dfa00 ffff3580
> ffff800080883ae0: 4090f600 ffff3580 80883cb0 ffff8000
> ffff800080883af0: 00000010 00000000 00000000 00000000
> ffff800080883b00: 4090f700 ffff3580 00000001 00000000
> 
> Generally, we do not dump more than 64 KB memory. It is sufficient to
> print only the lower 16 bits of the address.
> 
> dump memory at sp=ffff800080883a90:
> 3a90: 80883ac0 ffff8000 3d8e936c ffffbd5b
> 3aa0: 5833f000 ffff3580 00000001 00000000
> 3ab0: 40299840 ffff3580 590dfa00 ffff3580
> 3ac0: 80883b30 ffff8000 3d938b28 ffffbd5b
> 3ad0: 40877180 ffff3580 590dfa00 ffff3580
> 3ae0: 4090f600 ffff3580 80883cb0 ffff8000
> 3af0: 00000010 00000000 00000000 00000000
> 3b00: 4090f700 ffff3580 00000001 00000000

I find the "before" much easier to read than the "after".


-- Steve

Re: [PATCH 2/2] hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

[Leizhen (ThunderTown)]

On 2023/8/8 9:14, Steven Rostedt wrote:
> On Sat,  5 Aug 2023 15:21:16 +0800
> thunder.leizhen@huaweicloud.com wrote:
> 
>> For example:
>> dump memory at sp=ffff800080883a90:
>> ffff800080883a90: 80883ac0 ffff8000 3d8e936c ffffbd5b
>> ffff800080883aa0: 5833f000 ffff3580 00000001 00000000
>> ffff800080883ab0: 40299840 ffff3580 590dfa00 ffff3580
>> ffff800080883ac0: 80883b30 ffff8000 3d938b28 ffffbd5b
>> ffff800080883ad0: 40877180 ffff3580 590dfa00 ffff3580
>> ffff800080883ae0: 4090f600 ffff3580 80883cb0 ffff8000
>> ffff800080883af0: 00000010 00000000 00000000 00000000
>> ffff800080883b00: 4090f700 ffff3580 00000001 00000000
>>
>> Generally, we do not dump more than 64 KB memory. It is sufficient to
>> print only the lower 16 bits of the address.
>>
>> dump memory at sp=ffff800080883a90:
>> 3a90: 80883ac0 ffff8000 3d8e936c ffffbd5b
>> 3aa0: 5833f000 ffff3580 00000001 00000000
>> 3ab0: 40299840 ffff3580 590dfa00 ffff3580
>> 3ac0: 80883b30 ffff8000 3d938b28 ffffbd5b
>> 3ad0: 40877180 ffff3580 590dfa00 ffff3580
>> 3ae0: 4090f600 ffff3580 80883cb0 ffff8000
>> 3af0: 00000010 00000000 00000000 00000000
>> 3b00: 4090f700 ffff3580 00000001 00000000
> 
> I find the "before" much easier to read than the "after".

That's because you're experienced and know how to look around colons
for offsets. But I can remove "and unintuitive" from the commit message.

> 
> 
> -- Steve
> 
> .
> 

-- 
Regards,
  Zhen Lei

Re: [PATCH 2/2] hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

[Sergey Senozhatsky]

On (23/08/08 15:52), Leizhen (ThunderTown) wrote:
> >> I find the "before" much easier to read than the "after".
> 
> I added the boot option no_hash_pointers, so "before" can print the
> address. Otherwise, just print the 32-bit hash value, as shown below:


> [   14.872153] dump memory at sp=ffff800080903aa0:

This line is not guaranteed to be printed. If you get
"** N printk messages dropped **" instead then the
ADDRESS_LOW16 math doesn't work.

> Actually, I added DUMP_PREFIX_ADDRESS_LOW16 because of another scene:
> slab kmalloc-512 start ffff3b3c0094e800 pointer offset 168 size 512
> e888: 00400000 00000000 000f7704 00000000
> e898: 000f7704 00000000 12345678 00000000
> e8a8: 00000000 00000000 00000000 00000000
> e8b8: 9abcdef0 00000000 00507dd0 00000000
> 
> Here, the start address ffff3b3c0094e800 of slab object is printed by %px,
> the address of the error data is at p=ffff3b3c0094e8a8 = ffff3b3c0094e800 + offset 168.
> To locate the problem, dump up to 64 bytes centered on 'p'.

Re: [PATCH 2/2] hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

[Leizhen (ThunderTown)]

On 2023/8/9 12:10, Sergey Senozhatsky wrote:
> On (23/08/08 15:52), Leizhen (ThunderTown) wrote:
>>>> I find the "before" much easier to read than the "after".
>>
>> I added the boot option no_hash_pointers, so "before" can print the
>> address. Otherwise, just print the 32-bit hash value, as shown below:
> 
> 
>> [   14.872153] dump memory at sp=ffff800080903aa0:
> 
> This line is not guaranteed to be printed. If you get
> "** N printk messages dropped **" instead then the
> ADDRESS_LOW16 math doesn't work.

Anyone is vulnerable in the face of nature. Even DUMP_PREFIX_ADDRESS,
without the preface, no one know what's dumped. In any case,
DUMP_PREFIX_ADDRESS_LOW16 has its own unique value in this ecosystem.
The only regret is that it has a slightly longer name than others.
Maybe it could be DUMP_PREFIX_ADDRLOW or something.

By the way, would you consider changing %p to %px in case DUMP_PREFIX_ADDRESS?

> 
>> Actually, I added DUMP_PREFIX_ADDRESS_LOW16 because of another scene:
>> slab kmalloc-512 start ffff3b3c0094e800 pointer offset 168 size 512
>> e888: 00400000 00000000 000f7704 00000000
>> e898: 000f7704 00000000 12345678 00000000
>> e8a8: 00000000 00000000 00000000 00000000
>> e8b8: 9abcdef0 00000000 00507dd0 00000000
>>
>> Here, the start address ffff3b3c0094e800 of slab object is printed by %px,
>> the address of the error data is at p=ffff3b3c0094e8a8 = ffff3b3c0094e800 + offset 168.
>> To locate the problem, dump up to 64 bytes centered on 'p'.
> .
> 

-- 
Regards,
  Zhen Lei

Re: [PATCH 2/2] hexdump: add a new dump prefix DUMP_PREFIX_ADDRESS_LOW16

[Leizhen (ThunderTown)]

On 2023/8/9 15:05, Leizhen (ThunderTown) wrote:
> 
> 
> On 2023/8/9 12:10, Sergey Senozhatsky wrote:
>> On (23/08/08 15:52), Leizhen (ThunderTown) wrote:
>>>>> I find the "before" much easier to read than the "after".
>>>
>>> I added the boot option no_hash_pointers, so "before" can print the
>>> address. Otherwise, just print the 32-bit hash value, as shown below:
>>
>>
>>> [   14.872153] dump memory at sp=ffff800080903aa0:
>>
>> This line is not guaranteed to be printed. If you get
>> "** N printk messages dropped **" instead then the
>> ADDRESS_LOW16 math doesn't work.

I have a new idea. Replace DUMP_PREFIX_ADDRESS_LOW16 with DUMP_PREFIX_CUSTOM.
Let the user to specify the format string.

For example:
pr_info("dump memory at sp=%px:\n", sp);
print_hex_dump(KERN_INFO, "%s%16hx: %s\n", DUMP_PREFIX_CUSTOM, 16, 1, sp, 16, false);
print_hex_dump(KERN_INFO, "%s%16x: %s\n",  DUMP_PREFIX_CUSTOM, 16, 1, sp, 16, false);
print_hex_dump(KERN_INFO, "%s%px: %s\n",   DUMP_PREFIX_CUSTOM, 16, 1, sp, 16, false);

dump memory at sp=ffff80008091baa0:
            baa0: c0 ba 91 80 00 80 ff ff d4 38 16 ce fc a7 ff ff
        8091baa0: c0 ba 91 80 00 80 ff ff d4 38 16 ce fc a7 ff ff
ffff80008091baa0: c0 ba 91 80 00 80 ff ff d4 38 16 ce fc a7 ff ff

> 
> Anyone is vulnerable in the face of nature. Even DUMP_PREFIX_ADDRESS,
> without the preface, no one know what's dumped. In any case,
> DUMP_PREFIX_ADDRESS_LOW16 has its own unique value in this ecosystem.
> The only regret is that it has a slightly longer name than others.
> Maybe it could be DUMP_PREFIX_ADDRLOW or something.
> 
> By the way, would you consider changing %p to %px in case DUMP_PREFIX_ADDRESS?
> 
>>
>>> Actually, I added DUMP_PREFIX_ADDRESS_LOW16 because of another scene:
>>> slab kmalloc-512 start ffff3b3c0094e800 pointer offset 168 size 512
>>> e888: 00400000 00000000 000f7704 00000000
>>> e898: 000f7704 00000000 12345678 00000000
>>> e8a8: 00000000 00000000 00000000 00000000
>>> e8b8: 9abcdef0 00000000 00507dd0 00000000
>>>
>>> Here, the start address ffff3b3c0094e800 of slab object is printed by %px,
>>> the address of the error data is at p=ffff3b3c0094e8a8 = ffff3b3c0094e800 + offset 168.
>>> To locate the problem, dump up to 64 bytes centered on 'p'.
>> .
>>
> 

-- 
Regards,
  Zhen Lei