Date: Tue, 28 Nov 2023 09:44:32 +0100 From: Emmanuel Vadot <manu@bidouilliste.com> To: Gleb Smirnoff <glebius@freebsd.org> Cc: John Baldwin <jhb@freebsd.org>, Emmanuel Vadot <manu@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 2a35f3cdf63d - main - sys/mutex.h: Include sys/lock.h instead of sys/_lock.h Message-ID: <20231128094432.decd3248611d3f1bf231dd23@bidouilliste.com> In-Reply-To: <ZWTzJ-C7nmONFpVx@FreeBSD.org> References: <202311240950.3AO9oSeH089169@gitrepo.freebsd.org> <79e6e3a1-96a5-4d3e-9c48-a89bcbd6303d@FreeBSD.org> <ZWTzJ-C7nmONFpVx@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 27 Nov 2023 11:51:03 -0800 Gleb Smirnoff <glebius@freebsd.org> wrote: > On Fri, Nov 24, 2023 at 09:27:39AM -0800, John Baldwin wrote: > J> > commit 2a35f3cdf63d1f9b1ea5ab0174adabb631757210 > J> > Author: Emmanuel Vadot <manu@FreeBSD.org> > J> > AuthorDate: 2022-10-27 09:43:19 +0000 > J> > Commit: Emmanuel Vadot <manu@FreeBSD.org> > J> > CommitDate: 2023-11-24 09:49:58 +0000 > J> > > J> > sys/mutex.h: Include sys/lock.h instead of sys/_lock.h > J> > It uses the LA_ defines when INVARIANTS is set. > J> > This unbreak dpaa2 with FDT only kernel (like ALLWINNER or ROCKCHIP) as > J> > the driver only include sys/lock.h via header polution for ACPI kernels. > J> > Sponsored by: Beckhoff Automation GmbH & Co. KG > J> > Differential Revision: https://reviews.freebsd.org/D37145 > J> > Reviewed by: kib, mjg > J> > J> Avoiding the nested include here was originally an intentional design decision. > J> It was supposed to be a compile error if you didn't include lock.h first, and > J> callers are always supposed to include both (up until now). However, I'm fine > J> with changing this, but we should be consistent and change all the other lock > J> headers at once including sys/rwlock.h, sys/sx.h, and sys/lockmgr.h. > J> > J> You will also need to patch all of these headers to remove the #error if > J> LOCK_DEBUG or LOCK_FILE isn't defined (including sys/mutex.h which you missed > J> in this commit). > J> > J> You will also need to update all the relevant manpages (mutex.9, sx.9, rwlock.9, > J> and lockmgr.9) to remove the #include <sys/lock.h>. > J> > J> For MFC purposes I would suggest to also fix dpaa2 to #include <sys/lock.h> > J> explicitly as it was supposed to do previously. > > I'd rather recommend to revert this commit and fix dpaa2 properly. This module has > a long history of incorrect include usage, which we were able to handle properly: > > d6eabdac2ef444b62aba186c793fbd5d4226b157 > 7fb975c8fb970b35fc34561ed30a0fe220346cb6 > b0484678d405722f40278e93cdebe95829c71f3b > > We should not modify system headers to satisfy dpaa2 or any other driver. > > -- > Gleb Smirnoff Yes I've just reverted those commits. This was an old review that I cleaned up and it's not even needed anymore (also as you said the fix wasn't correct). Cheers, -- Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20231128094432.decd3248611d3f1bf231dd23>