From nobody Tue Jan 18 21:48:50 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id E8099195C916; Tue, 18 Jan 2022 21:48:59 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4Jdj8W4ZjVz4Ysk; Tue, 18 Jan 2022 21:48:59 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.16.1/8.16.1) with ESMTPS id 20ILmoPx063661 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Tue, 18 Jan 2022 23:48:53 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 20ILmo8P063660; Tue, 18 Jan 2022 23:48:50 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 18 Jan 2022 23:48:50 +0200 From: Konstantin Belousov To: Vladimir Kondratyev Cc: Vladimir Kondratyev , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 02ea6033020e - main - LinuxKPI: Allow spin_lock_irqsave to be called within a critical section Message-ID: References: <202201182015.20IKFaWL053942@gitrepo.freebsd.org> <540a6a93-3101-02e8-b86a-50caa19f9653@kondratyev.su> List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <540a6a93-3101-02e8-b86a-50caa19f9653@kondratyev.su> X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.5 X-Spam-Checker-Version: SpamAssassin 3.4.5 (2021-03-20) on tom.home X-Rspamd-Queue-Id: 4Jdj8W4ZjVz4Ysk X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On Wed, Jan 19, 2022 at 12:35:41AM +0300, Vladimir Kondratyev wrote: > On 18.01.2022 23:22, Konstantin Belousov wrote: > > On Tue, Jan 18, 2022 at 08:15:36PM +0000, Vladimir Kondratyev wrote: > > > The branch main has been updated by wulf: > > > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=02ea6033020e11afec6472bf560b0ddebd0fa97a > > > > > > commit 02ea6033020e11afec6472bf560b0ddebd0fa97a > > > Author: Vladimir Kondratyev > > > AuthorDate: 2022-01-18 20:14:12 +0000 > > > Commit: Vladimir Kondratyev > > > CommitDate: 2022-01-18 20:14:12 +0000 > > > > > > LinuxKPI: Allow spin_lock_irqsave to be called within a critical section > > > with spinning on spin_trylock. dma-buf part of drm-kmod depends on this > > > property and absence of it support results in "mi_switch: switch in a > > > critical section" assertions [1][2]. > > > [1] https://github.com/freebsd/drm-kmod/issues/116 > > > [2] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=261166 > > > MFC after: 1 week > > > Reviewed by: manu > > > Differential Revision: https://reviews.freebsd.org/D33887 > > > --- > > > .../linuxkpi/common/include/linux/spinlock.h | 27 ++++++++++++++++++---- > > > 1 file changed, 23 insertions(+), 4 deletions(-) > > > > > > diff --git a/sys/compat/linuxkpi/common/include/linux/spinlock.h b/sys/compat/linuxkpi/common/include/linux/spinlock.h > > > index a87cb7180b28..31d47fa73986 100644 > > > --- a/sys/compat/linuxkpi/common/include/linux/spinlock.h > > > +++ b/sys/compat/linuxkpi/common/include/linux/spinlock.h > > > @@ -37,6 +37,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > @@ -117,14 +118,32 @@ typedef struct { > > > local_bh_disable(); \ > > > } while (0) > > > -#define spin_lock_irqsave(_l, flags) do { \ > > > - (flags) = 0; \ > > > - spin_lock(_l); \ > > > +#define __spin_trylock_nested(_l, _n) ({ \ > > > + int __ret; \ > > > + if (SPIN_SKIP()) { \ > > > + __ret = 1; \ > > > + } else { \ > > > + __ret = mtx_trylock_flags(&(_l)->m, MTX_DUPOK); \ > > > + if (likely(__ret != 0)) \ > > > + local_bh_disable(); \ > > > + } \ > > > + __ret; \ > > > +}) > > > + > > > +#define spin_lock_irqsave(_l, flags) do { \ > > > + (flags) = 0; \ > > > + if (unlikely(curthread->td_critnest != 0)) \ > > > + while (!spin_trylock(_l)) {} \ > > > + else \ > > > + spin_lock(_l); \ > > > } while (0) > > > #define spin_lock_irqsave_nested(_l, flags, _n) do { \ > > > (flags) = 0; \ > > > - spin_lock_nested(_l, _n); \ > > > + if (unlikely(curthread->td_critnest != 0)) \ > > > + while (!__spin_trylock_nested(_l, _n)) {} \ > > > + else \ > > > + spin_lock_nested(_l, _n); \ > > > } while (0) > > > #define spin_unlock_irqrestore(_l, flags) do { \ > > You are spin-waiting for blockable mutex, am I right? > > Both, yes and no. On Linux spin_lock_irqsave is generally unblockable as it > disables preemption and interrupts while our version does not do this as > LinuxKPI is not ready for such a tricks. > It seems that we should explicitly add critical_enter()/critical_exit calls > to related dma-buf parts to make it unblockable too. LinuxKPI does +1 to the level of locks comparing with Linux, so their spinlocks become our blockable mutexes. Can you please explain why dmabufs need critical section? What is achieved there by disabled preemption? > > This means a deadlock. > > Just for example, on UP machine the spinning thread could starve the thread > > that owns the mutex, which never gets to the CPU. > > Any ideas how to avoid it in a generic way? First I need to understand why do you need to mix mutexes and critical sections.