From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752096AbcEGIqZ (ORCPT ); Sat, 7 May 2016 04:46:25 -0400 Received: from www.linutronix.de ([62.245.132.108]:33434 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750936AbcEGIqW (ORCPT ); Sat, 7 May 2016 04:46:22 -0400 Date: Sat, 7 May 2016 10:44:39 +0200 (CEST) From: Thomas Gleixner To: Darren Hart cc: LKML , Sebastian Andrzej Siewior , Linus Torvalds , Darren Hart , Peter Zijlstra , Ingo Molnar , Michael Kerrisk , Davidlohr Bueso , Chris Mason , "Carlos O'Donell" , Torvald Riegel , Eric Dumazet Subject: Re: [patch V2 2/7] futex: Hash private futexes per process In-Reply-To: <20160506180933.GE48432@f23x64.localdomain> Message-ID: References: <20160505204230.932454245@linutronix.de> <20160505204353.973009518@linutronix.de> <20160506180933.GE48432@f23x64.localdomain> 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, 6 May 2016, Darren Hart wrote: > On Thu, May 05, 2016 at 08:44:04PM -0000, Thomas Gleixner wrote: > > --- /dev/null > > +++ b/include/linux/futex_types.h > > @@ -0,0 +1,12 @@ > > +#ifndef _LINUX_FUTEX_TYPES_H > > +#define _LINUX_FUTEX_TYPES_H > > + > > +struct futex_hash_bucket; > > + > > +struct futex_hash { > > + struct raw_spinlock lock; > > As it isn't always obvious to everone, it would be good to add a single line > comment stating why a *raw* spinlock is necessary. Well. Necessary. It protects the hash pointer and the hash bits. So the scope is very limited and really does not need the heavy weight version of a sleeping spinlock in RT. > In this case... I suppose this could lead to some nasty scenarios setting up IPC > mechanisms between threads if they weren't strictly serialized? Something else? Sure, we need to serialize attempts to populate the hash. Especially in the non preallocated case. The thing with raw vs. non raw spinlocks is that the latter are expensive on RT and if there are just 5 instructions to protect it does not make any sense to chose the heavy version. > > +config FUTEX_PRIVATE_HASH > > + bool > > + default FUTEX && SMP > > + > > So no prompt, not user selectable. If you have SMP, you get this? I think > automatic is a good call... but is SMP the right criteria, or would NUMA be more > appropriate since I thought it was keeping the hash local to the NUMA node that > was the big win? Yes, we can make it depend on NUMA. I even thought about making a run time decision for non preallocated ones when the machine is not numa. But for test coverage I wanted to have it as widely used as possible. > > + /* > > + * Futexes which use the per process hash have the lower bits cleared > > + */ > > + if (key->both.offset & (FUT_OFF_INODE | FUT_OFF_MMSHARED)) > > + return hash_global_futex(key); > > + > > + slot = hash_long(key->private.address, mm->futex_hash.hash_bits); > > + return &mm->futex_hash.hash[slot]; > > Don't we also need to check if the private hash exists? Per the commit > description, if we fail to allocate the private hash, we fall back to using the > global hash... If we fall back to the global hash, then the lower bits in offset are not 0. So the hash is guaranteed to be available. Thanks, tglx