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

Compile-time detection of MADV_FREE availability is unfortunate on Linux #780

Closed
RedBeard0531 opened this issue Mar 16, 2016 · 5 comments
Closed

Comments

@RedBeard0531
Copy link
Contributor

Currently the detection of whether the system supports MADV_FREE is done at compile time: https://github.com/gperftools/gperftools/blob/gperftools-2.5/src/system-alloc.cc#L65-L70. This means that if the compiling system supports it, MADV_FREE will be used instead of MADV_DONTNEED. Executables compiled on such a system will not be able to return memory to the OS when run on a system that doesn't support MADV_FREE.

This is a realistic situation now that Linux has added support for MADV_FREE in 4.5 (http://kernelnewbies.org/LinuxChanges section 1.6). Long term, it would be ideal if tcmalloc could use runtime detection of MADV_FREE availability with a fallback to MADV_DONTNEED. As a short-term solution, it is probably worth always using MADV_DONTNEED on linux, possibly with a compile-time option for users who know they will only run on systems that support MADV_FREE.

@alk
Copy link
Contributor

alk commented Mar 16, 2016

Great point. I've missed this thing that we're detecting it at compile time.

In fact I don't want us to use MADV_FREE without further testing. IMHO
lack of clear signal "we're using this pages again" in this entire approach
is suboptimal (the risk is that active page ends up on inactive list
without good reason; compromising overall kswapd efficiency). And I'd
prefer API closer to what windows or andoid have with explicit "please feel
free to take this pages" and "hey I want them back".

So in addition to runtime detection I'd like us to have runtime on/off
switch for that.

If you can offer a quick patch that adds runtime detection (compile time
might still be needed so that constant for MADV_FREE is known) and
ability to enable/disable at runtime I'd be happy.

Thanks.

On Wed, Mar 16, 2016 at 1:49 PM, Mathias Stearn notifications@github.com
wrote:

Currently the detection of whether the system supports MADV_FREE is done
at compile time:
https://github.com/gperftools/gperftools/blob/gperftools-2.5/src/system-alloc.cc#L65-L70.
This means that if the compiling system supports it, MADV_FREE will be used
instead of MADV_DONTNEED. Executables compiled on such a system will not be
able to return memory to the OS when run on a system that doesn't support
MADV_FREE.

This is a realistic situation now that Linux has added support for
MADV_FREE in 4.5 (http://kernelnewbies.org/LinuxChanges section 1.6).
Long term, it would be ideal if tcmalloc could use runtime detection of
MADV_FREE availability with a fallback to MADV_DONTNEED. As a short-term
solution, it is probably worth always using MADV_DONTNEED on linux,
possibly with a compile-time option for users who know they will only run
on systems that support MADV_FREE.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#780

RedBeard0531 added a commit to RedBeard0531/gperftools that referenced this issue Mar 16, 2016
@RedBeard0531
Copy link
Contributor Author

Sorry, I don't have time for that right now, nor am I quite clear on how you'd like that to work. For now I submitted #781 which implements a short-term solution of just pretending that linux doesn't have MADV_FREE.

alk added a commit that referenced this issue Mar 20, 2016
Building with -DTCMALLOC_USE_MADV_FREE will enable usage of MADV_FREE on
Linux if glibc copy of kernel headers has MADV_FREE defined.

I.e. so that people can test this more easily.

Affects ticket #780.
@alk
Copy link
Contributor

alk commented Mar 20, 2016

Applied. Thanks.

I've also added subsequent commit that adds define to enable usage of MADV_FREE. I.e. so that testing it is easier.

@lathiat
Copy link

lathiat commented Sep 8, 2022

For future travellers that may be digging into a related Issue, Linux MADV_FREE had a race condition where Direct I/O reads may incorrectly return a zeroed page due a race between MADV_FREE and Direct I/O that happens under memory pressure.

Noticed and solved due to being hit as checksum failures in Ceph:
https://tracker.ceph.com/issues/22464

Fixed in Linux v5.18+ here with a very comprehensive and detailed commit message:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6c8e2a256915a223f6289f651d6b926cd7135c9e (also with stable backports to many older versions)

Notably was being hit on Ubuntu 18.04 as it shipped gperftools v2.5 without this change to disable MADV_FREE but a new enough kernel to hit the issue.

@alk
Copy link
Contributor

alk commented Jul 3, 2023

Closing this ticket as I think we should keep doing plain MADV_DONTNEED.

@alk alk closed this as completed Jul 3, 2023
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

No branches or pull requests

3 participants