Date: Thu, 27 Jun 2013 07:27:08 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r252292 - stable/9/sys/kern Message-ID: <201306270727.r5R7R8oB043923@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kib Date: Thu Jun 27 07:27:08 2013 New Revision: 252292 URL: http://svnweb.freebsd.org/changeset/base/252292 Log: MFC r251684: Fix two issues with the spin loops in the umtx(2) implementation. - When looping, check for the pending suspension. - Add missed checks for the faults from casuword*(). Modified: stable/9/sys/kern/kern_umtx.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/kern/kern_umtx.c ============================================================================== --- stable/9/sys/kern/kern_umtx.c Thu Jun 27 06:57:09 2013 (r252291) +++ stable/9/sys/kern/kern_umtx.c Thu Jun 27 07:27:08 2013 (r252292) @@ -505,6 +505,32 @@ umtxq_count_pi(struct umtx_key *key, str return (0); } +static int +umtxq_check_susp(struct thread *td) +{ + struct proc *p; + int error; + + /* + * The check for TDF_NEEDSUSPCHK is racy, but it is enough to + * eventually break the lockstep loop. + */ + if ((td->td_flags & TDF_NEEDSUSPCHK) == 0) + return (0); + error = 0; + p = td->td_proc; + PROC_LOCK(p); + if (P_SHOULDSTOP(p) || + ((p->p_flag & P_TRACED) && (td->td_dbgflags & TDB_SUSPEND))) { + if (p->p_flag & P_SINGLE_EXIT) + error = EINTR; + else + error = ERESTART; + } + PROC_UNLOCK(p); + return (error); +} + /* * Wake up threads waiting on an userland object. */ @@ -666,6 +692,10 @@ _do_lock_umtx(struct thread *td, struct if (owner == -1) return (EFAULT); + error = umtxq_check_susp(td); + if (error != 0) + break; + /* If this failed the lock has changed, restart. */ continue; } @@ -715,6 +745,9 @@ _do_lock_umtx(struct thread *td, struct umtxq_remove(uq); umtxq_unlock(&uq->uq_key); umtx_key_release(&uq->uq_key); + + if (error == 0) + error = umtxq_check_susp(td); } return (0); @@ -865,6 +898,10 @@ _do_lock_umtx32(struct thread *td, uint3 if (owner == -1) return (EFAULT); + error = umtxq_check_susp(td); + if (error != 0) + break; + /* If this failed the lock has changed, restart. */ continue; } @@ -914,6 +951,9 @@ _do_lock_umtx32(struct thread *td, uint3 umtxq_remove(uq); umtxq_unlock(&uq->uq_key); umtx_key_release(&uq->uq_key); + + if (error == 0) + error = umtxq_check_susp(td); } return (0); @@ -1157,6 +1197,10 @@ _do_lock_normal(struct thread *td, struc if (owner == -1) return (EFAULT); + error = umtxq_check_susp(td); + if (error != 0) + return (error); + /* If this failed the lock has changed, restart. */ continue; } @@ -1215,6 +1259,9 @@ _do_lock_normal(struct thread *td, struc umtxq_remove(uq); umtxq_unlock(&uq->uq_key); umtx_key_release(&uq->uq_key); + + if (error == 0) + error = umtxq_check_susp(td); } return (0); @@ -1374,6 +1421,11 @@ do_wake2_umutex(struct thread *td, struc if (old == owner) break; owner = old; + if (old == -1) + break; + error = umtxq_check_susp(td); + if (error != 0) + break; } } else if (count == 1) { owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner)); @@ -1384,6 +1436,11 @@ do_wake2_umutex(struct thread *td, struc if (old == owner) break; owner = old; + if (old == -1) + break; + error = umtxq_check_susp(td); + if (error != 0) + break; } } umtxq_lock(&key); @@ -1848,6 +1905,10 @@ _do_lock_pi(struct thread *td, struct um break; } + error = umtxq_check_susp(td); + if (error != 0) + break; + /* If this failed the lock has changed, restart. */ continue; } @@ -1904,6 +1965,10 @@ _do_lock_pi(struct thread *td, struct um umtxq_unbusy(&uq->uq_key); umtxq_unlock(&uq->uq_key); } + + error = umtxq_check_susp(td); + if (error != 0) + break; } umtxq_lock(&uq->uq_key); @@ -2585,10 +2650,17 @@ do_rw_rdlock(struct thread *td, struct u return (EAGAIN); } oldstate = casuword32(&rwlock->rw_state, state, state + 1); + if (oldstate == -1) { + umtx_key_release(&uq->uq_key); + return (EFAULT); + } if (oldstate == state) { umtx_key_release(&uq->uq_key); return (0); } + error = umtxq_check_susp(td); + if (error != 0) + break; state = oldstate; } @@ -2609,9 +2681,22 @@ do_rw_rdlock(struct thread *td, struct u /* set read contention bit */ while ((state & wrflags) && !(state & URWLOCK_READ_WAITERS)) { oldstate = casuword32(&rwlock->rw_state, state, state | URWLOCK_READ_WAITERS); + if (oldstate == -1) { + error = EFAULT; + break; + } if (oldstate == state) goto sleep; state = oldstate; + error = umtxq_check_susp(td); + if (error != 0) + break; + } + if (error != 0) { + umtxq_lock(&uq->uq_key); + umtxq_unbusy(&uq->uq_key); + umtxq_unlock(&uq->uq_key); + break; } /* state is changed while setting flags, restart */ @@ -2619,6 +2704,9 @@ do_rw_rdlock(struct thread *td, struct u umtxq_lock(&uq->uq_key); umtxq_unbusy(&uq->uq_key); umtxq_unlock(&uq->uq_key); + error = umtxq_check_susp(td); + if (error != 0) + break; continue; } @@ -2650,15 +2738,24 @@ sleep: for (;;) { oldstate = casuword32(&rwlock->rw_state, state, state & ~URWLOCK_READ_WAITERS); + if (oldstate == -1) { + error = EFAULT; + break; + } if (oldstate == state) break; state = oldstate; + error = umtxq_check_susp(td); + if (error != 0) + break; } } umtxq_lock(&uq->uq_key); umtxq_unbusy(&uq->uq_key); umtxq_unlock(&uq->uq_key); + if (error != 0) + break; } umtx_key_release(&uq->uq_key); return (error); @@ -2713,11 +2810,18 @@ do_rw_wrlock(struct thread *td, struct u state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state)); while (!(state & URWLOCK_WRITE_OWNER) && URWLOCK_READER_COUNT(state) == 0) { oldstate = casuword32(&rwlock->rw_state, state, state | URWLOCK_WRITE_OWNER); + if (oldstate == -1) { + umtx_key_release(&uq->uq_key); + return (EFAULT); + } if (oldstate == state) { umtx_key_release(&uq->uq_key); return (0); } state = oldstate; + error = umtxq_check_susp(td); + if (error != 0) + break; } if (error) { @@ -2747,15 +2851,31 @@ do_rw_wrlock(struct thread *td, struct u while (((state & URWLOCK_WRITE_OWNER) || URWLOCK_READER_COUNT(state) != 0) && (state & URWLOCK_WRITE_WAITERS) == 0) { oldstate = casuword32(&rwlock->rw_state, state, state | URWLOCK_WRITE_WAITERS); + if (oldstate == -1) { + error = EFAULT; + break; + } if (oldstate == state) goto sleep; state = oldstate; + error = umtxq_check_susp(td); + if (error != 0) + break; + } + if (error != 0) { + umtxq_lock(&uq->uq_key); + umtxq_unbusy(&uq->uq_key); + umtxq_unlock(&uq->uq_key); + break; } if (!(state & URWLOCK_WRITE_OWNER) && URWLOCK_READER_COUNT(state) == 0) { umtxq_lock(&uq->uq_key); umtxq_unbusy(&uq->uq_key); umtxq_unlock(&uq->uq_key); + error = umtxq_check_susp(td); + if (error != 0) + break; continue; } sleep: @@ -2784,9 +2904,21 @@ sleep: for (;;) { oldstate = casuword32(&rwlock->rw_state, state, state & ~URWLOCK_WRITE_WAITERS); + if (oldstate == -1) { + error = EFAULT; + break; + } if (oldstate == state) break; state = oldstate; + error = umtxq_check_susp(td); + /* + * We are leaving the URWLOCK_WRITE_WAITERS + * behind, but this should not harm the + * correctness. + */ + if (error != 0) + break; } blocked_readers = fuword32(&rwlock->rw_blocked_readers); } else @@ -2848,12 +2980,19 @@ do_rw_unlock(struct thread *td, struct u for (;;) { oldstate = casuword32(&rwlock->rw_state, state, state & ~URWLOCK_WRITE_OWNER); + if (oldstate == -1) { + error = EFAULT; + goto out; + } if (oldstate != state) { state = oldstate; if (!(oldstate & URWLOCK_WRITE_OWNER)) { error = EPERM; goto out; } + error = umtxq_check_susp(td); + if (error != 0) + goto out; } else break; } @@ -2861,14 +3000,20 @@ do_rw_unlock(struct thread *td, struct u for (;;) { oldstate = casuword32(&rwlock->rw_state, state, state - 1); + if (oldstate == -1) { + error = EFAULT; + goto out; + } if (oldstate != state) { state = oldstate; if (URWLOCK_READER_COUNT(oldstate) == 0) { error = EPERM; goto out; } - } - else + error = umtxq_check_susp(td); + if (error != 0) + goto out; + } else break; } } else {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201306270727.r5R7R8oB043923>