Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include: posix.mman += MADV_* (MADV_NORMAL, MADV_DONTNEED, etc ...) #3012

Merged
merged 1 commit into from Jun 29, 2019

Conversation

navytux
Copy link
Contributor

@navytux navytux commented Jun 24, 2019

I was going to use posix_madvise(POSIX_MADV_DONTNEED) in my tests to
make sure a memory page should be evicted from kernel cache, but found
out that this operation is hardcoded to be NOOP on Glibc/Linux systems:

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/posix_madvise.c;h=c89fa64f0749;hb=HEAD#l25
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8889e7aa461a

On the other hand, as also verified by strace, madvise(MADV_DONTNEED)
is not a NOOP as it actually goes to kernel by making syscall.

Since it is not possible to use posix_madvise for what should be
POSIX_MADV_DONTNEED behaviour, let's add all raw madvise behaviour flags
as documented on http://man7.org/linux/man-pages/man2/madvise.2.html

Presumably MADV_NORMAL and other behaviours that should translate 1-1 to
POSIX ones were omitted in 59ac999 (Add pxd for mmap & friends
from "sys/mman.h" covering POSIX/Linux/BSD) to make people use the
standardised POSIX interface. However given that it turned out to be not
possible to use posix_madvise for all standard behaviours,
system-specific madvise behaviours have to be used.

I was going to use posix_madvise(POSIX_MADV_DONTNEED) in my tests to
make sure a memory page should be evicted from kernel cache, but found
out that this operation is hardcoded to be NOOP on Glibc/Linux systems:

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/posix_madvise.c;h=c89fa64f0749;hb=HEAD#l25
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8889e7aa461a

On the other hand, as also verified by strace, `madvise(MADV_DONTNEED)`
is not a NOOP as it actually goes to kernel by making syscall.

Since it is not possible to use posix_madvise for what should be
POSIX_MADV_DONTNEED behaviour, let's add all raw madvise behaviour flags
as documented on http://man7.org/linux/man-pages/man2/madvise.2.html

Presumably MADV_NORMAL and other behaviours that should translate 1-1 to
POSIX ones were omitted in 59ac999 (Add pxd for mmap & friends
from "sys/mman.h" covering POSIX/Linux/BSD) to make people use the
standardised POSIX interface. However given that it turned out to be not
possible to use posix_madvise for all standard behaviours,
system-specific madvise behaviours have to be used.
@navytux
Copy link
Contributor Author

navytux commented Jun 24, 2019

( CI failures: while most variants passed tests ok, if I read logs correctly, there are several failures due to some network or travis flakiness - the failures are not related to the patch )

@navytux
Copy link
Contributor Author

navytux commented Jun 27, 2019

@scoder, CI failure here is untrue; please have a look.

Thanks beforehand,
Kirill

@scoder scoder merged commit b394295 into cython:master Jun 29, 2019
@navytux
Copy link
Contributor Author

navytux commented Jun 29, 2019

Thanks

@navytux navytux deleted the y/madv branch June 29, 2019 10:22
navytux added a commit to navytux/cython that referenced this pull request Jul 7, 2019
…ythonGH-3012)

I was going to use posix_madvise(POSIX_MADV_DONTNEED) in my tests to
make sure a memory page should be evicted from kernel cache, but found
out that this operation is hardcoded to be NOOP on Glibc/Linux systems:

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/posix_madvise.c;h=c89fa64f0749;hb=HEAD#l25
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8889e7aa461a

On the other hand, as also verified by strace, `madvise(MADV_DONTNEED)`
is not a NOOP as it actually goes to kernel by making syscall.

Since it is not possible to use posix_madvise for what should be
POSIX_MADV_DONTNEED behaviour, let's add all raw madvise behaviour flags
as documented on http://man7.org/linux/man-pages/man2/madvise.2.html

Presumably MADV_NORMAL and other behaviours that should translate 1-1 to
POSIX ones were omitted in 59ac999 (Add pxd for mmap & friends
from "sys/mman.h" covering POSIX/Linux/BSD) to make people use the
standardised POSIX interface. However given that it turned out to be not
possible to use posix_madvise for all standard behaviours,
system-specific madvise behaviours have to be used.

(cherry picked from commit b394295)
scoder pushed a commit that referenced this pull request Jul 19, 2019
…H-3012)

I was going to use posix_madvise(POSIX_MADV_DONTNEED) in my tests to
make sure a memory page should be evicted from kernel cache, but found
out that this operation is hardcoded to be NOOP on Glibc/Linux systems:

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/posix_madvise.c;h=c89fa64f0749;hb=HEAD#l25
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8889e7aa461a

On the other hand, as also verified by strace, `madvise(MADV_DONTNEED)`
is not a NOOP as it actually goes to kernel by making syscall.

Since it is not possible to use posix_madvise for what should be
POSIX_MADV_DONTNEED behaviour, let's add all raw madvise behaviour flags
as documented on http://man7.org/linux/man-pages/man2/madvise.2.html

Presumably MADV_NORMAL and other behaviours that should translate 1-1 to
POSIX ones were omitted in 59ac999 (Add pxd for mmap & friends
from "sys/mman.h" covering POSIX/Linux/BSD) to make people use the
standardised POSIX interface. However given that it turned out to be not
possible to use posix_madvise for all standard behaviours,
system-specific madvise behaviours have to be used.

(cherry picked from commit b394295)
@scoder scoder added this to the 0.29.13 milestone Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants