Received-SPF: pass (google.com: domain of linux-kernel+bounces-84052-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <a4b957de-90d1-4779-b6a5-c6a9c1b005cc@linuxfoundation.org>
Date: Tue, 27 Feb 2024 14:28:02 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RESEND PATCH] selftests/overlayfs: fix compilation error in
 overlayfs
Content-Language: en-US
To: Andrei Vagin <avagin@google.com>
Cc: Meng Li <li.meng@amd.com>, Huang Rui <ray.huang@amd.com>,
 linux-pm@vger.kernel.org, Nathan Fontenot <nathan.fontenot@amd.com>,
 Deepak Sharma <deepak.sharma@amd.com>,
 Alex Deucher <alexander.deucher@amd.com>,
 Mario Limonciello <mario.limonciello@amd.com>,
 Perry Yuan <Perry.Yuan@amd.com>, Xiaojian Du <Xiaojian.Du@amd.com>,
 Viresh Kumar <viresh.kumar@linaro.org>, Borislav Petkov <bp@alien8.de>,
 linux-kernel@vger.kernel.org,
 "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org>,
 Shuah Khan <skhan@linuxfoundation.org>
References: <20240227074204.3573450-1-li.meng@amd.com>
 <2aaa2a61-f753-4434-8c92-886fc4bb9aef@linuxfoundation.org>
 <CAEWA0a6D1YQtCuW1FswyGXXWNTrmyerNS3zzcqBPpyQ-GtOopA@mail.gmail.com>
From: Shuah Khan <skhan@linuxfoundation.org>
In-Reply-To: <CAEWA0a6D1YQtCuW1FswyGXXWNTrmyerNS3zzcqBPpyQ-GtOopA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 2/27/24 14:20, Andrei Vagin wrote:
> On Tue, Feb 27, 2024 at 8:41 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 2/27/24 00:42, Meng Li wrote:
>>> make -C tools/testing/selftests, compiling dev_in_maps fail.
>>> In file included from dev_in_maps.c:10:
>>> /usr/include/x86_64-linux-gnu/sys/mount.h:35:3: error: expected identifier before numeric constant
>>>      35 |   MS_RDONLY = 1,                /* Mount read-only.  */
>>>         |   ^~~~~~~~~
>>>
>>> That sys/mount.h has to be included before linux/mount.h.
>>>
>>> Signed-off-by: Meng Li <li.meng@amd.com>
>>> ---
>>>    tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>
>> I don't see this problem when I build it on my system when
>> I run:
>>
>> make -C tools/testing/selftests
>> or
>> make -C tools/testing/selftests/filesystems/overlayfs
>>
>> Are you running this after doing headers_install?
> 
> It depends on libc headers. It can work with one libc and doesn't work
> with another one. I have seen many times when linux headers conflicted
> with libc headers. The only reliable way to avoid this sort of issues is
> to include just one linux or libc header.
> 
> In this case, we can do something like this:
> 
> diff --git a/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c
> b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c
> index e19ab0e85709..f1ba82e52192 100644
> --- a/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c
> +++ b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c
> @@ -10,7 +10,6 @@
>   #include <linux/mount.h>
>   #include <sys/syscall.h>
>   #include <sys/stat.h>
> -#include <sys/mount.h>
>   #include <sys/mman.h>
>   #include <sched.h>
>   #include <fcntl.h>
> @@ -40,6 +39,14 @@ static int sys_move_mount(int from_dfd, const char
> *from_pathname,
>          return syscall(__NR_move_mount, from_dfd, from_pathname,
> to_dfd, to_pathname, flags);
>   }
> 
> +static int sys_mount(const char *source, const char *target,
> +                    const char *filesystemtype, unsigned long mountflags,
> +                    const void *data)
> +{
> +       return syscall(__NR_mount, source, target, filesystemtype,
> mountflags, data);
> +}
> +
> +
>   static long get_file_dev_and_inode(void *addr, struct statx *stx)
>   {
>          char buf[4096];
> @@ -167,7 +174,7 @@ int main(int argc, char **argv)
>                  return 1;
>          }
> 
> -       if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) == -1) {
> +       if (sys_mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) == -1) {
>                  pr_perror("mount");
>                  return 1;
>          }
> 


This is definitely better solution to this problem than reordering
the includes only find another problem down the road.

thanks,
-- Shuah