On 1/10/23 16:43, David Hildenbrand wrote:
On 10.01.23 16:06, Mirsad Todorovac wrote:
On 1/10/23 15:28, David Hildenbrand wrote:
On 10.01.23 15:24, Mirsad Todorovac wrote:
On 1/10/23 15:04, David Hildenbrand wrote:
Of course, we can easily test with "gcc -E" what exactly gets included.
We don't seem to have a uniform way of handling MADV_PAGEOUT and
MADV_POPULATE(READ|WRITE) even within different C sources within
tools/testing/selftests/vm itself, so it means that we spread the spectrum
of systems and compilations that we will break. :-/
To repeat what is obvious, I am not an authority on the Linux kernel,
but as I said, I have the benefit of a fresh perspective ...
There was an open source proverb that "more eyes see more" :)
The thing is: I don't lose sleeps about defines that will never change as long as it compiles. That's the most important part (for
me) with these tests. Even if it would change, and I don't see how, we'd get test failures and could fix it up.
... and I raised that I ran into issues with what you propose. That's really all I am saying :)
And to repeat what I said: If we can find a cleanup for all these relevant "ifndef" in selftests/mm I'd be very happy. As long
as it
compiles I'm all for cleanups. It would also be acceptable to say "what David tested was wrong" -- but that needs clear
justification.
Then I rest my case.
Just to assert:
Probably if MADV_PAGEOUT or MADV_POPULATE_(READ|WRITE) isn't defined in in-tree
includes, then also the kernel wouldn't support them.
#include <sys/mman.h>
#include <linux/mman.h>
#ifdef MADV_PAGEOUT
if (swapout) {
madvise(mem, pagesize, MADV_PAGEOUT);
if (!pagemap_is_swapped(pagemap_fd, mem)) {
ksft_test_result_skip("MADV_PAGEOUT did not work, is swap enabled?\n");
goto munmap;
}
}
#endif /* MADV_PAGEOUT */
... the above seems like the silver bullet that would catch all cases.
That wouldn't have allowed me to run the tests properly in the crazy sparc64 environment, where the kernel is new but the headers
are old :)
Didn't see that one coming :-/
I don't have a sparc64 box available for testing. Or at all.
Me neither, I was using a qemu-system-sparc64 to test some of my adventurous changes like :
https://lkml.kernel.org/r/20221212130213.136267-1-david@xxxxxxxxxx
and
https://lkml.kernel.org/r/20221206144730.163732-1-david@xxxxxxxxxx
This requires time to analyse. I am not into sparc assembly since about the 2000s.
I tend to sometimes OCD on detail and lose the big picture. That's why "the war is
made by multitude of counselors".
I am not territorial about any of my patches, so what the community thinks its the
best ... This is not my day job, but a pet project, and I like the way it makes my
brain tick. :-)
Again, I'd be very happy if we could just get the in-kernel headers considered properly. But I gave up on that and decided for the
"easy" fixes.
Actually, I admire your work on THP and I won't use more of your time in vain.
I appreciate your review :)
Don't mention it :)
Just to remind you of the advice Yitro gave to Moses: "Don't judge 2,000,000 people by
yourself - delegate some less important work!" :-)
so ... you're going to make it work with the in-tree kernel headers? :P
I would that challenging, if the task offer stands. :)
Of course, as of original author, you will always have the last word.
Having done some homework, I understood that you are actually doing the VM work
in the kernel proper, so naturally it would be prudent to appreciate your insight.
With the Linux kernel, just when I thought you have gained some knowledge, I
see that I know virtually nothing :-/
Thanks,
Mirsad
--
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
