Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes

2007-01-18 Thread Christoph Hellwig
On Thu, Jan 18, 2007 at 08:45:56AM +0100, Ingo Molnar wrote:
> 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.

dding new syscalls for the old functionality is totally pointless as
we have to support the old entry point forever anyway.  And new code
is using the new entry point, so let's get that right.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes

2007-01-18 Thread Christoph Hellwig
On Thu, Jan 18, 2007 at 08:45:56AM +0100, Ingo Molnar wrote:
> 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.

Old code doing things wrong is no excuse for new code doing the same
mistake.  Much better send a patch to fix up the previous unreadable
mess aswell.  Then again some indentation fixups won't help that much
to make futrsx not the totally unreadable spaghetti code it's now ;-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes

2007-01-18 Thread Christoph Hellwig
On Thu, Jan 18, 2007 at 08:45:56AM +0100, Ingo Molnar wrote:
 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.

dding new syscalls for the old functionality is totally pointless as
we have to support the old entry point forever anyway.  And new code
is using the new entry point, so let's get that right.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes

2007-01-17 Thread Ingo Molnar

* Christoph Hellwig <[EMAIL PROTECTED]> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes

2007-01-17 Thread Christoph Hellwig
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.

> + 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.

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


Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes

2007-01-17 Thread Christoph Hellwig
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.

 + 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.

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


Re: [PATCH 2.6.20-rc5 4/4] sys_futex64 : allows 64bit futexes

2007-01-17 Thread Ingo Molnar

* Christoph Hellwig [EMAIL PROTECTED] 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/