Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752031AbcLEQUo (ORCPT ); Mon, 5 Dec 2016 11:20:44 -0500 Received: from mailapp01.imgtec.com ([195.59.15.196]:62482 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751455AbcLEQUk (ORCPT ); Mon, 5 Dec 2016 11:20:40 -0500 Date: Mon, 5 Dec 2016 16:20:27 +0000 From: "Maciej W. Rozycki" To: Matt Redfearn CC: Ralf Baechle , , "Jason A . Donenfeld" , Thomas Gleixner , , James Hogan , Paul Burton Subject: Re: [PATCH 3/5] MIPS: Only change $28 to thread_info if coming from user mode In-Reply-To: <1480685957-18809-4-git-send-email-matt.redfearn@imgtec.com> Message-ID: References: <1480685957-18809-1-git-send-email-matt.redfearn@imgtec.com> <1480685957-18809-4-git-send-email-matt.redfearn@imgtec.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-Originating-IP: [10.20.78.176] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1333 Lines: 34 On Fri, 2 Dec 2016, Matt Redfearn wrote: > diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h > index eebf39549606..5782fa3d63be 100644 > --- a/arch/mips/include/asm/stackframe.h > +++ b/arch/mips/include/asm/stackframe.h > @@ -216,12 +216,22 @@ > LONG_S $25, PT_R25(sp) > LONG_S $28, PT_R28(sp) > LONG_S $31, PT_R31(sp) > + > + /* Set thread_info if we're coming from user mode */ > + .set reorder > + mfc0 k0, CP0_STATUS > + sll k0, 3 /* extract cu0 bit */ > + .set noreorder > + bltz k0, 9f > + nop This code is already `.set reorder', although a badly applied CONFIG_EVA change made things slightly less obvious. So why do you need this `.set reorder' in the first place, and then why do you switch code that follows to `.set noreorder'? Overall I think all code should be using the (default) `.set reorder' mode, perhaps forced explicitly in case these macros are pasted into `.set noreorder' code, to make it easier to avoid subtle data dependency bugs, and also to make R6 porting easier. Except maybe for the RFE sequence, for readability's sake, although even there GAS will do the right thing. Surely the BLTZ/MOVE piece does not have to be `.set noreorder' as GAS will schedule that delay slot automatically if allowed to. Maciej