On 19.01.2022 00:48, Konstantin Belousov wrote:
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 <[email protected]>
AuthorDate: 2022-01-18 20:14:12 +0000
Commit:     Vladimir Kondratyev <[email protected]>
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.
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?


dma-buf uses sequence locks for synchronization. If seqlock is taken for write, than thread it holding enters in to critical section to not force readers to spin if writer is preempted. Unfortunately, dma-buf writers execute callbacks which requires locks and spin_lock_irqsave is used for synchronize these callbacks.


   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.


--
WBR
Vladimir Kondratyev

Reply via email to