From nobody Sat Jan 22 11:04:20 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 42A58196F598; Sat, 22 Jan 2022 11:05:29 +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 4Jgth85dCDz4r6b; Sat, 22 Jan 2022 11:05:28 +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 2A1CD31926F; Sat, 22 Jan 2022 14:05:21 +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 Waql1flLCvFj; Sat, 22 Jan 2022 14:05:17 +0300 (MSK) Received: from mail.cicgroup.ru (unknown [195.170.219.74]) by corp.infotel.ru (Postfix) with ESMTP id 0F72D31926A; Sat, 22 Jan 2022 14:05:17 +0300 (MSK) Received: from mail.cicgroup.ru (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTP id 4E70F42211F; Sat, 22 Jan 2022 14:05:13 +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 yUfFyTqzdmuG; Sat, 22 Jan 2022 14:05:05 +0300 (MSK) Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTPA id C742742211C; Sat, 22 Jan 2022 14:05:05 +0300 (MSK) Message-ID: <105887a8-0ab0-e551-a883-79a055fb3c15@kondratyev.su> Date: Sat, 22 Jan 2022 14:04:20 +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: Hans Petter Selasky , Konstantin Belousov , Vladimir Kondratyev Cc: 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> <3f62b9e2-214b-b1d4-f682-9318f77f315d@kondratyev.su> <6b3eabe6-fcdf-e697-1295-e9ec9604ec41@selasky.org> From: Vladimir Kondratyev In-Reply-To: <6b3eabe6-fcdf-e697-1295-e9ec9604ec41@selasky.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4Jgth85dCDz4r6b 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 22.01.2022 12:49, Hans Petter Selasky wrote: > On 1/19/22 23:02, Vladimir Kondratyev wrote: >> On 19.01.2022 12:50, Hans Petter Selasky wrote: >>> On 1/18/22 22:35, Vladimir Kondratyev wrote: >>>> >>>> Any ideas how to avoid it in a generic way? >>> >>> Hi, >>> >>> On a non-SMP system this will lead to deadlock. >>> >>> Is it possible you can pre-lock this spin lock earlier, so that it is= already=20 >>> locked, so instead of >>> >>> while(trylock()); >>> >>> You have: >>> >>> assert (trylock() XXX) >>> >> >> Unfortunately, vulnerable functions are called in too many code paths = to patch=20 >> them all. >> >>> Or else, >>> >>> convert this particular lock to a native FreeBSD spinlock mutex. >>> >> It can be done for wake_up() but not for dma_fence_signal() which suff= ers from=20 >> this problem too. Some code that uses that lock expect it to be spinlo= ck_t >> >> I think we can just drop critical section in seqlock-related part of d= ma-buf=20 >> code and replace it with rwlock as kib@ and mjg@ suggested. Leave seql= ock for=20 >> actual locking to preserve semantics as much as possible and add rwloc= k to=20 >> implement reader's blocking. Following snippets show code conversion r= equired=20 >> for this change: >> >> >> Lock seqlock as reader: >> >> retry: >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0seq =3D read_seqcount_begin(&obj->seq); >> >> ... reader payload ... >> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (read_seqcount_retry(&obj->seq, seq))= { >> #ifdef __FreeBSD__ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Wait for rwlock to= be released by writer */ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 rw_rlock(&obj->rwlock= ); >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 rw_runlock(&obj->rwlo= ck); >> #endif >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto retry; >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} >> >> >> Lock seqlock as writer: >> >> #ifdef __linux__ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0preempt_disable(); >> #elif defined (__FreeBSD__) >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rw_wlock(&obj->rwlock); >> #endif >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0write_seqcount_begin(&obj->seq); >> >> ... writer payload ... >> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0write_seqcount_end(&obj->seq); >> #ifdef __linux__ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0preempt_enable(); >> #elif defined (__FreeBSD__) >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rw_wunlock(&obj->rwlock); >> #endif >> >=20 > Hi Vladimir, >=20 > Waiting for a differential revision. >=20 > --HPS See https://github.com/freebsd/drm-kmod/pull/138 --=20 WBR Vladimir Kondratyev