From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22417 invoked by alias); 30 Jul 2007 18:08:28 -0000 Received: (qmail 22400 invoked by uid 22791); 30 Jul 2007 18:08:27 -0000 X-Spam-Check-By: sourceware.org Received: from sunsite.ms.mff.cuni.cz (HELO sunsite.mff.cuni.cz) (195.113.15.26) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 30 Jul 2007 18:08:22 +0000 Received: from sunsite.mff.cuni.cz (localhost.localdomain [127.0.0.1]) by sunsite.mff.cuni.cz (8.13.8/8.13.8) with ESMTP id l6UICtZJ029329; Mon, 30 Jul 2007 20:12:55 +0200 Received: (from jakub@localhost) by sunsite.mff.cuni.cz (8.13.8/8.13.8/Submit) id l6UICt8D029328; Mon, 30 Jul 2007 20:12:55 +0200 Date: Mon, 30 Jul 2007 18:08:00 -0000 From: Jakub Jelinek To: Ulrich Drepper Cc: Glibc hackers Subject: [PATCH] i[456]86 rwlock fixes Message-ID: <20070730181254.GU4603@sunsite.mff.cuni.cz> Reply-To: Jakub Jelinek Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.2.2i Mailing-List: contact libc-hacker-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sourceware.org X-SW-Source: 2007-07/txt/msg00050.txt.bz2 Hi! While working on i386 lowlevellock* updates (to match the x86_64/ppc changes I posted as RFC), I found a couple of bugs in i?86 rwlock code and I think it is better to post them separately outside of a huge patch, for backporting etc. purposes: 1) if after futex returns -ETIMEDOUT or gettimeofday comparison jumps with -ETIMEDOUT stright to label 17 the internal lock isn't acquired immediately by lock cmpxchgl (== we jump to label 12), %ecx (which holds the saved futex return value) is first overwritten by 12: #if MUTEX == 0 movl %ebp, %ecx #else leal MUTEX(%ebp), %ecx #endif and afterwards inside of __lll_mutex_lock_wait (it will be FUTEX_WAIT). This means the comparison with -ETIMEDOUT later on will be always false even if futex returned that error code and we'll loop again. The effect of this will be that pthread_rwlock_timed*lock will be looping in this case until it manages to grab the internal lock immediately without having to wait. I believe %esi isn't used for anything at this point (earlier it is used to hold some value from before unlock of the internal lock until the futex FUTEX_WAIT syscall), then to pass one of the arguments to that syscall, but then nothing until it is poped before ret. 2) only documentation issue in rwlock_unlock - the MUTEX != 0 case uses wrong base register, fortunately MUTEX = 0 branch gets it right and we know MUTEX is 0. 2007-07-30 Jakub Jelinek * sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_timedrdlock.S (pthread_rwlock_timedrdlock): Copy futex retval to %esi rather than %ecx. * sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_timedwrlock.S (pthread_rwlock_timedwrlock): Likewise. * sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_unlock.S (__pthread_rwlock_unlock): Fix MUTEX != 0 args to __lll_*. --- nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_timedrdlock.S.jj 2007-06-04 08:42:06.000000000 +0200 +++ nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_timedrdlock.S 2007-07-30 18:10:05.000000000 +0200 @@ -124,7 +124,7 @@ pthread_rwlock_timedrdlock: leal READERS_WAKEUP(%ebp), %ebx movl $SYS_futex, %eax ENTER_KERNEL - movl %eax, %ecx + movl %eax, %esi 17: /* Reget the lock. */ @@ -139,7 +139,7 @@ pthread_rwlock_timedrdlock: jnz 12f 13: subl $1, READERS_QUEUED(%ebp) - cmpl $-ETIMEDOUT, %ecx + cmpl $-ETIMEDOUT, %esi jne 2b 18: movl $ETIMEDOUT, %ecx @@ -217,7 +217,7 @@ pthread_rwlock_timedrdlock: call __lll_mutex_lock_wait jmp 13b -16: movl $-ETIMEDOUT, %ecx +16: movl $-ETIMEDOUT, %esi jmp 17b 19: movl $EINVAL, %ecx --- nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_timedwrlock.S.jj 2007-06-04 08:42:06.000000000 +0200 +++ nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_timedwrlock.S 2007-07-30 18:10:59.000000000 +0200 @@ -122,7 +122,7 @@ pthread_rwlock_timedwrlock: leal WRITERS_WAKEUP(%ebp), %ebx movl $SYS_futex, %eax ENTER_KERNEL - movl %eax, %ecx + movl %eax, %esi 17: /* Reget the lock. */ @@ -137,7 +137,7 @@ pthread_rwlock_timedwrlock: jnz 12f 13: subl $1, WRITERS_QUEUED(%ebp) - cmpl $-ETIMEDOUT, %ecx + cmpl $-ETIMEDOUT, %esi jne 2b 18: movl $ETIMEDOUT, %ecx @@ -210,7 +210,7 @@ pthread_rwlock_timedwrlock: call __lll_mutex_lock_wait jmp 13b -16: movl $-ETIMEDOUT, %ecx +16: movl $-ETIMEDOUT, %esi jmp 17b 19: movl $EINVAL, %ecx --- nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_unlock.S.jj 2007-06-04 08:42:06.000000000 +0200 +++ nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_unlock.S 2007-07-30 18:19:01.000000000 +0200 @@ -117,7 +117,7 @@ __pthread_rwlock_unlock: #if MUTEX == 0 movl %edi, %ecx #else - leal MUTEX(%edx), %ecx + leal MUTEX(%edi), %ecx #endif call __lll_mutex_lock_wait jmp 2b @@ -126,7 +126,7 @@ __pthread_rwlock_unlock: #if MUTEX == 0 movl %edi, %eax #else - leal MUTEX(%edx), %eax + leal MUTEX(%edi), %eax #endif call __lll_mutex_unlock_wake jmp 4b @@ -135,7 +135,7 @@ __pthread_rwlock_unlock: #if MUTEX == 0 movl %edi, %eax #else - leal MUTEX(%edx), %eax + leal MUTEX(%edi), %eax #endif call __lll_mutex_unlock_wake jmp 8b Jakub