[libc-alpha] linuxthreads bug in 2.2.4 under ppc linux
Franz Sirl
Franz.Sirl-kernel@lauterbach.com
Mon Dec 10 11:48:00 GMT 2001
At 16:43 09.12.2001, Kevin B. Hendricks wrote:
>Hi,
>
>I think this is optimization related (but I am still not sure).
>
>Simply adding the one line:
> if (p_node == (struct wait_node *) 0) MSG("huh %p\n",&lock);
>
>in spinlock.c in __pthread_alt_unlock() as shown below is enough to make
>things work (and this if is never taken).
Yeah, because you take the address of a parameter, which forces it on the
stack, leading to quite different code layout. Best call a single argument
function here or directly abort().
> READ_MEMORY_BARRIER(); /* Prevent access to stale data through p_node
>*/
>
> while (p_node != (struct wait_node *) 1) {
> int prio;
>
> if (p_node == (struct wait_node *) 0) MSG("huh %p\n",&lock);
>
>
>The question here is why does accessing p_node in this way make such a big
>difference to the compiler.
Hmm, actually p_node == lock->__status == *lock and lock == pp_head, with
O3 inlining stuff the value is actually held in a 2 or 3 registers at once
and doesn't change it's value (it isn't even reloaded) over the whole function.
Kevin, can you try to replace alt_unlock with the manually inlined version
below and recompile glibc at -O2? If the problem remains, there must be a
race somewhere in there I guess (the produced code is nearly 100% identical
to the -O3 compiler inlined version). I wonder though why this code uses
CAS at all, wouldn't it be better to have a thread safe list insertion code
in pt-machine.h, like outlined in the PPC manuals?
Franz.
void __pthread_alt_unlock(struct _pthread_fastlock *lock)
{
struct wait_node *p_node, **pp_node, *p_max_prio, **pp_max_prio;
struct wait_node ** const pp_head = (struct wait_node **) &lock->__status;
int maxprio;
__asm__ __volatile__ ("sync" : : : "memory");
while (1)
{
{
long oldstatus = lock->__status;
if (oldstatus == 0 || oldstatus == 1)
{
if (__compare_and_swap_with_release_semantics
(&lock->__status, oldstatus, 0))
break;
else
continue;
}
}
pp_max_prio = pp_node = pp_head;
p_max_prio = p_node = *pp_head;
maxprio = (-2147483647 -1);
__asm__ __volatile__ ("sync" : : : "memory");
while (p_node != (struct wait_node *) 1)
{
int prio;
if (p_node == (struct wait_node *) 0) abort (&lock->__status);
if (p_node->abandoned)
{
{
struct wait_node **pp_headi = pp_head;
struct wait_node **pp_nodei = pp_node;
struct wait_node *p_nodei = p_node;
if (pp_nodei == pp_headi)
{
long oldvalue = (long) p_nodei;
long newvalue = (long) p_nodei->next;
if (__compare_and_swap((long *) pp_nodei, oldvalue,
newvalue))
goto exit1;
for (pp_nodei = pp_headi; p_nodei != *pp_nodei; )
{
pp_nodei = &(*pp_nodei)->next;
__asm__ __volatile__ ("sync" : : : "memory");
}
}
*pp_nodei = p_nodei->next;
}
exit1:
{
struct wait_node *wn = p_node;
long oldvalue, newvalue;
do
{
oldvalue = wait_node_free_list;
wn->next = (struct wait_node *) oldvalue;
newvalue = (long) wn;
__asm__ __volatile__ ("sync" : : : "memory");
} while (! __compare_and_swap(&wait_node_free_list,
oldvalue, newvalue));
}
p_node = *pp_node;
if (pp_node == pp_head)
__asm__ __volatile__ ("sync" : : : "memory");
continue;
}
else if ((prio = p_node->thr->p_priority) >= maxprio)
{
maxprio = prio;
pp_max_prio = pp_node;
p_max_prio = p_node;
}
pp_node = &p_node->next;
p_node = *pp_node;
}
if (maxprio == (-2147483647 -1))
continue;
if (!!__compare_and_swap((long int *) &p_max_prio->abandoned, 0, 1))
{
{
struct wait_node **pp_headi = pp_head;
struct wait_node **pp_nodei = pp_max_prio;
struct wait_node *p_nodei = p_max_prio;
if (pp_nodei == pp_headi)
{
long oldvalue = (long) p_nodei;
long newvalue = (long) p_nodei->next;
if (__compare_and_swap((long *) pp_nodei, oldvalue,
newvalue))
goto exit2;
for (pp_nodei = pp_headi; p_nodei != *pp_nodei; )
{
pp_nodei = &(*pp_nodei)->next;
__asm__ __volatile__ ("sync" : : : "memory");
}
}
*pp_nodei = p_nodei->next;
}
exit2:
restart(p_max_prio->thr);
break;
}
}
}
More information about the Libc-alpha
mailing list