From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932512AbbERVV5 (ORCPT ); Mon, 18 May 2015 17:21:57 -0400 Received: from www.linutronix.de ([62.245.132.108]:51290 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753069AbbERVV4 (ORCPT ); Mon, 18 May 2015 17:21:56 -0400 Date: Mon, 18 May 2015 23:22:06 +0200 (CEST) From: Thomas Gleixner To: Dave Hansen cc: linux-kernel@vger.kernel.org, x86@kernel.org, dave.hansen@linux.intel.com Subject: Re: [PATCH 15/19] x86, mpx: do 32-bit-only cmpxchg for 32-bit apps In-Reply-To: <20150508185955.3704EED5@viggo.jf.intel.com> Message-ID: References: <20150508185948.4C19F4B0@viggo.jf.intel.com> <20150508185955.3704EED5@viggo.jf.intel.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 8 May 2015, Dave Hansen wrote: > +static int mpx_cmpxchg_bd_entry(struct mm_struct *mm, > + unsigned long *actual_old_val_ptr, long __user *bd_entry_addr, > + unsigned long expected_old_val, unsigned long new_bd_entry) I know that you tried to use descriptive argument names, but the result is close to camel case argument lists :) Also why is *bd_entry_addr a long? Shouldn't that be unsigned long? static int mpx_cmpxchg_bd_entry(struct mm_struct *mm, unsigned long __user *addr, unsigned long oldval, unsigned long newval, unsigned long *curval) or something like that should be sufficient, methinks. > +{ > + int ret; > + /* > + * user_atomic_cmpxchg_inatomic() actually uses sizeof() > + * the pointer that we pass to it to figure out how much > + * data to cmpxchg. We have to be careful here not to > + * pass a pointer to a 64-bit data type when we only want > + * a 32-bit copy. > + */ > + if (is_64bit_mm(mm)) { > + ret = user_atomic_cmpxchg_inatomic(actual_old_val_ptr, > + bd_entry_addr, expected_old_val, new_bd_entry); > + } else { > + u32 uninitialized_var(actual_old_val_32); > + u32 expected_old_val_32 = expected_old_val; > + u32 new_bd_entry_32 = new_bd_entry; > + u32 __user *bd_entry_32 = (u32 __user *)bd_entry_addr; Newline between variables and code please. > + ret = user_atomic_cmpxchg_inatomic(&actual_old_val_32, > + bd_entry_32, expected_old_val_32, > + new_bd_entry_32); > + if (!ret) > + *actual_old_val_ptr = actual_old_val_32; You can update that unconditionally. Thanks, tglx