From owner-freebsd-hackers@FreeBSD.ORG Thu Jul 29 23:57:26 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 9D5671065673 for ; Thu, 29 Jul 2010 23:57:26 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id 60E658FC20 for ; Thu, 29 Jul 2010 23:57:26 +0000 (UTC) Received: by iwn35 with SMTP id 35so960151iwn.13 for ; Thu, 29 Jul 2010 16:57:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:content-type:content-transfer-encoding; bh=JofS1w1C5pEvTz/bYg2iJafvnkpVzgN2GwEf13Wku84=; b=MVX6SDjnXVf1N0n/VKO2BCCcHcoN1toO2ZPkgJj40gbHUqVrhCFLEHQsMSQ02VdJDH e0F4DUnZpc3/uebcojSTxlUCHA0I2a23SiufSZKyTCBMZf+984aDHpU/qq7QnrE4a8Ps 25RGtXOAEAHba0vLNX9Wq3i4Wk091uQVw0Jb4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:content-type :content-transfer-encoding; b=rKfRVw8WxuSy1fzHecX4/IQI7LLO1CRL//1b8O/Jnhp9EbPp1WwI8uSk/DB6rcjxn8 b/umbfzEVKdqYXsKW8x+AZykjxNfy47hwNlZJ5M5kQI9g1VOsqOWFN4xvrvsZl1uab8F 4mP3qgSB2i0bNAtGqlF83QGUDGaSvJgWxNNxA= MIME-Version: 1.0 Received: by 10.42.9.4 with SMTP id k4mr194785ick.72.1280447845206; Thu, 29 Jul 2010 16:57:25 -0700 (PDT) Sender: mdf356@gmail.com Received: by 10.42.6.85 with HTTP; Thu, 29 Jul 2010 16:57:25 -0700 (PDT) In-Reply-To: References: Date: Thu, 29 Jul 2010 16:57:25 -0700 X-Google-Sender-Auth: E1Ba7yahsoiKgm4UgP7rPIrjA_w Message-ID: From: mdf@FreeBSD.org To: freebsd-hackers@freebsd.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: Thu, 29 Jul 2010 23:57:26 -0000 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: > > =A0 =A0 =A0 =A0thread_lock(td); > =A0 =A0 =A0 =A0td->td_flags &=3D ~TDF_SELECT; > =A0 =A0 =A0 =A0thread_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: =A0 =A0 =A0 65 48 8b 1c 25 00 00 =A0 =A0mov =A0 =A0%gs:= 0x0,%rbx > ffffffff802db217: =A0 =A0 =A0 00 00 > ffffffff802db219: =A0 =A0 =A0 ff 83 04 01 00 00 =A0 =A0 =A0 incl =A0 0x10= 4(%rbx) > =A0 =A0 =A0 =A0 * Pin the thread in order to avoid problems with thread m= igration. > =A0 =A0 =A0 =A0 * Once that all verifies are passed about spinlocks owner= ship, > =A0 =A0 =A0 =A0 * the thread is in a safe path and it can be unpinned. > =A0 =A0 =A0 =A0 */ > =A0 =A0 =A0 =A0sched_pin(); > =A0 =A0 =A0 =A0lock_list =3D PCPU_GET(spinlocks); > ffffffff802db21f: =A0 =A0 =A0 65 48 8b 04 25 48 00 =A0 =A0mov =A0 =A0%gs:= 0x48,%rax > ffffffff802db226: =A0 =A0 =A0 00 00 > =A0 =A0 =A0 =A0if (lock_list !=3D NULL && lock_list->ll_count !=3D 0) { > ffffffff802db228: =A0 =A0 =A0 48 85 c0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tes= t =A0 %rax,%rax > =A0 =A0 =A0 =A0 * Pin the thread in order to avoid problems with thread m= igration. > =A0 =A0 =A0 =A0 * Once that all verifies are passed about spinlocks owner= ship, > =A0 =A0 =A0 =A0 * the thread is in a safe path and it can be unpinned. > =A0 =A0 =A0 =A0 */ > =A0 =A0 =A0 =A0sched_pin(); > =A0 =A0 =A0 =A0lock_list =3D PCPU_GET(spinlocks); > ffffffff802db22b: =A0 =A0 =A0 48 89 85 f0 fe ff ff =A0 =A0mov =A0 =A0%rax= ,-0x110(%rbp) > ffffffff802db232: =A0 =A0 =A0 48 89 85 f8 fe ff ff =A0 =A0mov =A0 =A0%rax= ,-0x108(%rbp) > =A0 =A0 =A0 =A0if (lock_list !=3D NULL && lock_list->ll_count !=3D 0) { > ffffffff802db239: =A0 =A0 =A0 0f 84 ff 00 00 00 =A0 =A0 =A0 je =A0 =A0 ff= ffffff802db33e > > ffffffff802db23f: =A0 =A0 =A0 44 8b 60 50 =A0 =A0 =A0 =A0 =A0 =A0 mov =A0= =A00x50(%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. =A0So clearly the thread was being migrated > right around panic time. > > This is the amd64 kernel on stable/7. =A0I'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? =A0Could the hardware have re-ordered > the "mov =A0 =A0%gs:0x48,%rax" PCPU_GET to before the sched_pin() > increment? 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. Thanks, matthew