On Tue, Jan 20, 2015 at 09:17:59AM -0800, David Daney wrote:
> >>>sub $reg, imm is not a real MIPS instruction. The assembler replaces
> >>>that with 'addi $reg, -imm'. However, addi has been removed from R6,
> >>>so we replace the 'sub' instruction with 'addiu' instead.
> >>>
> >>>Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx>
> >>>---
> >>> arch/mips/include/asm/spinlock.h | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/arch/mips/include/asm/spinlock.h b/arch/mips/include/asm/spinlock.h
> >>>index c6d06d383ef9..500050d3bda6 100644
> >>>--- a/arch/mips/include/asm/spinlock.h
> >>>+++ b/arch/mips/include/asm/spinlock.h
> >>>@@ -276,7 +276,7 @@ static inline void arch_read_unlock(arch_rwlock_t *rw)
> >>> do {
> >>> __asm__ __volatile__(
> >>> "1: ll %1, %2 # arch_read_unlock \n"
> >>>- " sub %1, 1 \n"
> >>>+ " addiu %1, -1 \n"
> >>> " sc %1, %0 \n"
> >>> : "=" GCC_OFF12_ASM() (rw->lock), "=&r" (tmp)
> >>> : GCC_OFF12_ASM() (rw->lock)
> >>
> >> This integer overflow trap is deliberate here -- have you seen the note
> >>just above:
> >>
> >>/* Note the use of sub, not subu which will make the kernel die with an
> >> overflow exception if we ever try to unlock an rwlock that is already
> >> unlocked or is being held by a writer. */
> >>
>
> According to a comment on another thread from Ralf, this has been observed
> in the wild only once. We can simplify the code and remove that comment.
> Why not just use the ADDIU and be done with it?
>
> There are many locking and atomic primitives that don't have any such error
> checking. What makes the read lock so special that it needs this extra
> protection?
Because I was desparate to find a use for the signed add ;-)
Honestly, it's nice to have such a safeguard if it's available at no
runtime overhead at all but these days are such nice lock debugging tools
that the loss won't be missed. So (cut'n'paste):
Why not just use the ADDIU and be done with it?
Ralf
