From nobody Fri May 17 21:36:02 2024 X-Original-To: dev-commits-src-main@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 4Vh0dN239kz5K81Y; Fri, 17 May 2024 21:36:08 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Vh0dN1Rspz4Nbm; Fri, 17 May 2024 21:36:08 +0000 (UTC) (envelope-from bz@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1715981768; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3fUFqEwQWBgcm8lsWcn8CqpAMzZ3eAHUeDz2RNJ9hyQ=; b=BEBzhp3l0lLp6n7LtpClCQUO1oFQ83gEEZIzQJtXLu/4pY9x6/t2PVi8GhW4tKIIKZmw6o QtH+YwmJwJXPkn9oKBjcNjU0+13erDYJ6fYHaK6CEjIrWX9WEtnZ6Lz6leXWHZ6J+7NozG woSwVRbrKQLBGKYV5jq4SjpT+NikfcX3JfJDyp989rohGAX+VXqWiLAFD4SUM3vCwf2Elx 2Y3q9q7z4gCJXiUPu1jQMGZUaKq3cuJUxidoLy6SDNiDyti38Ksdyaz3aA9PoEQYTlLSdX ZSymHUax4pTaeXcw60ackXJMhc7sB2B+lB4UtsHeemVxR1ktzm3AeuWX8gBQOQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1715981768; a=rsa-sha256; cv=none; b=E1avnb9Fu/9febIZzh3V5ykm5RmD7Dx0R7FMTr/ICLaUmUknfs281PZ3vQM2LP141o+gP4 3LvCvU30fEa4AoXcI55ZTO+HjBgDDRxFLfUEoLeW1/s33oYtEOFKv/UF+qSXyMfeCA3Qhz vBTkrUSxkgytLP8NgAyn8lBJaZQtISkszcvYdycyygO9obw0zBEFuDp+Zae9bESQ2WkFc8 NrVQDvx8Xvi8rcEOySKIA2ikO8omaJ4TjY+yFJ8/PFlXwQr3/HFQLbb/wVePnrj4GRBiuK VSIi1ckp6tMzljWKlqpZujG5R+AroiKkdN+m4/RAJqnM7EXSrOl7mCbWAg99SQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1715981768; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3fUFqEwQWBgcm8lsWcn8CqpAMzZ3eAHUeDz2RNJ9hyQ=; b=QUUelQ4Uy1f96BoQqJR4v9J71ajtVpojI3MUk1Ywy/qQHliRbXLy0XtLswdWVFbzqmDt5N dWCEmi0Dv2DLEtqzCvA1xm11wyDob9AVaLkOyhwWs3/hi7h1kZvBbGHCx5Ja7wwBfu0rSW X9J5oSD/PspbzJc2YBq8J5J7oiInQFv4Hs+FKxPQerarRIRyvooMCn3cyvdfZOTzIkhXKq juPTAo1wsuaom3czcDQO2bnabuFY4QTJwvJycClZyw0/BIfcH3xR3XLhg6x+N/1M7fsenN TaCIbBUUuNjhiE+HlcQ+gopC3vvePdbxaattotUrMKSIMCeNNOl/P/6fCxhYig== Received: from mx1.sbone.de (cross.sbone.de [195.201.62.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mx1.sbone.de", Issuer "SBone.DE Root Certificate Authority" (not verified)) (Authenticated sender: bz/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Vh0dN0JB1zNlb; Fri, 17 May 2024 21:36:08 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:4902:0:7404:2:1025]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id 96C078D4A126; Fri, 17 May 2024 21:36:05 +0000 (UTC) Received: from content-filter.t4-02.sbone.de (content-filter.t4-02.sbone.de [IPv6:fde9:577b:c1a9:4902:0:7404:2:2742]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id A688B2D029D8; Fri, 17 May 2024 21:36:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:4902:0:7404:2:1025]) by content-filter.t4-02.sbone.de (content-filter.t4-02.sbone.de [IPv6:fde9:577b:c1a9:4902:0:7404:2:2742]) (amavisd-new, port 10024) with ESMTP id w8yYAazTkx2N; Fri, 17 May 2024 21:36:03 +0000 (UTC) Received: from strong-iwl0.sbone.de (strong-iwl0.sbone.de [IPv6:fde9:577b:c1a9:4902:b66b:fcff:fef3:e3d2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id 30F222D029D2; Fri, 17 May 2024 21:36:02 +0000 (UTC) Date: Fri, 17 May 2024 21:36:02 +0000 (UTC) From: "Bjoern A. Zeeb" To: John Baldwin cc: Emmanuel Vadot , Emmanuel Vadot , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: cff79fd02636 - main - linuxkpi: Fix spin_lock_init In-Reply-To: <0f2c4189-e259-4bd8-ad31-212a8df0e1b5@FreeBSD.org> Message-ID: <188o7q6o-n0rn-235r-76n4-779o555094r9@SerrOFQ.bet> References: <202405170559.44H5xD7d019861@gitrepo.freebsd.org> <2cd3e698-1b42-4e7f-93a0-aacccb55c8d6@FreeBSD.org> <20240517183350.e45f54df07f670980d5a51c3@bidouilliste.com> <0f2c4189-e259-4bd8-ad31-212a8df0e1b5@FreeBSD.org> X-OpenPGP-Key-Id: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset=US-ASCII On Fri, 17 May 2024, John Baldwin wrote: > On 5/17/24 9:33 AM, Emmanuel Vadot wrote: >> On Fri, 17 May 2024 09:07:53 -0700 >> John Baldwin wrote: >> >>> On 5/16/24 10:59 PM, Emmanuel Vadot wrote: >>>> The branch main has been updated by manu: >>>> >>>> URL: >>>> https://cgit.FreeBSD.org/src/commit/?id=cff79fd02636f34010d8b835cc9e55401fa76e74 >>>> >>>> commit cff79fd02636f34010d8b835cc9e55401fa76e74 >>>> Author: Emmanuel Vadot >>>> AuthorDate: 2024-05-17 04:52:53 +0000 >>>> Commit: Emmanuel Vadot >>>> CommitDate: 2024-05-17 05:58:59 +0000 >>>> >>>> linuxkpi: Fix spin_lock_init >>>> Some linux code re-init some spinlock so add MTX_NEW to >>>> mtx_init. >>>> Reported by: David Wolfskill >>>> Fixes: ae38a1a1bfdf ("linuxkpi: spinlock: Simplify code") >>>> --- >>>> sys/compat/linuxkpi/common/include/linux/spinlock.h | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/sys/compat/linuxkpi/common/include/linux/spinlock.h >>>> b/sys/compat/linuxkpi/common/include/linux/spinlock.h >>>> index 3f6eb4bb70f6..2992e41c9c02 100644 >>>> --- a/sys/compat/linuxkpi/common/include/linux/spinlock.h >>>> +++ b/sys/compat/linuxkpi/common/include/linux/spinlock.h >>>> @@ -140,7 +140,7 @@ typedef struct mtx spinlock_t; >>>> #define spin_lock_name(name) _spin_lock_name(name, >>>> __FILE__, __LINE__) >>>> #define spin_lock_init(lock) mtx_init(lock, >>>> spin_lock_name("lnxspin"), \ >>>> - NULL, MTX_DEF | MTX_NOWITNESS) >>>> + NULL, MTX_DEF | MTX_NOWITNESS | MTX_NEW) >>>> #define spin_lock_destroy(_l) mtx_destroy(_l) >>> >>> This is only ok because of MTX_NOWITNESS. Reiniting locks without >>> destroying >>> them corrupts the internal linked lists in WITNESS for locks using >>> witness. >>> That may warrant a comment here explaining why we disable witness. >> >> I'll try to look at what linux expect for spinlocks, it could also be >> that we need to do this because some drivers via linuxkpi does weird >> things ... >> >>> It might be nice to add an extension to the various lock inits for code >>> that >>> wants to opt-int to using WITNESS where a name can be passed. Using those >>> would >>> be relatively small diffs in the client code and let select locks opt into >>> using WITNESS. You could make it work by adding an optional second >>> argument >>> to spin_lock_init, etc. that takes the name. >> >> We can't change spin_lock_init, we need to follow linux api here. > > You can use macro magic to add support for an optional second argument. > > #define _spin_lock_init2(lock, name) mtx_init(lock, name, NULL, MTX_DEF) > > #define _spin_lock_init1(lock) mtx_init(lock, spin_lock_name("lnxspin"), ...) > > #define _spin_lock_init_macro(lock, name, NAME, ...) > NAME > > #define spin_lock_init(...) \ > _spin_lock_init_macro(__VA_ARGS__, _spin_lock_init2, > _spin_lock_init1)(__VA_ARGS__) > > Then you can choose to specifically annotate certain locks with a name > instead in which case they will use WITNESS. I think the real confusing here comes from the fact that FreeBSD has spin_lock_destroy in LinuxKPI which I believe is a local addition (though not documented as such) for semi-native drivers using parts of LinuxKPI and in order to use spinlocks according to "FreeBSD expectations". I believe (and still hope someone would correct me) that Linux has not functions to destroy locks like we do? I believe for rwlocks I had left that remark on the review: https://reviews.freebsd.org/D45206#1031316 So if you use WITNESS anywhere you could only really do so for "internal" parts but nowhere in Linux driver code as that will likely simply break assumptions? -- Bjoern A. Zeeb r15:7