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

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

[Andreas Jaeger]

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 */
  int __spinlock;      /* Used by compare_and_swap emulation. Also,
			  adaptive SMP lock stores spin count here. */
};

Andreas

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

[David S. Miller]

Andreas Jaeger writes:
 > please comment on the appended bug report.  Should we really change
 > _pthread_fastlock to:
 ...
 >   volatile long int __status;   /* "Free" or "taken" or head of waiting list */

His analysis seems correct to me.

Later,
David S. Miller
davem@redhat.com

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

[Jakub Jelinek]

On Mon, Apr 30, 2001 at 08:52:14PM +0200, Andreas Jaeger wrote:
> 
> Hi glibc folks,
> 
> please comment on the appended bug report.  Should we really change
> _pthread_fastlock to:

If I'm not wrong, his report is against glibc 2.2.1, I remember fixing this
since then, see
http://sources.redhat.com/ml/libc-hacker/2001-02/msg00111.html
and the following thread.

	Jakub

error in sys/utsname.h prevents compiling mingetty 0.9.4

[Chmouel Boudjnah]

Hi,

on :
https://qa.mandrakesoft.com/cgi-bin/show_bug.cgi?id=1924

a user tell us  :

--=-=-=
in file /usr/include/sys/utsname.h

the following line is incorrect
#if _UTSNAME_DOMAIN_LENGTH - 0

should be
#if _UTSNAME_DOMAIN_LENGTH != 0

or

#if _UTSNAME_DOMAIN_LENGTH > 0
--=-=-=

which prevent compiling mingetty, is it a right fix or app is broken ?

Re: error in sys/utsname.h prevents compiling mingetty 0.9.4

[Andreas Jaeger]

Chmouel Boudjnah <chmouel@mandrakesoft.com> writes:

> Hi,
> 
> on :
> https://qa.mandrakesoft.com/cgi-bin/show_bug.cgi?id=1924
> 
> a user tell us  :
> 
> --=-=-=
> in file /usr/include/sys/utsname.h
> 
> the following line is incorrect
> #if _UTSNAME_DOMAIN_LENGTH - 0
> 
> should be
> #if _UTSNAME_DOMAIN_LENGTH != 0
> 
> or
> 
> #if _UTSNAME_DOMAIN_LENGTH > 0
> --=-=-=
> 
> which prevent compiling mingetty, is it a right fix or app is broken ?

Looks like a broken app or compiler,

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

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

[Kaz Kylheku]

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" );
	}
    }

Linuxthreads memory barrier bugs.

[Kaz Kylheku]

Alexander <TEREKHOV@de.ibm.com> just wrote me an e-mail about what looks
like a bug in pthread_once().  There is a READ_MEMORY_BARRIER()
missing in the case when the function bails when
it sees (*once == DONE).

Also Alexander pointed out that there is no memory barrier after
assignment of the DONE value to *once.   Now I did a code inspection of
pthread_once a long time ago specifically with regard to this point,
and verified that there was a memory barrier performed in
__pthread_lock(). However, Alexander's e-mail prompted me to redo
this inspection, and it turns out that __pthread_lock()
can now bail out without executing a memory barrier, if the caller
grabs the lock in the spin loop. (Another bug I introduced).

I think that the correct fix is to add the READ_MEMORY_BARRIER() and
WRITE_MEMORY_BARRIER() in the right places in pthread_once, and to add
a READ_MEMORY_BARRIER() before the return statement in the
__pthread_lock spin loop.

The rationale is that a read barrier is sufficient on entry to a
critical region, and pthread_once should take care of its own memory
consistency assumptions rather than assume that pthread_mutex_lock
provides a full write barrier.

I'm going to send a concrete patch in a subsequent e-mail.