Re: [PATCH -v5][RFC]: mutex: implement adaptive spinning

Peter Zijlstra
2009-01-07T21:52:36Z

On Wed, 2009-01-07 at 12:55 -0800, Linus Torvalds wrote:

> /*
> * Look out! "thread" is an entirely speculative pointer
> * access and not reliable.
> */
> void loop_while_oncpu(struct mutex *lock, struct thread_struct *thread)
> {
> for (;;) {
> unsigned cpu;
> struct runqueue *rq;
>
> if (lock->owner != thread)
> break;
>
> /*
> * Need to access the cpu field knowing that
> * DEBUG_PAGEALLOC could have unmapped it if
> * the mutex owner just released it and exited.
> */
> if (__get_user(cpu, &thread->cpu))
> break;
>
> /*
> * Even if the access succeeded (likely case),
> * the cpu field may no longer be valid. FIXME:
> * this needs to validate that we can do a
> * get_cpu() and that we have the percpu area.
> */
> if (cpu >= NR_CPUS)
> break;
>
> if (!cpu_online(cpu))
> break;
>
> /*
> * Is that thread really running on that cpu?
> */
> rq = cpu_rq(cpu);
> if (task_thread_info(rq->curr) != thread)
> break;
>
> cpu_relax();
> }
> }

Do we really have to re-do all that code every loop?

void loop_while_oncpu(struct mutex *lock, struct thread_struct *thread)
{
unsigned cpu;
struct runqueue *rq;

/*
* Need to access the cpu field knowing that
* DEBUG_PAGEALLOC could have unmapped it if
* the mutex owner just released it and exited.
*/
if (__get_user(cpu, &thread->cpu))
break;

/*
* Even if the access succeeded (likely case),
* the cpu field may no longer be valid. FIXME:
* this needs to validate that we can do a
* get_cpu() and that we have the percpu area.
*/
if (cpu >= NR_CPUS)
break;

if (!cpu_online(cpu))
break;

rq = cpu_rq(cpu);

for (;;) {
if (lock->owner != thread)
break;

/*
* Is that thread really running on that cpu?
*/
if (task_thread_info(rq->curr) != thread)
break;

cpu_relax();
}
}

Also, it would still need to do the funny:

l_owner = ACCESS_ONCE(lock->owner)
if (l_owner && l_owner != thread)
break;

thing, to handle the premature non-atomic lock->owner tracking.