Date:   Mon, 13 Mar 2023 10:47:52 +0100
From:   Christian Brauner <brauner@kernel.org>
To:     Shuah Khan <skhan@linuxfoundation.org>
Cc:     Seth Forshee <sforshee@kernel.org>, shuah@kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr
 build error
Message-ID: <20230313094752.3ytftynra5rejn6c@wittgenstein>
References: <20230213183149.231779-1-skhan@linuxfoundation.org>
 <Y+rMtlvx31w7eWCA@do-x1extreme>
 <f536ecc2-1889-6df6-43d3-1a04dc8f1b14@linuxfoundation.org>
 <Y+vzB1OTXr+zTCV7@do-x1extreme>
 <dda181d1-8a5b-adb4-6665-016f50e51487@linuxfoundation.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <dda181d1-8a5b-adb4-6665-016f50e51487@linuxfoundation.org>
Precedence: bulk

On Tue, Feb 14, 2023 at 04:37:05PM -0700, Shuah Khan wrote:
> On 2/14/23 13:45, Seth Forshee wrote:
> > On Tue, Feb 14, 2023 at 10:10:00AM -0700, Shuah Khan wrote:
> 
> > > > 
> > > 
> > > The header search looks up system headers followed by installed headers in
> > > the repo (both in-tree and out-of-tree builds). kselftest builds do depend
> > > on headers_install. Did you building after running headers_install?
> > 
> > I wasn't aware they depend on headers_install. Why doesn't
> > Documentation/dev-tools/kselftest.rst mention this in the section that
> > describes how to run tests?
> > 
> 
> It ahs always been a dependency. If you were to compile from the
> main (root) Makefile as below - headers_install get done before
> test compile:
> 
> make kselftest-all TARGETS=mount_setattr
> 
> > It seems what I really need to fix the build is to include
> > linux/mount.h, which works for me with or without headers_install,
> > because I have the struct in /usr/include/linux/mount.h. And I suppose
> > the makefile should use KHDR_INCLUDES. So maybe the changes below should
> > also be included.
> > 
> 
> Yes. Makefile change to use KHDR_INCLUDES is already done. Please
> take a look at linux-kselftest next - this was done as part of a
> tree-wide change.
> 
> If including linux/mount.h is thr correct solution, please send me
> the patch on top of linux-kselftest next and I will pull it in.
> > However I know Christian has said that there are challenges with
> > including the mount headers. He wrote this test, so I'd like to hear his
> > thoughts about adding the include. He's on vacation this week though.

The problem is that the linux/mount.h and sys/mount.h headers may
conflict depending on the libc version used. So if linux/mount.h is
included care needs to be taken that no headers are included that
implicitly pull in sys/mount.h and vica versa. But if that isn't a
problem in this test and it solves the issue we can just include it.