Date: Wed, 19 Jan 2022 00:35:41 +0300 From: Vladimir Kondratyev <vladimir@kondratyev.su> To: Konstantin Belousov <kostikbel@gmail.com>, Vladimir Kondratyev <wulf@freebsd.org> Cc: 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: <540a6a93-3101-02e8-b86a-50caa19f9653@kondratyev.su> In-Reply-To: <YechbCTSWuUs%2BNr5@kib.kiev.ua> References: <202201182015.20IKFaWL053942@gitrepo.freebsd.org> <YechbCTSWuUs%2BNr5@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <wulf@FreeBSD.org> >> AuthorDate: 2022-01-18 20:14:12 +0000 >> Commit: Vladimir Kondratyev <wulf@FreeBSD.org> >> 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 <sys/lock.h> >> #include <sys/mutex.h> >> #include <sys/kdb.h> >> +#include <sys/proc.h> >> >> #include <linux/compiler.h> >> #include <linux/rwlock.h> >> @@ -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. 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? -- WBR Vladimir Kondratyev
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?540a6a93-3101-02e8-b86a-50caa19f9653>