From nobody Tue Jan 18 22:01:45 2022 X-Original-To: dev-commits-src-all@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 00E371965EED; Tue, 18 Jan 2022 22:02:39 +0000 (UTC) (envelope-from vladimir@kondratyev.su) Received: from corp.infotel.ru (corp.infotel.ru [195.170.219.3]) by mx1.freebsd.org (Postfix) with ESMTP id 4JdjSG5F2Qz4gwQ; Tue, 18 Jan 2022 22:02:38 +0000 (UTC) (envelope-from vladimir@kondratyev.su) Received: from corp (corp.infotel.ru [195.170.219.3]) by corp.infotel.ru (Postfix) with ESMTP id CF084308EF1; Wed, 19 Jan 2022 01:02:35 +0300 (MSK) X-Virus-Scanned: amavisd-new at corp.infotel.ru Received: from corp.infotel.ru ([195.170.219.3]) by corp (corp.infotel.ru [195.170.219.3]) (amavisd-new, port 10024) with ESMTP id cPeGCJIfJHGC; Wed, 19 Jan 2022 01:02:31 +0300 (MSK) Received: from mail.cicgroup.ru (unknown [195.170.219.74]) by corp.infotel.ru (Postfix) with ESMTP id 6D0EC308EEE; Wed, 19 Jan 2022 01:02:31 +0300 (MSK) Received: from mail.cicgroup.ru (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTP id B0EFC42211F; Wed, 19 Jan 2022 01:02:29 +0300 (MSK) Received: from mail.cicgroup.ru ([127.0.0.1]) by mail.cicgroup.ru (mail.cicgroup.ru [127.0.0.1]) (amavisd-new, port 10024) with SMTP id mIJ484IS_aVu; Wed, 19 Jan 2022 01:02:26 +0300 (MSK) Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTPA id AD77842211C; Wed, 19 Jan 2022 01:02:26 +0300 (MSK) Message-ID: Date: Wed, 19 Jan 2022 01:01:45 +0300 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: git: 02ea6033020e - main - LinuxKPI: Allow spin_lock_irqsave to be called within a critical section Content-Language: en-US To: Konstantin Belousov Cc: Vladimir Kondratyev , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202201182015.20IKFaWL053942@gitrepo.freebsd.org> <540a6a93-3101-02e8-b86a-50caa19f9653@kondratyev.su> From: Vladimir Kondratyev In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4JdjSG5F2Qz4gwQ 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 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 >>>> 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? > 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