LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Christoph Hellwig <hch@infradead.org>,
	Pierre Peiffer <pierre.peiffer@bull.net>,
	LKML <linux-kernel@vger.kernel.org>,
	Ulrich Drepper <drepper@redhat.com>,
	Jakub Jelinek <jakub@redhat.com>,
	Jean-Pierre Dion <jean-pierre.dion@bull.net>
Subject: Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes
Date: Thu, 18 Jan 2007 08:45:56 +0100	[thread overview]
Message-ID: <20070118074556.GB29128@elte.hu> (raw)
In-Reply-To: <20070118001758.GB17257@infradead.org>


* Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Jan 17, 2007 at 10:04:53AM +0100, Pierre Peiffer wrote:
> > Hi,
> > 
> > This latest patch is an adaptation of the sys_futex64 syscall 
> > provided in -rt patch (originally written by Ingo). It allows the 
> > use of 64bit futex.
> 
> Big NACK here, we don't need yet another goddamn multiplexer.  Please 
> make this individual syscalls for the actual operations.

actually, we have a big multiplexer there already, so it's only 
symmetric. Nothing is served by doing it half-assed. I raised the issue 
of the multiplexer back when the first futex API was merged (years ago), 
and it was rejected. Now whether you like it or not we've got to live 
with that decision. You are certainly free to introduce a patchset with 
a completely new set of syscall vectors to demultiplex all futex APIs, 
but to just start a half-done demultiplexing makes zero sense.

> > +	if (!ret) {
> > +		switch (cmp) {
> > +		case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
> > +		case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
> 
> Please indent this properly, the ret = .. and reak need to go onto a 
> line on it's own.

this is the standard (already upstream) arithmetics style there for the 
futex cmp ops, and it expresses things in a compact way. See 
include/asm-i386/futex.h:

                switch (cmp) {
                case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
                case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
                case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
                case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
                case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
                case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
                default: ret = -ENOSYS;
                }

Pierre correctly matched the existing style.

	Ingo

  reply	other threads:[~2007-01-18  7:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <45ADDF60.5080704@bull.net>
2007-01-17  9:00 ` [PATCH 2.6.20-rc5 1/4] futex priority based wakeup Pierre Peiffer
2007-01-17  9:01 ` [PATCH 2.6.20-rc5 2/4] Make futex_wait() use an hrtimer for timeout Pierre Peiffer
2007-01-17  9:03 ` [PATCH 2.6.20-rc5 3/4] futex_requeue_pi optimization Pierre Peiffer
2007-01-17  9:04 ` [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes Pierre Peiffer
2007-01-18  0:17   ` Christoph Hellwig
2007-01-18  7:45     ` Ingo Molnar [this message]
2007-01-18 22:34       ` Christoph Hellwig
2007-01-18 22:35       ` Christoph Hellwig

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=20070118074556.GB29128@elte.hu \
    --to=mingo@elte.hu \
    --cc=drepper@redhat.com \
    --cc=hch@infradead.org \
    --cc=jakub@redhat.com \
    --cc=jean-pierre.dion@bull.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre.peiffer@bull.net \
    --subject='Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).