This is the mail archive of the libc-alpha@sources.redhat.com mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [aegl@unix-os.sc.intel.com] libc/2223: _pthread_lock sometimesspins uselessly


On 30 Apr 2001, Andreas Jaeger wrote:

> Date: 30 Apr 2001 20:52:14 +0200
> From: Andreas Jaeger <aj@suse.de>
> To: libc-alpha@sources.redhat.com
> Cc: aegl@unix-os.sc.intel.com
> Subject: [aegl@unix-os.sc.intel.com] libc/2223: _pthread_lock sometimes
>     spins uselessly
>
>
> Hi glibc folks,
>
> please comment on the appended bug report.  Should we really change
> _pthread_fastlock to:
>
> struct _pthread_fastlock
> {
>   volatile long int __status;   /* "Free" or "taken" or head of waiting list */

No! Ulrich Drepper already put in a fix for this, but it did not go
into 2.2.1 against which this bug report was made. The bug submitter
should be advised that the problem is addressed in 2.2.3.

There are a few plces in the spinlock.c code where the __status field
is subject to a direct membre access. Most of these are fine because
the either take place on entry to a function, or inside some loop which
also takes the address of the __status field and passes it to a
function (compare_and_swap). So the compiler can't optimize the value
in a register in these contexts.

The one troublesome place is the new spinning logic, where a loop can
repeatedly perform an access to lock->__status without calling any
external function. I should have seen this one! :(

To change the declaration to volatile over this one access would be
somewhat of a nuisance, since every place where a pointer to the
status field is manipulated would have to acquire the qualifier.

One solution that probably would have worked would have been to change
the access to something like

    /* access __status through volatile lvalue alias */
    if (*(volatile *) &lock->__status) ...

Ulrich went with another solution, which is to insert into the body
of the loop an __asm__ directive that introduces an optimization
barrier (but does not add any actual machine instruction):

    __asm__ __volatile__ ("" : "=m" (lock->__status) : "0" (lock->__status));

Another directive that would work is

    __asm__ __volatile__ ("" : : : "memory");

which is notably used in the Linux kernel. For fun, check the assembly
language produced by this program, and then see it again with the
__asm__ removed:

    struct foo {
	int i;
    } x;

    int main()
    {
	struct foo *p = &x;

	while (p->i != 0) {
	    __asm__ __volatile__ ( "" : : : "memory" );
	}
    }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]