linux-kernel - Re: [PATCH 2.6.19-rt12][RFC] - futex_requeue_pi implementation (requeue from futex1 to PI-futex2)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20061213175405.GB32441@elte.hu>
Date:	Wed, 13 Dec 2006 18:54:05 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Pierre Peiffer <pierre.peiffer@...l.net>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Dinakar Guniguntala <dino@...ibm.com>,
	Jean-Pierre Dion <jean-pierre.dion@...l.net>,
	Sébastien Dugué <sebastien.dugue@...l.net>,
	Ulrich Drepper <drepper@...hat.com>,
	Darren Hart <dvhltc@...ibm.com>,
	Jakub Jelinek <jakub@...hat.com>
Subject: Re: [PATCH 2.6.19-rt12][RFC] - futex_requeue_pi implementation (requeue from futex1 to PI-futex2)


* Pierre Peiffer <pierre.peiffer@...l.net> wrote:

> With the introduction of the PI-futex (used for the PI-pthread_mutex 
> in the glibc), the current futex_requeue optimization (*) can not be 
> used any more if the pthread_mutex used with the pthread-condvar is a 
> PI-mutex.
> 
> (*) this optimization is used in pthread_cond_broadcast.
> 
> To use this optimization with PI-mutex, we need a function that
> re-queues some threads from a normal futex  to a PI-futex (see why in
> this discussion:
> http://marc.theaimsgroup.com/?l=linux-kernel&m=115020355126851&w=2 )
> 
> Here is a proposal of an implementation of futex_requeue_pi.
> (Only the 32-bits version has been implemented for now)

the kernel patch looks straightforward and desirable to me. (modulo some 
minor style-nitpicking observations below.) I'd suggest to implement the 
64-bit and compat versions too, unless Jakub can see a hole in the 
concept or in the implementation.

> -#define FUTEX_TID_MASK		0x3fffffff
> +#define FUTEX_TID_MASK		0x1fffffff

ABI change but tthis should be fine i think ... right now we dont let 
PIDs go above 0xffff anyway. It might make sense to lower it to 
0x0fffffff, to have one more bit in that word ... just in case.

> +	/* This waiter is used in case of requeue from a
> +	   normal futex to a PI-futex */

please use proper comment style.

> +	if (key->both.offset & 1)
> +		/* shared mapping */
> +		uaddr = (void*)((key->shared.pgoff << PAGE_SHIFT)
> +				+ key->shared.offset - 1);
> +	else
> +		/* private mapping */
> +		uaddr = (void*)(key->private.address + key->private.offset);

such multi-line branches need curly braces.

> +static inline int lookup_pi_state_for_requeue(u32 __user *uaddr,
> +					      struct futex_hash_bucket *hb,
> +					      union futex_key *key,
> +					      struct futex_pi_state 
> **pi_state)

patch line wrap problem? Also, if the function name is so long, you can 
do:

static inline int
lookup_pi_state_for_requeue(u32 __user *uaddr, struct futex_hash_bucket *hb,

	Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/