[parisc-linux] [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[parisc-linux] [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[Carlos O'Donell]

libc,

Found the problem with bug-iconv3, it was related to the fact that
hppa's locks can't be initialized to zero (or left unitialized). When
running a non-threaded application and dlopening libpthread the
__libc_maybe_call2's start using the proper strong symbol of those
funcions. At this point malloc on hppa would in certain occassions
fget stuck in a lock that it thought was taken (value of zero).

The following patch fixes the issue, but I'm not content about it's
placement in malloc/thread-m.h. Any comments about where I might put
this in order to make maintenance easier?

Cheers,
Carlos.

diff -u -p -r1.23 thread-m.h
--- glibc/malloc/thread-m.h	1 Jul 2003 08:29:43 -0000	1.23
+++ glibc/malloc/thread-m.h	25 Sep 2003 20:43:55 -0000
@@ -59,6 +59,28 @@ __libc_lock_define (typedef, mutex_t)
 #define mutex_unlock(m)		\
   __libc_maybe_call2 (pthread_mutex_unlock, (m), (*(int *)(m) = 0))
 
+# if(defined __hppa__)
+/* Since our lock structure does not tolerate being initialized to zero, we must
+   modify the standard function calls made by malloc */
+#  undef mutex_init
+#  undef mutex_lock
+#  undef mutex_trylock
+#  undef mutex_unlock
+#  define mutex_init(m)		\
+	__libc_maybe_call (__pthread_mutex_init, (m, NULL), \
+			(((m)->__m_lock.__spinlock = __LT_SPINLOCK_INIT),(*(int *)(m))) )
+#  define mutex_lock(m)		\
+	__libc_maybe_call (__pthread_mutex_lock, (m), \
+			(__load_and_clear(&((m)->__m_lock.__spinlock)), 0))
+#  define mutex_trylock(m)	\
+	__libc_maybe_call (__pthread_mutex_trylock, (m), \
+			(*(int *)(m) ? 1 : (__load_and_clear(&((m)->__m_lock.__spinlock)), 0)))
+#  define mutex_unlock(m)	\
+	__libc_maybe_call (__pthread_mutex_unlock, (m), \
+			(((m)->__m_lock.__spinlock = __LT_SPINLOCK_INIT), (*(int *)(m))) )
+# endif 
+/* if(defined __hppa__) */
+	
 #else
 
 #define mutex_init(m)		\

[parisc-linux] Re: [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[wmglo]

Hi,

> The following patch fixes the issue, but I'm not content about it's
> placement in malloc/thread-m.h. Any comments about where I might put
> this in order to make maintenance easier?

Ok, I have become convinced it is best to move thread-m.h into
sysdeps.  We should rename it, too (my suggestion: malloc-machine.h),
and make sure that one can put e.g.

#define DEFAULT_MMAP_THRESHOLD (256*1024)

in there and have it reflected in malloc.

I'll send a patch for this tomorrow.

Regards,
Wolfram.

[parisc-linux] Re: [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[wmglo]

Hi,

> The following patch fixes the issue, but I'm not content about it's
> placement in malloc/thread-m.h. Any comments about where I might put
> this in order to make maintenance easier?

Ok, I have become convinced it is best to move thread-m.h into
sysdeps.  We should rename it, too (my suggestion: malloc-machine.h),
and make sure that one can put e.g.

#define DEFAULT_MMAP_THRESHOLD (256*1024)

in there and have it reflected in malloc.

I'll send a patch for this tomorrow.

Regards,
Wolfram.

[parisc-linux] Re: [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[Carlos O'Donell]

On Fri, Sep 26, 2003 at 01:39:54PM -0000, wmglo@dent.med.uni-muenchen.de wrote:
> > The following patch fixes the issue, but I'm not content about it's
> > placement in malloc/thread-m.h. Any comments about where I might put
> > this in order to make maintenance easier?
> 
> Ok, I have become convinced it is best to move thread-m.h into
> sysdeps.  We should rename it, too (my suggestion: malloc-machine.h),
> and make sure that one can put e.g.
> 
> #define DEFAULT_MMAP_THRESHOLD (256*1024)
> 
> in there and have it reflected in malloc.
> 
> I'll send a patch for this tomorrow.

Thank you. Sounds like the best solution in this case.

c.

[parisc-linux] Re: [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[Carlos O'Donell]

On Fri, Sep 26, 2003 at 01:39:54PM -0000, wmglo@dent.med.uni-muenchen.de wrote:
> > The following patch fixes the issue, but I'm not content about it's
> > placement in malloc/thread-m.h. Any comments about where I might put
> > this in order to make maintenance easier?
> 
> Ok, I have become convinced it is best to move thread-m.h into
> sysdeps.  We should rename it, too (my suggestion: malloc-machine.h),
> and make sure that one can put e.g.
> 
> #define DEFAULT_MMAP_THRESHOLD (256*1024)
> 
> in there and have it reflected in malloc.
> 
> I'll send a patch for this tomorrow.

Thank you. Sounds like the best solution in this case.

c.

[parisc-linux] Re: [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[Wolfram Gloger]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

Hi,

>The following patch fixes the issue, but I'm not content about it's
>placement in malloc/thread-m.h. Any comments about where I might put
>this in order to make maintenance easier?

The malloc-machine.h specific headers are now in place,
so you might want to propose something like the following?

Regards,
Wolfram.

2003-10-10  Carlos O'Donell  <carlos@baldric.uwo.ca>

	* sysdeps/hppa/malloc-machine.h: New file.

[-- Attachment #2: malloc-machine.h --]
[-- Type: application/octet-stream, Size: 2774 bytes --]

/* Basic platform-independent macro definitions for mutexes,
   thread-specific data and parameters for malloc.
   Copyright (C) 2003 Free Software Foundation, Inc.
   This file is part of the GNU C Library.

   The GNU C Library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
   License as published by the Free Software Foundation; either
   version 2.1 of the License, or (at your option) any later version.

   The GNU C Library is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
   Lesser General Public License for more details.

   You should have received a copy of the GNU Lesser General Public
   License along with the GNU C Library; if not, write to the Free
   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
   02111-1307 USA.  */

#ifndef _MALLOC_MACHINE_H
#define _MALLOC_MACHINE_H

#undef thread_atfork_static

#include <atomic.h>
#include <bits/libc-lock.h>

__libc_lock_define (typedef, mutex_t)

/* Since our lock structure does not tolerate being initialized to
   zero, we must modify the standard function calls made by malloc */

#define mutex_init(m)		\
	__libc_maybe_call (__pthread_mutex_init, (m, NULL), \
			(((m)->__m_lock.__spinlock = __LT_SPINLOCK_INIT),(*(int *)(m))) )
#define mutex_lock(m)		\
	__libc_maybe_call (__pthread_mutex_lock, (m), \
			(__load_and_clear(&((m)->__m_lock.__spinlock)), 0))
#define mutex_trylock(m)	\
	__libc_maybe_call (__pthread_mutex_trylock, (m), \
			(*(int *)(m) ? 1 : (__load_and_clear(&((m)->__m_lock.__spinlock)), 0)))
#define mutex_unlock(m)	\
	__libc_maybe_call (__pthread_mutex_unlock, (m), \
			(((m)->__m_lock.__spinlock = __LT_SPINLOCK_INIT), (*(int *)(m))) )

/* This is defined by newer gcc version unique for each module.  */
extern void *__dso_handle __attribute__ ((__weak__));

#include <fork.h>

#ifdef SHARED
# define thread_atfork(prepare, parent, child) \
   __register_atfork (prepare, parent, child, __dso_handle)
#else
# define thread_atfork(prepare, parent, child) \
   __register_atfork (prepare, parent, child,				      \
		      &__dso_handle == NULL ? NULL : __dso_handle)
#endif

/* thread specific data for glibc */

#include <bits/libc-tsd.h>

typedef int tsd_key_t[1];	/* no key data structure, libc magic does it */
__libc_tsd_define (static, MALLOC)	/* declaration/common definition */
#define tsd_key_create(key, destr)	((void) (key))
#define tsd_setspecific(key, data)	__libc_tsd_set (MALLOC, (data))
#define tsd_getspecific(key, vptr)	((vptr) = __libc_tsd_get (MALLOC))

#include <sysdeps/generic/malloc-machine.h>

#endif /* !defined(_MALLOC_MACHINE_H) */

[parisc-linux] Re: [RFC] Where to put arch-dependant locking in malloc/thread-m.h

[Carlos O'Donell]

On Fri, Oct 10, 2003 at 12:22:06PM -0000, Wolfram Gloger wrote:
> The malloc-machine.h specific headers are now in place,
> so you might want to propose something like the following?
> 
> Regards,
> Wolfram.

Many thanks Wolfram! I've just gotten back from a trip and I had all my
patches ready to send out :)

Cheers,
Carlos.