From nobody Sun May 21 16:57:47 2023 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 4QPRbM66txz4CD2l; Sun, 21 May 2023 16:57:51 +0000 (UTC) (envelope-from hselasky@freebsd.org) Received: from mail.turbocat.net (turbocat.net [88.99.82.50]) (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 did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4QPRbM3pchz3jhR; Sun, 21 May 2023 16:57:51 +0000 (UTC) (envelope-from hselasky@freebsd.org) Authentication-Results: mx1.freebsd.org; none Received: from [10.36.2.145] (unknown [46.212.121.255]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id ACD4D260B9B; Sun, 21 May 2023 18:57:47 +0200 (CEST) Message-ID: <21c9532e-4ca7-a7fe-1ff6-07a94cbad6ab@freebsd.org> Date: Sun, 21 May 2023 18:57:47 +0200 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: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: git: 805d759338a2 - main - mlx4: Move DEFINE_MUTEX() outside function body. To: Jessica Clarke Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" References: <202305211621.34LGLsup059861@gitrepo.freebsd.org> <54EF67D8-2A79-4EAB-8EFB-232F14FFE792@freebsd.org> Content-Language: en-US From: Hans Petter Selasky In-Reply-To: <54EF67D8-2A79-4EAB-8EFB-232F14FFE792@freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 4QPRbM3pchz3jhR X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:24940, ipnet:88.99.0.0/16, country:DE] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On 5/21/23 18:33, Jessica Clarke wrote: > On 21 May 2023, at 17:21, Hans Petter Selasky wrote: >> >> The branch main has been updated by hselasky: >> >> URL: https://cgit.FreeBSD.org/src/commit/?id=805d759338a2be939fffc8bf3f25cfaab981a9be >> >> commit 805d759338a2be939fffc8bf3f25cfaab981a9be >> Author: Hans Petter Selasky >> AuthorDate: 2023-05-21 11:25:28 +0000 >> Commit: Hans Petter Selasky >> CommitDate: 2023-05-21 16:20:16 +0000 >> >> mlx4: Move DEFINE_MUTEX() outside function body. >> >> Move static mutex declaration outside function body, to avoid global >> variables being declared on the stack, when using SYSINITs. > > What? This is nonsense. It’s not on the stack either way round. > > Please revert this. > > Jess Hi Jess, I think this is a false positive of yours. You need to look through all the macros used there. Basically DEFINE_MUTEX() expands to a bunch of structures, which are not in any block. The "static" you see in patch just covers the first mutex structure. SYSINITs use "static" in front of all structure definitions. If you want to change from static structures to global symbols, then my change is correct. Before: static DEFINE_MUTEX(xxx); Expands to something like: static struct yyy xxx; static struct sysinit zzz; .... If you want to change from "static struct sysinit zzz;" to "extern struct sysinit zzz" and also initialize the structure there, then that won't work, based on what I currently know about C-programming. I tried, but clang gave me a warning about it. You can't declare global variables inside a function or it is not good style. From what I can see, this location is the only place I've come accross in the FreeBSD kernel, where a SYSINIT() is used inside a function, and I thought I would just move that outside the function instead. This change also allows for: https://reviews.freebsd.org/D40193 --HPS