Messages in this thread Patch in this message |  | | Subject | Re: [PATCH 1/5] x86/alternative: simplify DUMP_BYTES macro | From | Joe Perches <> | Date | Sat, 12 Mar 2022 08:36:11 -0800 |
| |
On Fri, 2022-03-11 at 17:43 +0300, Alexey Dobriyan wrote: > Avoid zero length check with clever whitespace placement in the format > string. [] > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c [] > @@ -66,13 +66,10 @@ do { \ > if (unlikely(debug_alternative)) { \ > int j; \ > \ > - if (!(len)) \ > - break; \ > - \ > printk(KERN_DEBUG pr_fmt(fmt), ##args); \ > - for (j = 0; j < (len) - 1; j++) \ > - printk(KERN_CONT "%02hhx ", buf[j]); \ > - printk(KERN_CONT "%02hhx\n", buf[j]); \ > + for (j = 0; j < (len); j++) \ > + printk(KERN_CONT " %02hhx", buf[j]); \ > + printk(KERN_CONT "\n"); \ > } \
This could also use %02x and not %02hhx
And MAX_PATCH_LEN is 255 but is that really possible?
Maybe if the actual patch length is always <= 64 this could use printk(KERN_CONT "%*ph\n", (int)len, buf); instead and avoid all possible interleaving?
If so, maybe just remove DUMP_BYTES and use DPRINTK directly.
Perhaps: --- arch/x86/kernel/alternative.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 018b61febf0e7..74fa946093467 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -61,21 +61,6 @@ do { \ printk(KERN_DEBUG pr_fmt(fmt) "\n", ##args); \ } while (0) -#define DUMP_BYTES(buf, len, fmt, args...) \ -do { \ - if (unlikely(debug_alternative)) { \ - int j; \ - \ - if (!(len)) \ - break; \ - \ - printk(KERN_DEBUG pr_fmt(fmt), ##args); \ - for (j = 0; j < (len) - 1; j++) \ - printk(KERN_CONT "%02hhx ", buf[j]); \ - printk(KERN_CONT "%02hhx\n", buf[j]); \ - } \ -} while (0) - static const unsigned char x86nops[] = { BYTES_NOP1, @@ -214,7 +199,8 @@ static __always_inline int optimize_nops_range(u8 *instr, u8 instrlen, int off) add_nops(instr + off, nnops); local_irq_restore(flags); - DUMP_BYTES(instr, instrlen, "%px: [%d:%d) optimized NOPs: ", instr, off, i); + DPRINTK("%px: [%d:%d) optimized NOPs: %*ph", + instr, off, i, (int)instrlen, instr); return nnops; } @@ -303,8 +289,10 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, instr, instr, a->instrlen, replacement, a->replacementlen); - DUMP_BYTES(instr, a->instrlen, "%px: old_insn: ", instr); - DUMP_BYTES(replacement, a->replacementlen, "%px: rpl_insn: ", replacement); + DPRINTK("%px: old_insn: %*ph", + instr, (int)a->instrlen, instr); + DPRINTK("%px: rpl_insn: %*ph", + replacement, (int)a->replacementlen, replacement); memcpy(insn_buff, replacement, a->replacementlen); insn_buff_sz = a->replacementlen; @@ -328,7 +316,8 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, for (; insn_buff_sz < a->instrlen; insn_buff_sz++) insn_buff[insn_buff_sz] = 0x90; - DUMP_BYTES(insn_buff, insn_buff_sz, "%px: final_insn: ", instr); + DPRINTK("%px: final_insn: %*ph", + instr, (int)insn_buff_sz, insn_buff); text_poke_early(instr, insn_buff, insn_buff_sz); @@ -499,8 +488,8 @@ void __init_or_module noinline apply_retpolines(s32 *start, s32 *end) len = patch_retpoline(addr, &insn, bytes); if (len == insn.length) { optimize_nops(bytes, len); - DUMP_BYTES(((u8*)addr), len, "%px: orig: ", addr); - DUMP_BYTES(((u8*)bytes), len, "%px: repl: ", addr); + DPRINTK("%px: orig: %*ph", addr, (int)len, addr); + DPRINTK("%px: repl: %*ph", addr, (int)len, bytes); text_poke_early(addr, bytes, len); } }
|  |