Re: git: 02ea6033020e - main - LinuxKPI: Allow spin_lock_irqsave to be called within a critical section

[Vladimir Kondratyev]

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 <w...@freebsd.org>
AuthorDate: 2022-01-18 20:14:12 +0000
Commit:     Vladimir Kondratyev <w...@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