From owner-freebsd-hackers@FreeBSD.ORG Fri Jul 30 09:44:18 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 E61C01065675; Fri, 30 Jul 2010 09:44:18 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 7EC948FC20; Fri, 30 Jul 2010 09:44:17 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id o6U9iDmO001589 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 30 Jul 2010 12:44:13 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id o6U9iD0M029019; Fri, 30 Jul 2010 12:44:13 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id o6U9iD0r029018; Fri, 30 Jul 2010 12:44:13 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 30 Jul 2010 12:44:13 +0300 From: Kostik Belousov To: mdf@freebsd.org Message-ID: <20100730094413.GJ22295@deviant.kiev.zoral.com.ua> References: Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="OdQvBiqfLsaeimeB" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-2.2 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_50, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-hackers@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 09:44:19 -0000 --OdQvBiqfLsaeimeB Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 29, 2010 at 04:57:25PM -0700, mdf@freebsd.org wrote: > On Thu, Jul 29, 2010 at 4:39 PM, 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: > > > > =9A =9A =9A =9Athread_lock(td); > > =9A =9A =9A =9Atd->td_flags &=3D ~TDF_SELECT; > > =9A =9A =9A =9Athread_unlock(td); > > > > 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: > > > > ffffffff802db210: =9A =9A =9A 65 48 8b 1c 25 00 00 =9A =9Amov =9A =9A%g= s:0x0,%rbx > > ffffffff802db217: =9A =9A =9A 00 00 > > ffffffff802db219: =9A =9A =9A ff 83 04 01 00 00 =9A =9A =9A incl =9A 0x= 104(%rbx) > > =9A =9A =9A =9A * Pin the thread in order to avoid problems with thread= migration. > > =9A =9A =9A =9A * Once that all verifies are passed about spinlocks own= ership, > > =9A =9A =9A =9A * the thread is in a safe path and it can be unpinned. > > =9A =9A =9A =9A */ > > =9A =9A =9A =9Asched_pin(); > > =9A =9A =9A =9Alock_list =3D PCPU_GET(spinlocks); > > ffffffff802db21f: =9A =9A =9A 65 48 8b 04 25 48 00 =9A =9Amov =9A =9A%g= s:0x48,%rax > > ffffffff802db226: =9A =9A =9A 00 00 > > =9A =9A =9A =9Aif (lock_list !=3D NULL && lock_list->ll_count !=3D 0) { > > ffffffff802db228: =9A =9A =9A 48 85 c0 =9A =9A =9A =9A =9A =9A =9A =9At= est =9A %rax,%rax > > =9A =9A =9A =9A * Pin the thread in order to avoid problems with thread= migration. > > =9A =9A =9A =9A * Once that all verifies are passed about spinlocks own= ership, > > =9A =9A =9A =9A * the thread is in a safe path and it can be unpinned. > > =9A =9A =9A =9A */ > > =9A =9A =9A =9Asched_pin(); > > =9A =9A =9A =9Alock_list =3D PCPU_GET(spinlocks); > > ffffffff802db22b: =9A =9A =9A 48 89 85 f0 fe ff ff =9A =9Amov =9A =9A%r= ax,-0x110(%rbp) > > ffffffff802db232: =9A =9A =9A 48 89 85 f8 fe ff ff =9A =9Amov =9A =9A%r= ax,-0x108(%rbp) > > =9A =9A =9A =9Aif (lock_list !=3D NULL && lock_list->ll_count !=3D 0) { > > ffffffff802db239: =9A =9A =9A 0f 84 ff 00 00 00 =9A =9A =9A je =9A =9A = ffffffff802db33e > > > > ffffffff802db23f: =9A =9A =9A 44 8b 60 50 =9A =9A =9A =9A =9A =9A mov = =9A =9A0x50(%rax),%r12d > > > > is it possible for the hardware to do any re-ordering here? > > > > 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. =9ASo clearly the thread was being migrated > > right around panic time. > > > > This is the amd64 kernel on stable/7. =9AI'm not sure exactly what kind > > of hardware; it's a 4-way Intel chip from about 3 or 4 years ago IIRC. > > > > So... do we need some kind of barrier in the code for sched_pin() for > > it to really do what it claims? =9ACould the hardware have re-ordered > > the "mov =9A =9A%gs:0x48,%rax" PCPU_GET to before the sched_pin() > > increment? >=20 > So after some research, the answer I'm getting is "maybe". What I'm > concerned about is whether the h/w reordered the read of PCPU_GET in > front of the previous store to increment td_pinned. While not an > ultimate authority, > http://en.wikipedia.org/wiki/Memory_ordering#In_SMP_microprocessor_systems > implies that stores can be reordered after loads for both Intel and > amd64 chips, which would I believe account for the behavior seen here. >=20 Am I right that you suggest that in the sequence mov %gs:0x0,%rbx [1] incl 0x104(%rbx) [2] mov %gs:0x48,%rax [3] interrupt and preemption happen between points [2] and [3] ? And the %rax value after the thread was put back onto the (different) new CPU and executed [3] was still from the old cpu' pcpu area ? I do not believe this is possible. CPU is always self-consistent. Context switch from the thread can only occur on the return from interrupt handler, in critical_exit() or such. This code is executing on the same processor, and thus should already see the effect of [2], that would prevent context switch. If interrupt happens between [1] and [2], then context saving code should still see the consistent view of the register file state, regardless of the processor issuing speculative read of *%gs:0x48. Return from the interrupt is the serialization point due to iret, causing read in [3] to be reissued. --OdQvBiqfLsaeimeB Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (FreeBSD) iEYEARECAAYFAkxSnuwACgkQC3+MBN1Mb4hAcwCgpwr8EgJm76cM3HJSlDyM9MaF 8UcAn2570On4CnWqPKpIDR70UoY+AVg9 =EFO7 -----END PGP SIGNATURE----- --OdQvBiqfLsaeimeB--