From owner-freebsd-hackers@freebsd.org Mon Apr 10 08:48:03 2017 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7F984D36E00 for ; Mon, 10 Apr 2017 08:48:03 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E63B723E for ; Mon, 10 Apr 2017 08:48:02 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id v3A8lvou016982 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 10 Apr 2017 11:47:57 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua v3A8lvou016982 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id v3A8lvrt016981; Mon, 10 Apr 2017 11:47:57 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 10 Apr 2017 11:47:56 +0300 From: Konstantin Belousov To: Chris Torek Cc: ablacktshirt@gmail.com, ed@nuxi.nl, freebsd-hackers@freebsd.org, rysto32@gmail.com, vasanth.raonaik@gmail.com Subject: Re: Understanding the FreeBSD locking mechanism Message-ID: <20170410084756.GJ1788@kib.kiev.ua> References: <20170410071137.GH1788@kib.kiev.ua> <201704100811.v3A8BP8B049595@elf.torek.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201704100811.v3A8BP8B049595@elf.torek.net> User-Agent: Mutt/1.8.0 (2017-02-23) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Apr 2017 08:48:03 -0000 On Mon, Apr 10, 2017 at 01:11:25AM -0700, Chris Torek wrote: > >Signal trampolines never were put on the kernel stack ... > > Oops, right, not sure why I was thinking that. However I would still > prefer to have libc supply the trampoline address (the underlying > signal system calls can do this, since until you are catching a > signal in the first place, there is no need for a known-in-advance > trampoline address). I considered some variation of this scheme when I worked on the non-executable stack support. AFAIR the reason why I decided not to do this was that the kernel-injected signal trampoline is still needed for backward ABI-compat. In other words, the shared page would be still needed, and we would end up with both libc trampoline and kernel trampoline, which felt somewhat excessive. Selecting one scheme or another based e.g. on the binary osrel was too fragile, e.g. new binary might have loaded old library, and the kernel trampoline still must be present in this situation. > What I meant was that it's a dreadful error to do, e.g.: > > critical_enter(); > mtx_lock(mtx); > ... > mtx_unlock(mtx); > critical_exit(); > > but the other order (lock first, then enter/exit) is OK. This > is similar to the prohibition against obtaining a default mutex > while holding a spin mutex. Sure, this is a bug. Debugging kernel would catch this, at least mi_switch() asserts that td_critnest == 0 (technically it checks that td_critnest == 1 but the thread lock is owned there). So if such code tries to lock contested mutex, the bug causes panic. I am sorry my previous mail contained an error: the spinlock_enter() also increments td_critnest. Still, since interrupts are disabled, this is mostly cosmetics. The more important consequence is that critical_exit() on spinlock unlock re-checks td_owepreempt and executes potential postponed scheduling actions.