From: David Hildenbrand <david@redhat.com>
To: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>, linux-mm@kvack.org
Cc: Andrew Morton <akpm@linux-foundation.org>,
Shuah Khan <shuah@kernel.org>,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: PROBLEM: selftest/vm/cow.c failed to compile (‘MADV_PAGEOUT’ undeclared)
Date: Tue, 10 Jan 2023 13:29:16 +0100 [thread overview]
Message-ID: <a0985987-384a-d44f-c365-b0b2b935d417@redhat.com> (raw)
In-Reply-To: <dda726db-6276-f312-be12-cee05228161d@alu.unizg.hr>
On 10.01.23 13:25, Mirsad Todorovac wrote:
> On 1/10/23 11:05, David Hildenbrand wrote:
>> On 09.01.23 22:41, Mirsad Goran Todorovac wrote:
>>> On 1/9/2023 5:42 PM, David Hildenbrand wrote:
>>>
>>> Hi, thank you for your reply.
>>>
>>>>> I hope this is enough information for you to debug the issue.
>>>>>
>>>>> I am standing by for any additional diagnostics needed.
>>>>
>>>> Won't userfaultfd.c fail in a similar way?
>>>>
>>>> Anyhow, khugepaged.c jas
>>>>
>>>> #ifndef MADV_PAGEOUT
>>>> #define MADV_PAGEOUT 21
>>>> #endif
>>>>
>>>> So most probably we should do the same.
>>>
>>> Actually, David, it turned out that userfaultfd.c compiled
>>> out-of-the-box, and side-by-side comparison showed that it also included
>>> "/home/marvin/linux/kernel/linux_torvalds/usr/include/asm-generic/mman-common.h"
>>>
>>> The only remaining difference was including <linux/mman.h>, which fixed
>>> the issue w/o #ifdef ... #endif
>>>
>>> Hope this helps.
>>>
>>> Please find the following diff.
>>>
>>> Regards,
>>> Mirsad
>>>
>>> ------------------------------------------------------------------------------
>>> diff --git a/tools/testing/selftests/vm/cow.c
>>> b/tools/testing/selftests/vm/cow.c
>>> index 26f6ea3079e2..dd8cf12c6776 100644
>>> --- a/tools/testing/selftests/vm/cow.c
>>> +++ b/tools/testing/selftests/vm/cow.c
>>> @@ -16,6 +16,7 @@
>>> #include <fcntl.h>
>>> #include <dirent.h>
>>> #include <assert.h>
>>> +#include <linux/mman.h>
>>> #include <sys/mman.h>
>>> #include <sys/ioctl.h>
>>> #include <sys/wait.h>
>>>
>>
>> I already sent a different fix [1]. I suspect when including
>> linux/mman.h, it would still be problematic with older kernel
>> headers that lack MADV_PAGEOUT (< v5.4).
>
> I see your point.
>
>> But yeah, I saw that userfaultfd.c was fixed that way:
>>
>> commit b773827e361952b3f53ac6fa4c4e39ccd632102e
>> Author: Chengming Zhou <zhouchengming@bytedance.com>
>> Date: Fri Mar 4 20:29:04 2022 -0800
>>
>> kselftest/vm: fix tests build with old libc
>> The error message when I build vm tests on debian10 (GLIBC 2.28):
>> userfaultfd.c: In function `userfaultfd_pagemap_test':
>> userfaultfd.c:1393:37: error: `MADV_PAGEOUT' undeclared (first use
>> in this function); did you mean `MADV_RANDOM'?
>> if (madvise(area_dst, test_pgsize, MADV_PAGEOUT))
>> ^~~~~~~~~~~~
>> MADV_RANDOM
>> This patch includes these newer definitions from UAPI linux/mman.h, is
>> useful to fix tests build on systems without these definitions in
>> glibc
>> sys/mman.h.
>>
>>
>> [1] https://lkml.kernel.org/r/20230109171255.488749-1-david@redhat.com
>
> You're the boss :)
Heh, no I'm not :) I'm just raising that this turned out to be
problematic unfortunately.
>
> However, IMHO, having MADV_PAGEOUT defined in three or four places could
> make like miserable. OK, it is unlikely to change value, but something
> tells me that the right way to do it is to guarantee that the macro
> definition is unique.
>
> I don't know what would be the right thing to do in pre-5.4 kernels
> w/o MADV_PAGEOUT defined.
>
> Probably then the "(madvise(area_dst, test_pgsize, MADV_PAGEOUT)" gives
> EINVAL or is undefined?
Yes. The expectation is that it fails with ENOSYS or EINVAL if the
kernel doesn't support it. The kernel might be different to the
installed kernel headers.
In an ideal world, we'd be using the in-tree headers -- they are
guaranteed to define what we need. So far, I failed to make it work (I
thought it would work as expected before I had to do above mentioned
fix). Maybe that can be made working?
--
Thanks,
David / dhildenb
prev parent reply other threads:[~2023-01-10 12:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-08 10:58 Mirsad Goran Todorovac
2023-01-09 16:42 ` David Hildenbrand
2023-01-09 21:41 ` Mirsad Goran Todorovac
2023-01-10 10:05 ` David Hildenbrand
2023-01-10 12:25 ` Mirsad Todorovac
2023-01-10 12:29 ` David Hildenbrand [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a0985987-384a-d44f-c365-b0b2b935d417@redhat.com \
--to=david@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mirsad.todorovac@alu.unizg.hr \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®