On Sat, 22 Mar 2025, WangYuli wrote:
> The nop instruction surrounding "breakinst:\tbreak\n\t" appears to
> serve no real purpose.
"If it ain't broke, don't fix it" -- what problem are you trying to
solve here? Saving 8 bytes of kernel text space?
> Its introduction can be traced back to commit 51c6022fdb ("[PATCH]
> MIPS update") within the Linux history tree [1]. This commit was
> substantial, comprising 41010 lines, and provides no justification
> for the insertion of this nop instruction.
The actual origin is:
commit e7c2a72e2680827d6a733931273a93461c0d8d1b
Author: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
Date: Tue Nov 14 08:00:00 1995 +0000
Import of Linux/MIPS 1.3.0
in the LMO tree and this is less than a year from the inception of the
port while Ralf still bulk-imported MIPS changes on top of Linus's tree
rather than maintaining them locally. This was still a couple of years
before my days and we won't ever know more I'm afraid.
> Based on the MIPS architecture specification, delay slots are only
> present after jump instructions or MIPS1 load instructions.
> Consequently, the nop here is not intended to satisfy a delay slot
> requirement.
There are more cases of delay slots in various earlier revisions of the
MIPS architecture. But inline assembly uses the default reorder mode of
assembly, which means no delay slot will ever span inside from preceding
compiled code, and back in 1995 GCC did not itself schedule delay slots
anyway, all compiled code used the reorder mode.
> Thus, this instruction is suspicious and should probably be removed.
Based on my experience these padding NOPs are likely there to ensure a
clean state in the debug stub which may assign special meaning to other
instructions present around the breakpoint instruction and they've been
there for 30 years now, so I think they're best left alone unless you
can name an actual problem they cause you.
FAOD it's a NAK then. Let's better fix actual bugs instead.
Maciej
