Vlastimil Babka <vbabka@xxxxxxx> wrote: > On 10/17/24 22:57, Jeff Xu wrote: > > On Thu, Oct 17, 2024 at 1:49 PM Pedro Falcato <pedro.falcato@xxxxxxxxx> wrote: > >> > > >> > > > For file-backed, private, read-only memory mappings, we previously did > >> > > > not block the madvise(MADV_DONTNEED). This was based on > >> > > > the assumption that the memory's content, being file-backed, could be > >> > > > retrieved from the file if accessed again. However, this assumption > >> > > > failed to consider scenarios where a mapping is initially created as > >> > > > read-write, modified, and subsequently changed to read-only. The newly > >> > > > introduced VM_WASWRITE flag addresses this oversight. > >> > > > >> > > We *do not* need this. It's sufficient to just block discard operations on read-only > >> > > private mappings. > >> > I think you meant blocking madvise(MADV_DONTNEED) on all read-only > >> > private file-backed mappings. > >> > > >> > I considered that option, but there is a use case for madvise on those > >> > mappings that never get modified. > >> > > >> > Apps can use that to free up RAM. e.g. Considering read-only .text > >> > section, which never gets modified, madvise( MADV_DONTNEED) can free > >> > up RAM when memory is in-stress, memory will be reclaimed from a > >> > backed-file on next read access. Therefore we can't just block all > >> > read-only private file-backed mapping, only those that really need to, > >> > such as mapping changed from rw=>r (what you described) > >> > >> Does anyone actually do this? If so, why? WHYYYY? > >> > > This is a legit use case, I can't argue that it isn't. > > Could the same effect be simply achieved with MADV_COLD/MADV_PAGEOUT? That > should be able to reclaim the pages as well if they are indeed not used, but > it's non-destructive and you don't want to allow destructive madvise anyway > (i.e. no throwing away data that would be replaced by zeroes or original > file content on the next touch) so it seems overall a better fit for sealed > areas? Comment from the sidelines: That seems clever enough.
