Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 23 Apr 2010 17:15:45 GMT
From:      Kostik Belousov <kostikbel@gmail.com>
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/131597: [kernel] c++ exceptions very slow on FreeBSD 7.1/amd64
Message-ID:  <201004231715.o3NHFj90083731@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/131597; it has been noted by GNATS.

From: Kostik Belousov <kostikbel@gmail.com>
To: John Baldwin <jhb@freebsd.org>
Cc: bug-followup@freebsd.org, guillaume@morinfr.org, kan@freebsd.org,
        davidxu@freebsd.org
Subject: Re: kern/131597: [kernel] c++ exceptions very slow on FreeBSD 7.1/amd64
Date: Fri, 23 Apr 2010 15:25:01 +0300

 --kG2acDqmwoBDcCHP
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable
 
 On Thu, Apr 22, 2010 at 04:09:34PM -0400, John Baldwin wrote:
 > I tracked the sigprocmask() system calls down to the operations to
 > acquire a write lock in the runtime linker. The lock was added to fix
 > an earlier bug with throwing exceptions in multithreaded C++ apps. The
 > relevant commit that added the lock is this:
 >
 >    http://svn.freebsd.org/viewvc/base?view=3Drevision&revision=3D178807
 >
 > Are exceptions permitted during a signal handler? If not, then in
 > theory we would not need to invoke sigprocmask() for this particular
 > lock perhaps? I'm not sure how easy that would be to achieve given the
 > hooks to allow the thread library to overload the locking routines.
 > Also, this doesn't explain the lack of sigprocmask() calls under i386.
 > FreeBSD/i386 should be using the same locking code and thus invoking
 > sigprocmask() for each exception as well.
 
 Throwing an exception during asyncronous signal execution rises undefined
 behaviour, AFAIK. sigprocmask() is there to support libc_r, and cannot
 be removed as far as we need to provide FreeBSD 4.x compatibility.
 
 What can be done is to provide completely dummy implementation of rtld
 locks by the modern libc. Fortunately, libthr only injects its rtld
 locks implementation into rtld on first thread creation. The simple
 stack of RtldLockInfo seems to give us proper restoration to the libc
 provided locks instead of default locks when process is back to
 single-thread.
 
 The prototype is below. It does not work for static linking, and this is
 the first usage of __attribute__((constructor)), at least in libc.
 Alexander, I do remember about -DDEBUG in rtld-elf/Makefile.
 
 diff --git a/lib/libc/Makefile b/lib/libc/Makefile
 index b58b6cb..be41c1c 100644
 --- a/lib/libc/Makefile
 +++ b/lib/libc/Makefile
 @@ -16,6 +16,8 @@ SHLIB_MAJOR=3D 7
  WARNS?=3D	2
  CFLAGS+=3D-I${.CURDIR}/include -I${.CURDIR}/../../include
  CFLAGS+=3D-I${.CURDIR}/${MACHINE_ARCH}
 +CFLAGS+=3D-I${.CURDIR}/../../libexec/rtld-elf
 +CFLAGS+=3D-I${.CURDIR}/../../libexec/rtld-elf/${MACHINE_ARCH}
  CFLAGS+=3D-DNLS
  CLEANFILES+=3Dtags
  INSTALL_PIC_ARCHIVE=3D
 diff --git a/lib/libc/gen/Makefile.inc b/lib/libc/gen/Makefile.inc
 index 2f562da..fadf339 100644
 --- a/lib/libc/gen/Makefile.inc
 +++ b/lib/libc/gen/Makefile.inc
 @@ -10,7 +10,7 @@ SRCS+=3D  __getosreldate.c __xuname.c \
  	alarm.c arc4random.c assert.c basename.c check_utility_compat.c \
  	clock.c closedir.c confstr.c \
  	crypt.c ctermid.c daemon.c devname.c dirname.c disklabel.c \
 -	dlfcn.c drand48.c erand48.c err.c errlst.c errno.c \
 +	dlfcn.c dllock.c drand48.c erand48.c err.c errlst.c errno.c \
  	exec.c fdevname.c feature_present.c fmtcheck.c fmtmsg.c fnmatch.c \
  	fpclassify.c frexp.c fstab.c ftok.c fts.c fts-compat.c ftw.c \
  	getbootfile.c getbsize.c \
 diff --git a/lib/libc/gen/dllock.c b/lib/libc/gen/dllock.c
 new file mode 100644
 index 0000000..0980147
 --- /dev/null
 +++ b/lib/libc/gen/dllock.c
 @@ -0,0 +1,107 @@
 +/*-
 + * Copyright (c) 2010 Konstantin Belousov
 + * All rights reserved.
 + *
 + * Redistribution and use in source and binary forms, with or without
 + * modification, are permitted provided that the following conditions
 + * are met:
 + * 1. Redistributions of source code must retain the above copyright
 + *    notice, this list of conditions and the following disclaimer.
 + * 2. Redistributions in binary form must reproduce the above copyright
 + *    notice, this list of conditions and the following disclaimer in the
 + *    documentation and/or other materials provided with the distribution.
 + *
 + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
 + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURP=
 OSE
 + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
 + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENT=
 IAL
 + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STR=
 ICT
 + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY W=
 AY
 + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 + * SUCH DAMAGE.
 + */
 +
 +#include <sys/cdefs.h>
 +__FBSDID("$FreeBSD$");
 +
 +#include <sys/types.h>
 +#include <machine/atomic.h>
 +#include <stdlib.h>
 +
 +#include "rtld_lock.h"
 +
 +static void *
 +_dummy_dl_lock_create(void)
 +{
 +
 +	return ((void *)1);
 +}
 +
 +static void
 +_dummy_dl_lock_destroy(void *lock __unused)
 +{
 +}
 +
 +static void
 +_dummy_dl_rlock_acquire(void *lock __unused)
 +{
 +}
 +
 +static void
 +_dummy_dl_wlock_acquire(void *lock __unused)
 +{
 +}
 +
 +static void
 +_dummy_dl_lock_release(void *lock __unused)
 +{
 +}
 +
 +static int _dummy_dl_mask;
 +
 +static int
 +_dummy_dl_set_flag(int mask)
 +{
 +	int old, new;
 +
 +	do {
 +		old =3D _dummy_dl_mask;
 +		new =3D old | mask;
 +	} while (!atomic_cmpset_acq_int(&_dummy_dl_mask, old, new));
 +	return (old);
 +}
 +
 +static int
 +_dummy_dl_clr_flag(int mask __unused)
 +{
 +
 +	int old, new;
 +
 +	do {
 +		old =3D _dummy_dl_mask;
 +		new =3D old & (~mask);
 +	} while (!atomic_cmpset_rel_int(&_dummy_dl_mask, old, new));
 +	return (old);
 +}
 +
 +static void _dllock_init(void) __attribute__((constructor));
 +static void
 +_dllock_init(void)
 +{
 +	struct RtldLockInfo li;
 +
 +	li.lock_create  =3D _dummy_dl_lock_create;
 +	li.lock_destroy =3D _dummy_dl_lock_destroy;
 +	li.rlock_acquire =3D _dummy_dl_rlock_acquire;
 +	li.wlock_acquire =3D _dummy_dl_wlock_acquire;
 +	li.lock_release  =3D _dummy_dl_lock_release;
 +	li.thread_set_flag =3D _dummy_dl_set_flag;
 +	li.thread_clr_flag =3D _dummy_dl_clr_flag;
 +	li.at_fork =3D NULL;
 +
 +	_rtld_thread_init(&li);
 +}
 +
 diff --git a/libexec/rtld-elf/Makefile b/libexec/rtld-elf/Makefile
 index d6df617..d451681 100644
 --- a/libexec/rtld-elf/Makefile
 +++ b/libexec/rtld-elf/Makefile
 @@ -11,6 +11,7 @@ MAN=3D		rtld.1
  CSTD?=3D		gnu99
  CFLAGS+=3D	-Wall -DFREEBSD_ELF -DIN_RTLD
  CFLAGS+=3D	-I${.CURDIR}/${MACHINE_ARCH} -I${.CURDIR}
 +CFLAGS+=3D	-g -DDEBUG
  LDFLAGS+=3D	-nostdlib -e .rtld_start
  WARNS?=3D		2
  INSTALLFLAGS=3D	-C -b
 diff --git a/libexec/rtld-elf/rtld_lock.c b/libexec/rtld-elf/rtld_lock.c
 index c5e582e..5c8be68 100644
 --- a/libexec/rtld-elf/rtld_lock.c
 +++ b/libexec/rtld-elf/rtld_lock.c
 @@ -158,19 +158,30 @@ def_thread_clr_flag(int mask)
  /*
   * Public interface exposed to the rest of the dynamic linker.
   */
 -static struct RtldLockInfo lockinfo;
 +static struct RtldLockInfo pli_stack[8];
 +static int pli_current_idx =3D -1;
 +
 +static struct RtldLockInfo *
 +lockinfo(void)
 +{
 +
 +	if (pli_current_idx =3D=3D -1)
 +		abort();
 +	return (&pli_stack[pli_current_idx]);
 +}
 +
  static struct RtldLockInfo deflockinfo;
 =20
  static __inline int
  thread_mask_set(int mask)
  {
 -	return lockinfo.thread_set_flag(mask);
 +	return lockinfo()->thread_set_flag(mask);
  }
 =20
  static __inline void
  thread_mask_clear(int mask)
  {
 -	lockinfo.thread_clr_flag(mask);
 +	lockinfo()->thread_clr_flag(mask);
  }
 =20
  #define	RTLD_LOCK_CNT	3
 @@ -190,7 +201,7 @@ rlock_acquire(rtld_lock_t lock)
  	    dbg("rlock_acquire: recursed");
  	    return (0);
  	}
 -	lockinfo.rlock_acquire(lock->handle);
 +	lockinfo()->rlock_acquire(lock->handle);
  	return (1);
  }
 =20
 @@ -201,7 +212,7 @@ wlock_acquire(rtld_lock_t lock)
  	    dbg("wlock_acquire: recursed");
  	    return (0);
  	}
 -	lockinfo.wlock_acquire(lock->handle);
 +	lockinfo()->wlock_acquire(lock->handle);
  	return (1);
  }
 =20
 @@ -211,7 +222,7 @@ rlock_release(rtld_lock_t lock, int locked)
  	if (locked =3D=3D 0)
  	    return;
  	thread_mask_clear(lock->mask);
 -	lockinfo.lock_release(lock->handle);
 +	lockinfo()->lock_release(lock->handle);
  }
 =20
  void
 @@ -220,7 +231,7 @@ wlock_release(rtld_lock_t lock, int locked)
  	if (locked =3D=3D 0)
  	    return;
  	thread_mask_clear(lock->mask);
 -	lockinfo.lock_release(lock->handle);
 +	lockinfo()->lock_release(lock->handle);
  }
 =20
  void
 @@ -243,7 +254,6 @@ lockdflt_init()
  	    rtld_locks[i].handle =3D NULL;
      }
 =20
 -    memcpy(&lockinfo, &deflockinfo, sizeof(lockinfo));
      _rtld_thread_init(NULL);
      /*
       * Construct a mask to block all signals except traps which might
 @@ -272,13 +282,33 @@ _rtld_thread_init(struct RtldLockInfo *pli)
  {
  	int flags, i;
  	void *locks[RTLD_LOCK_CNT];
 +	struct RtldLockInfo *prev_pli;
 =20
  	/* disable all locking while this function is running */
 -	flags =3D	thread_mask_set(~0);
 -
 -	if (pli =3D=3D NULL)
 -		pli =3D &deflockinfo;
 -
 +	if (pli =3D=3D NULL && pli_current_idx =3D=3D -1)
 +		flags =3D def_thread_set_flag(~0);
 +	else
 +		flags =3D	thread_mask_set(~0);
 +
 +	if (pli =3D=3D NULL) {
 +		if (pli_current_idx =3D=3D -1) {
 +			pli_current_idx =3D 0;
 +			pli_stack[pli_current_idx] =3D deflockinfo;
 +			pli =3D &pli_stack[pli_current_idx];
 +			prev_pli =3D NULL;
 +		} else {
 +			prev_pli =3D &pli_stack[pli_current_idx];
 +			pli =3D &pli_stack[pli_current_idx--];
 +			if (pli_current_idx =3D=3D -1)
 +				abort();
 +		}
 +	} else {
 +		prev_pli =3D &pli_stack[pli_current_idx];
 +		if (++pli_current_idx >=3D
 +		    sizeof(pli_stack) / sizeof(pli_stack[0]))
 +			abort();
 +		pli_stack[pli_current_idx] =3D *pli;
 +	}
 =20
  	for (i =3D 0; i < RTLD_LOCK_CNT; i++)
  		if ((locks[i] =3D pli->lock_create()) =3D=3D NULL)
 @@ -290,12 +320,14 @@ _rtld_thread_init(struct RtldLockInfo *pli)
  		abort();
  	}
 =20
 -	for (i =3D 0; i < RTLD_LOCK_CNT; i++) {
 -		if (rtld_locks[i].handle =3D=3D NULL)
 -			continue;
 -		if (flags & rtld_locks[i].mask)
 -			lockinfo.lock_release(rtld_locks[i].handle);
 -		lockinfo.lock_destroy(rtld_locks[i].handle);
 +	if (prev_pli !=3D NULL) {
 +		for (i =3D 0; i < RTLD_LOCK_CNT; i++) {
 +			if (rtld_locks[i].handle =3D=3D NULL)
 +				continue;
 +			if (flags & rtld_locks[i].mask)
 +				prev_pli->lock_release(rtld_locks[i].handle);
 +			prev_pli->lock_destroy(rtld_locks[i].handle);
 +		}
  	}
 =20
  	for (i =3D 0; i < RTLD_LOCK_CNT; i++) {
 @@ -304,15 +336,6 @@ _rtld_thread_init(struct RtldLockInfo *pli)
  			pli->wlock_acquire(rtld_locks[i].handle);
  	}
 =20
 -	lockinfo.lock_create =3D pli->lock_create;
 -	lockinfo.lock_destroy =3D pli->lock_destroy;
 -	lockinfo.rlock_acquire =3D pli->rlock_acquire;
 -	lockinfo.wlock_acquire =3D pli->wlock_acquire;
 -	lockinfo.lock_release  =3D pli->lock_release;
 -	lockinfo.thread_set_flag =3D pli->thread_set_flag;
 -	lockinfo.thread_clr_flag =3D pli->thread_clr_flag;
 -	lockinfo.at_fork =3D pli->at_fork;
 -
  	/* restore thread locking state, this time with new locks */
  	thread_mask_clear(~0);
  	thread_mask_set(flags);
 
 --kG2acDqmwoBDcCHP
 Content-Type: application/pgp-signature
 Content-Disposition: inline
 
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.10 (FreeBSD)
 
 iEYEARECAAYFAkvRkZ0ACgkQC3+MBN1Mb4j5cACeNiBETt3fYfRk9AW5sEndBmjd
 U3AAoO6LQenG9fp1XjBTLJMrSDdC/a2Z
 =8mxC
 -----END PGP SIGNATURE-----
 
 --kG2acDqmwoBDcCHP--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201004231715.o3NHFj90083731>