From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Gleixner Subject: Re: [PATCH 1/1] futex: remove duplicated code Date: Mon, 22 May 2017 23:11:33 +0200 (CEST) Message-ID: References: <20170515130742.18357-1-jslaby@suse.cz> <20170515131644.GA3605@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:Message-ID: In-Reply-To:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=K8pT/xaSjgAEhVSSmZnuJ8B9u/8RxsUit+1OX3ui+wU=; b=EA6idWCOnC4+Qa OTzE6QR0B3qkGrHFGL7gbC5cw24tl9fUkB0mNhdHckGGvSfgoNOQGfxVg0YPmmpaUgxpMtq55F3MF iqiMfIcNT1xYzAlXsTsxiXthTHyajI1uFGszU424nt06XuKsiQ9gR2jrA/c1PQasBxTZLXgymBSfZ J+6YVcrJURDFB1kac+4DB9pEoOhecaQ6qdYSNanSj9oZ4I2ARpIWHyINdlC7XyOegK3Mma41QVXlc B7ajYxhYZQtOsgeEhh0Vp2ZShYlVN43qjzny1DWOLq5TsGsKr2XAbbF2wlKVDjBd2H3u9YHm7fhok 0xkMEEv2xPuQ/xnRRmZw==; In-Reply-To: <20170515131644.GA3605@arm.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-snps-arc" Errors-To: linux-snps-arc-bounces+gla-linux-snps-arc=m.gmane.org@lists.infradead.org To: Will Deacon Cc: linux-mips@linux-mips.org, Rich Felker , linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org, Benjamin Herrenschmidt , Max Filippov , Paul Mackerras , "H. Peter Anvin" , sparclinux@vger.kernel.org, Jiri Slaby , Jonas Bonn , linux-s390@vger.kernel.org, linux-arch@vger.kernel.org, Yoshinori Sato , linux-hexagon@vger.kernel.org, Helge Deller , x86@kernel.org, "James E.J. Bottomley" , Ingo Molnar , Catalin Marinas , Matt Turner , linux-snps-arc@lists.infradead.org, Fenghua Yu , Arnd Bergmann , linux-xtensa@linux-xtensa.org, Stefan Kristiansson On Mon, 15 May 2017, Will Deacon wrote: > Hi Jiri, > > On Mon, May 15, 2017 at 03:07:42PM +0200, Jiri Slaby wrote: > > There is code duplicated over all architecture's headers for > > futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr, > > and comparison of the result. > > > > Remove this duplication and leave up to the arches only the needed > > assembly which is now in arch_futex_atomic_op_inuser. > > > > Note that s390 removed access_ok check in d12a29703 ("s390/uaccess: > > remove pointless access_ok() checks") as access_ok there returns true. > > We introduce it back to the helper for the sake of simplicity (it gets > > optimized away anyway). > > Whilst I think this is a good idea, the code in question actually results > in undefined behaviour per the C spec and is reported by UBSAN. See my > patch fixing arm64 here (which I'd forgotten about): > > https://www.spinics.net/lists/linux-arch/msg38564.html > > But, as stated in the thread above, I think we should go a step further > and remove FUTEX_OP_{OR,ANDN,XOR,OPARG_SHIFT} altogether. They don't > appear to be used by userspace, and this whole thing is a total mess. You wish. The constants are not used, but FUTEX_WAKE_OP _IS_ used by glibc. They only have one argument it seems: #define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE ((4 << 24) | 1) but I'm pretty sure that there is enough (probably horrible) code (think java) out there using FUTEX_WAKE_OP for whatever (non)sensical reasons in any available combination. Thanks, tglx