Re: [rfc][patch] futex: restartable futex_wait?

2007-03-09 Thread Thomas Gleixner
On Fri, 2007-03-09 at 13:24 +0100, Nick Piggin wrote:
> > > > 'time' here is relative, so the restarted syscall will do a /full/ wait 
> > > > again.
> > > 
> > > But it has been modified by schedule_timeout?
> > 
> > But this does not change the syscall registers, so it is restarted in
> > the same way. We need a new futex OP for this, which takes absolute time
> > like the PI futex op does.
> 
> Forgive me if I'm missing something here, but I'm using the restart block
> and saving the updated value of time in ->arg2, and using that as the new
> time parameter passed into futex_wait from futex_wait_restart.

Oops. I went into confusion mode. You are right, the restart block keeps
that.

tglx


-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-09 Thread Nick Piggin
On Fri, Mar 09, 2007 at 10:38:35AM +0100, Thomas Gleixner wrote:
> On Fri, 2007-03-09 at 06:10 +0100, Nick Piggin wrote:
> > > i think that's quite right. I'm wondering why this never came up before? 
> > > But your fix is not complete i think:
> > > 
> > > > + restart->arg2 = time;
> > > > + return -ERESTART_RESTARTBLOCK;
> > > > + }
> > > 
> > > 'time' here is relative, so the restarted syscall will do a /full/ wait 
> > > again.
> > 
> > But it has been modified by schedule_timeout?
> 
> But this does not change the syscall registers, so it is restarted in
> the same way. We need a new futex OP for this, which takes absolute time
> like the PI futex op does.

Forgive me if I'm missing something here, but I'm using the restart block
and saving the updated value of time in ->arg2, and using that as the new
time parameter passed into futex_wait from futex_wait_restart.

-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-09 Thread Thomas Gleixner
On Fri, 2007-03-09 at 06:10 +0100, Nick Piggin wrote:
> > i think that's quite right. I'm wondering why this never came up before? 
> > But your fix is not complete i think:
> > 
> > > + restart->arg2 = time;
> > > + return -ERESTART_RESTARTBLOCK;
> > > + }
> > 
> > 'time' here is relative, so the restarted syscall will do a /full/ wait 
> > again.
> 
> But it has been modified by schedule_timeout?

But this does not change the syscall registers, so it is restarted in
the same way. We need a new futex OP for this, which takes absolute time
like the PI futex op does.

tglx


-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-09 Thread Thomas Gleixner
On Fri, 2007-03-09 at 06:10 +0100, Nick Piggin wrote:
  i think that's quite right. I'm wondering why this never came up before? 
  But your fix is not complete i think:
  
   + restart-arg2 = time;
   + return -ERESTART_RESTARTBLOCK;
   + }
  
  'time' here is relative, so the restarted syscall will do a /full/ wait 
  again.
 
 But it has been modified by schedule_timeout?

But this does not change the syscall registers, so it is restarted in
the same way. We need a new futex OP for this, which takes absolute time
like the PI futex op does.

tglx


-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-09 Thread Nick Piggin
On Fri, Mar 09, 2007 at 10:38:35AM +0100, Thomas Gleixner wrote:
 On Fri, 2007-03-09 at 06:10 +0100, Nick Piggin wrote:
   i think that's quite right. I'm wondering why this never came up before? 
   But your fix is not complete i think:
   
+ restart-arg2 = time;
+ return -ERESTART_RESTARTBLOCK;
+ }
   
   'time' here is relative, so the restarted syscall will do a /full/ wait 
   again.
  
  But it has been modified by schedule_timeout?
 
 But this does not change the syscall registers, so it is restarted in
 the same way. We need a new futex OP for this, which takes absolute time
 like the PI futex op does.

Forgive me if I'm missing something here, but I'm using the restart block
and saving the updated value of time in -arg2, and using that as the new
time parameter passed into futex_wait from futex_wait_restart.

-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-09 Thread Thomas Gleixner
On Fri, 2007-03-09 at 13:24 +0100, Nick Piggin wrote:
'time' here is relative, so the restarted syscall will do a /full/ wait 
again.
   
   But it has been modified by schedule_timeout?
  
  But this does not change the syscall registers, so it is restarted in
  the same way. We need a new futex OP for this, which takes absolute time
  like the PI futex op does.
 
 Forgive me if I'm missing something here, but I'm using the restart block
 and saving the updated value of time in -arg2, and using that as the new
 time parameter passed into futex_wait from futex_wait_restart.

Oops. I went into confusion mode. You are right, the restart block keeps
that.

tglx


-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Nick Piggin
On Fri, Mar 09, 2007 at 12:02:31AM +0100, Thomas Gleixner wrote:
> On Thu, 2007-03-08 at 18:29 +0100, Ingo Molnar wrote:
> > * Nick Piggin <[EMAIL PROTECTED]> wrote:
> > 
> > > Hi Ingo,
> > > 
> > > I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
> > > it tests whether the SA_RESTART flag works for the sem_wait operation.
> 
> Not sure, whether the testcase is correct or not. See below
> 
> > > I see sem_wait is implemented with futex_wait, so I wonder whether we 
> > > can make it restartable? Am I going about it the right way? (Seems to 
> > > fix the testcase here).
> > 
> > i think that's quite right. I'm wondering why this never came up before? 
> > But your fix is not complete i think:
> > 
> > > + restart->arg2 = time;
> > > + return -ERESTART_RESTARTBLOCK;
> > > + }
> > 
> > 'time' here is relative, so the restarted syscall will do a /full/ wait 
> > again.
> > 
> > maybe we should rather convert futex timed-waits to hrtimers? Thomas?
> 
> The problem is that the original API is based on relative time and
> therefor can not be changed. 
> 
> sem_wait returns -EINTR to the application when it is interrupted, while
> pthread_mutex_lock does not.

But this still means sem_wait should restart if SA_RESTART is set, right?

And pthread_mutex_lock could be implemented to not return -EINTR, even if
futex_wait does, couldn't it? (I guess it probably already is, considering
that futex_wait alsready returns -EINTR).

> 
> http://www.opengroup.org/onlinepubs/009695399/functions/sem_wait.html
> 
> http://www.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_lock.html
> 
> We need to create a seperate op for the futex - just like the pi_futex
> and use absolute time there too. 
> 
>   tglx
> 
-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Nick Piggin
On Thu, Mar 08, 2007 at 06:29:02PM +0100, Ingo Molnar wrote:
> 
> * Nick Piggin <[EMAIL PROTECTED]> wrote:
> 
> > Hi Ingo,
> > 
> > I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
> > it tests whether the SA_RESTART flag works for the sem_wait operation.
> > 
> > I see sem_wait is implemented with futex_wait, so I wonder whether we 
> > can make it restartable? Am I going about it the right way? (Seems to 
> > fix the testcase here).
> 
> i think that's quite right. I'm wondering why this never came up before? 
> But your fix is not complete i think:
> 
> > + restart->arg2 = time;
> > + return -ERESTART_RESTARTBLOCK;
> > + }
> 
> 'time' here is relative, so the restarted syscall will do a /full/ wait 
> again.

But it has been modified by schedule_timeout?

-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Thomas Gleixner
On Thu, 2007-03-08 at 18:29 +0100, Ingo Molnar wrote:
> * Nick Piggin <[EMAIL PROTECTED]> wrote:
> 
> > Hi Ingo,
> > 
> > I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
> > it tests whether the SA_RESTART flag works for the sem_wait operation.

Not sure, whether the testcase is correct or not. See below

> > I see sem_wait is implemented with futex_wait, so I wonder whether we 
> > can make it restartable? Am I going about it the right way? (Seems to 
> > fix the testcase here).
> 
> i think that's quite right. I'm wondering why this never came up before? 
> But your fix is not complete i think:
> 
> > + restart->arg2 = time;
> > + return -ERESTART_RESTARTBLOCK;
> > + }
> 
> 'time' here is relative, so the restarted syscall will do a /full/ wait 
> again.
> 
> maybe we should rather convert futex timed-waits to hrtimers? Thomas?

The problem is that the original API is based on relative time and
therefor can not be changed. 

sem_wait returns -EINTR to the application when it is interrupted, while
pthread_mutex_lock does not.

http://www.opengroup.org/onlinepubs/009695399/functions/sem_wait.html

http://www.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_lock.html

We need to create a seperate op for the futex - just like the pi_futex
and use absolute time there too. 

tglx


-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Ingo Molnar

* Nick Piggin <[EMAIL PROTECTED]> wrote:

> Hi Ingo,
> 
> I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
> it tests whether the SA_RESTART flag works for the sem_wait operation.
> 
> I see sem_wait is implemented with futex_wait, so I wonder whether we 
> can make it restartable? Am I going about it the right way? (Seems to 
> fix the testcase here).

i think that's quite right. I'm wondering why this never came up before? 
But your fix is not complete i think:

> + restart->arg2 = time;
> + return -ERESTART_RESTARTBLOCK;
> + }

'time' here is relative, so the restarted syscall will do a /full/ wait 
again.

maybe we should rather convert futex timed-waits to hrtimers? Thomas?

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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Ingo Molnar

* Nick Piggin [EMAIL PROTECTED] wrote:

 Hi Ingo,
 
 I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
 it tests whether the SA_RESTART flag works for the sem_wait operation.
 
 I see sem_wait is implemented with futex_wait, so I wonder whether we 
 can make it restartable? Am I going about it the right way? (Seems to 
 fix the testcase here).

i think that's quite right. I'm wondering why this never came up before? 
But your fix is not complete i think:

 + restart-arg2 = time;
 + return -ERESTART_RESTARTBLOCK;
 + }

'time' here is relative, so the restarted syscall will do a /full/ wait 
again.

maybe we should rather convert futex timed-waits to hrtimers? Thomas?

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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Thomas Gleixner
On Thu, 2007-03-08 at 18:29 +0100, Ingo Molnar wrote:
 * Nick Piggin [EMAIL PROTECTED] wrote:
 
  Hi Ingo,
  
  I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
  it tests whether the SA_RESTART flag works for the sem_wait operation.

Not sure, whether the testcase is correct or not. See below

  I see sem_wait is implemented with futex_wait, so I wonder whether we 
  can make it restartable? Am I going about it the right way? (Seems to 
  fix the testcase here).
 
 i think that's quite right. I'm wondering why this never came up before? 
 But your fix is not complete i think:
 
  + restart-arg2 = time;
  + return -ERESTART_RESTARTBLOCK;
  + }
 
 'time' here is relative, so the restarted syscall will do a /full/ wait 
 again.
 
 maybe we should rather convert futex timed-waits to hrtimers? Thomas?

The problem is that the original API is based on relative time and
therefor can not be changed. 

sem_wait returns -EINTR to the application when it is interrupted, while
pthread_mutex_lock does not.

http://www.opengroup.org/onlinepubs/009695399/functions/sem_wait.html

http://www.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_lock.html

We need to create a seperate op for the futex - just like the pi_futex
and use absolute time there too. 

tglx


-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Nick Piggin
On Thu, Mar 08, 2007 at 06:29:02PM +0100, Ingo Molnar wrote:
 
 * Nick Piggin [EMAIL PROTECTED] wrote:
 
  Hi Ingo,
  
  I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
  it tests whether the SA_RESTART flag works for the sem_wait operation.
  
  I see sem_wait is implemented with futex_wait, so I wonder whether we 
  can make it restartable? Am I going about it the right way? (Seems to 
  fix the testcase here).
 
 i think that's quite right. I'm wondering why this never came up before? 
 But your fix is not complete i think:
 
  + restart-arg2 = time;
  + return -ERESTART_RESTARTBLOCK;
  + }
 
 'time' here is relative, so the restarted syscall will do a /full/ wait 
 again.

But it has been modified by schedule_timeout?

-
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: [rfc][patch] futex: restartable futex_wait?

2007-03-08 Thread Nick Piggin
On Fri, Mar 09, 2007 at 12:02:31AM +0100, Thomas Gleixner wrote:
 On Thu, 2007-03-08 at 18:29 +0100, Ingo Molnar wrote:
  * Nick Piggin [EMAIL PROTECTED] wrote:
  
   Hi Ingo,
   
   I'm seeing an LTP test fail for ltp test sigaction_16_24. Basically, 
   it tests whether the SA_RESTART flag works for the sem_wait operation.
 
 Not sure, whether the testcase is correct or not. See below
 
   I see sem_wait is implemented with futex_wait, so I wonder whether we 
   can make it restartable? Am I going about it the right way? (Seems to 
   fix the testcase here).
  
  i think that's quite right. I'm wondering why this never came up before? 
  But your fix is not complete i think:
  
   + restart-arg2 = time;
   + return -ERESTART_RESTARTBLOCK;
   + }
  
  'time' here is relative, so the restarted syscall will do a /full/ wait 
  again.
  
  maybe we should rather convert futex timed-waits to hrtimers? Thomas?
 
 The problem is that the original API is based on relative time and
 therefor can not be changed. 
 
 sem_wait returns -EINTR to the application when it is interrupted, while
 pthread_mutex_lock does not.

But this still means sem_wait should restart if SA_RESTART is set, right?

And pthread_mutex_lock could be implemented to not return -EINTR, even if
futex_wait does, couldn't it? (I guess it probably already is, considering
that futex_wait alsready returns -EINTR).

 
 http://www.opengroup.org/onlinepubs/009695399/functions/sem_wait.html
 
 http://www.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_lock.html
 
 We need to create a seperate op for the futex - just like the pi_futex
 and use absolute time there too. 
 
   tglx
 
-
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/