All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Pierre.Peiffer@bull.net
Cc: akpm@linux-foundation.org, mingo@elte.hu, drepper@redhat.com,
	linux-kernel@vger.kernel.org, jean-pierre.dion@bull.net
Subject: Re: [PATCH 2.6.21-rc4-mm1 4/4] sys_futex64 : allows 64bit futexes
Date: Tue, 27 Mar 2007 06:07:57 -0500	[thread overview]
Message-ID: <20070327110757.GY355@devserv.devel.redhat.com> (raw)
In-Reply-To: <20070321100542.438317770@bull.net>

On Wed, Mar 21, 2007 at 10:54:36AM +0100, Pierre.Peiffer@bull.net wrote:
> This last patch is an adaptation of the sys_futex64 syscall provided in -rt
> patch (originally written by Ingo Molnar). It allows the use of 64-bit futex.
> 
> I have re-worked most of the code to avoid the duplication of the code.
> 
> It does not provide the functionality for all architectures (only for x64 for now).

I don't think you should blindly add all operations to sys_futex64 without
thinking what they really do.
E.g. FUTEX_{{,UN,TRY}LOCK,CMP_REQUEUE}_PI doesn't really make any sense for 64-bit
futexes, the format of PI futexes is hardcoded in the kernel and is always
32-bit, see FUTEX_TID_MASK, FUTEX_WAITERS, FUTEX_OWNER_DIED definitions.
exit_robust_list/handle_futex_death will handle 32-bit PI futexes anyway.
Similarly, sys_futex64 shouldn't support the obsolete operations that
are there solely for compatibility (e.g. FUTEX_REQUEUE or FUTEX_FD).

When you just -ENOSYS on the PI ops, there is no need to implement
futex_atomic_cmpxchg_inatomic64.

FUTEX_WAKE_OP is questionable for 64-bit, IMHO it is better to just
-ENOSYS on it and only if anyone ever finds actual uses for it, add it.

For 64-bit futexes the only needed operations are actually
FUTEX_WAIT and perhaps FUTEX_CMP_REQUEUE, so I wonder if it isn't
better to just add FUTEX_WAIT64 and FUTEX_CMP_REQUEUE64 ops to sys_futex
instead of adding a new syscall.

But the FUTEX_{{,UN,TRY}LOCK,CMP_REQUEUE}_PI removal for 64-bit futexes
is IMHO the most important part of my complain.

	Jakub

  parent reply	other threads:[~2007-03-27 11:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-21  9:54 [PATCH 2.6.21-rc4-mm1 0/4] Futexes functionalities and improvements Pierre.Peiffer
2007-03-21  9:54 ` [PATCH 2.6.21-rc4-mm1 1/4] futex priority based wakeup Pierre.Peiffer
2007-03-21  9:54 ` [PATCH 2.6.21-rc4-mm1 2/4] Make futex_wait() use an hrtimer for timeout Pierre.Peiffer
2007-03-26  9:57   ` Andrew Morton
2007-03-21  9:54 ` [PATCH 2.6.21-rc4-mm1 3/4] futex_requeue_pi optimization Pierre.Peiffer
2007-03-21  9:54 ` [PATCH 2.6.21-rc4-mm1 4/4] sys_futex64 : allows 64bit futexes Pierre.Peiffer
2007-03-26 11:20   ` Andrew Morton
2007-03-27 11:07   ` Jakub Jelinek [this message]
2007-04-23 14:35     ` [PATCH -mm] 64bit-futex - provide new commands instead of new syscall Pierre Peiffer
2007-04-23 15:30       ` Ulrich Drepper
2007-04-24  8:07         ` [PATCH -mm take2] " Pierre Peiffer
2007-04-24 13:25           ` Ulrich Drepper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070327110757.GY355@devserv.devel.redhat.com \
    --to=jakub@redhat.com \
    --cc=Pierre.Peiffer@bull.net \
    --cc=akpm@linux-foundation.org \
    --cc=drepper@redhat.com \
    --cc=jean-pierre.dion@bull.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.