Re: git: 2a35f3cdf63d - main - sys/mutex.h: Include sys/lock.h instead of sys/_lock.h

From: Emmanuel Vadot <manu_at_bidouilliste.com>
Date: Tue, 28 Nov 2023 08:44:32 UTC
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>