From nobody Tue Jan 18 22:11:12 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 72D25196D564; Tue, 18 Jan 2022 22:12:03 +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 4Jdjg71KjGz4lpr; Tue, 18 Jan 2022 22:12:03 +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 399A8309588; Wed, 19 Jan 2022 01:12:02 +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 qUM3oP5KDstw; Wed, 19 Jan 2022 01:11:58 +0300 (MSK) Received: from mail.cicgroup.ru (unknown [195.170.219.74]) by corp.infotel.ru (Postfix) with ESMTP id F2A43309587; Wed, 19 Jan 2022 01:11:57 +0300 (MSK) Received: from mail.cicgroup.ru (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTP id 6F8E442211F; Wed, 19 Jan 2022 01:11:56 +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 AUnNM2F3Or-5; Wed, 19 Jan 2022 01:11:53 +0300 (MSK) Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTPA id 68D8242211C; Wed, 19 Jan 2022 01:11:53 +0300 (MSK) Message-ID: <17f62f59-b601-4f5a-9967-21cb46f904c0@kondratyev.su> Date: Wed, 19 Jan 2022 01:11:12 +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: 4Jdjg71KjGz4lpr 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 01:08, Konstantin Belousov wrote: > On Wed, Jan 19, 2022 at 01:01:45AM +0300, Vladimir Kondratyev wrote: >> 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. > > Then, it seems that locking should be changed either to rwlocks or rmlocks, > not sure which. > > Do you mean our seqlocks as presented in sys/seqc.h, or something Linuxish? LinuxKPI seqlocks wraps sys/seqc.h -- WBR Vladimir Kondratyev