From owner-freebsd-hackers@FreeBSD.ORG Fri Jul 30 14:10:04 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 974D71065672; Fri, 30 Jul 2010 14:10:04 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 4F9E48FC0C; Fri, 30 Jul 2010 14:10:04 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id BCBEA46B2C; Fri, 30 Jul 2010 10:10:03 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id BC5438A03C; Fri, 30 Jul 2010 10:10:02 -0400 (EDT) From: John Baldwin To: freebsd-hackers@freebsd.org Date: Fri, 30 Jul 2010 10:08:22 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100217; KDE/4.4.5; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <201007301008.22501.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Fri, 30 Jul 2010 10:10:02 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: mdf@freebsd.org Subject: Re: sched_pin() versus PCPU_GET X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Jul 2010 14:10:04 -0000 On Thursday, July 29, 2010 7:39:02 pm mdf@freebsd.org wrote: > We've seen a few instances at work where witness_warn() in ast() > indicates the sched lock is still held, but the place it claims it was > held by is in fact sometimes not possible to keep the lock, like: >=20 > thread_lock(td); > td->td_flags &=3D ~TDF_SELECT; > thread_unlock(td); >=20 > What I was wondering is, even though the assembly I see in objdump -S > for witness_warn has the increment of td_pinned before the PCPU_GET: >=20 > ffffffff802db210: 65 48 8b 1c 25 00 00 mov %gs:0x0,%rbx > ffffffff802db217: 00 00 > ffffffff802db219: ff 83 04 01 00 00 incl 0x104(%rbx) > * Pin the thread in order to avoid problems with thread migration. > * Once that all verifies are passed about spinlocks ownership, > * the thread is in a safe path and it can be unpinned. > */ > sched_pin(); > lock_list =3D PCPU_GET(spinlocks); > ffffffff802db21f: 65 48 8b 04 25 48 00 mov %gs:0x48,%rax > ffffffff802db226: 00 00 > if (lock_list !=3D NULL && lock_list->ll_count !=3D 0) { > ffffffff802db228: 48 85 c0 test %rax,%rax > * Pin the thread in order to avoid problems with thread migration. > * Once that all verifies are passed about spinlocks ownership, > * the thread is in a safe path and it can be unpinned. > */ > sched_pin(); > lock_list =3D PCPU_GET(spinlocks); > ffffffff802db22b: 48 89 85 f0 fe ff ff mov %rax,-0x110(%rbp) > ffffffff802db232: 48 89 85 f8 fe ff ff mov %rax,-0x108(%rbp) > if (lock_list !=3D NULL && lock_list->ll_count !=3D 0) { > ffffffff802db239: 0f 84 ff 00 00 00 je ffffffff802db33e > > ffffffff802db23f: 44 8b 60 50 mov 0x50(%rax),%r12d >=20 > is it possible for the hardware to do any re-ordering here? >=20 > The reason I'm suspicious is not just that the code doesn't have a > lock leak at the indicated point, but in one instance I can see in the > dump that the lock_list local from witness_warn is from the pcpu > structure for CPU 0 (and I was warned about sched lock 0), but the > thread id in panic_cpu is 2. So clearly the thread was being migrated > right around panic time. >=20 > This is the amd64 kernel on stable/7. I'm not sure exactly what kind > of hardware; it's a 4-way Intel chip from about 3 or 4 years ago IIRC. >=20 > So... do we need some kind of barrier in the code for sched_pin() for > it to really do what it claims? Could the hardware have re-ordered > the "mov %gs:0x48,%rax" PCPU_GET to before the sched_pin() > increment? Hmmm, I think it might be able to because they refer to different locations. Note this rule in section 8.2.2 of Volume 3A: =E2=80=A2 Reads may be reordered with older writes to different locations= but not with older writes to the same location. It is certainly true that sparc64 could reorder with RMO. I believe ia64=20 could reorder as well. Since sched_pin/unpin are frequently used to provid= e=20 this sort of synchronization, we could use memory barriers in pin/unpin like so: sched_pin() { td->td_pinned =3D atomic_load_acq_int(&td->td_pinned) + 1; } sched_unpin() { atomic_store_rel_int(&td->td_pinned, td->td_pinned - 1); } We could also just use atomic_add_acq_int() and atomic_sub_rel_int(), but t= hey=20 are slightly more heavyweight, though it would be more clear what is happen= ing=20 I think. =2D-=20 John Baldwin