On Mon, Jan 6, 2025 at 8:28 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > On Sun 22-12-24 10:34:12, Yafang Shao wrote: > > On Sat, Dec 21, 2024 at 3:21 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > > On Fri 20-12-24 19:52:16, Yafang Shao wrote: > > > > On Fri, Dec 20, 2024 at 6:23 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > > > > > > On Sun 15-12-24 15:34:13, Yafang Shao wrote: > > > > > > Implementation Options > > > > > > ---------------------- > > > > > > > > > > > > - Solution A: Allow file caches on the unevictable list to become > > > > > > reclaimable. > > > > > > This approach would require significant refactoring of the page reclaim > > > > > > logic. > > > > > > > > > > > > - Solution B: Prevent file caches from being moved to the unevictable list > > > > > > during mlock and ignore the VM_LOCKED flag during page reclaim. > > > > > > This is a more straightforward solution and is the one we have chosen. > > > > > > If the file caches are reclaimed from the download-proxy's memcg and > > > > > > subsequently accessed by tasks in the application’s memcg, a filemap > > > > > > fault will occur. A new file cache will be faulted in, charged to the > > > > > > application’s memcg, and locked there. > > > > > > > > > > Both options are silently breaking userspace because a non failing mlock > > > > > doesn't give guarantees it is supposed to AFAICS. > > > > > > > > It does not bypass the mlock mechanism; rather, it defers the actual > > > > locking operation to the page fault path. Could you clarify what you > > > > mean by "a non-failing mlock"? From what I can see, mlock can indeed > > > > fail if there isn’t sufficient memory available. With this change, we > > > > are simply shifting the potential failure point to the page fault path > > > > instead. > > > > > > Your change will cause mlocked pages (as mlock syscall returns success) > > > to be reclaimable later on. That breaks the basic mlock contract. > > > > AFAICS, the mlock() behavior was originally designed with only a > > single root memory cgroup in mind. In other words, when mlock() was > > introduced, all locked pages were confined to the same memcg. > > yes and this is the case to any other syscalls that might have an impact > on the memory consumption. This is by design. Memory cgroup controller > aims to provide a completely transparent resource control without any > modifications to applications. This is the case for all other cgroup > controllers. If memcg (or other controller) affects a specific syscall > behavior then this has to be communicated explicitly to the caller. > > The purpose of mlock syscall is to _guarantee_ memory to be resident > (never swapped out). There might be additional constrains to prevent > from mlock succeeding - e.g. rlimit or if memcg aims to control amount > of the mlocked memory but those failures need to be explicitly > communicated via syscall failure. Returning an error code like EBUSY to userspace is straightforward when attempting to mlock a page that is charged to a different memcg. > > > However, this changed with the introduction of memcg support. Now, > > mlock() can lock pages that belong to a different memcg than the > > current task. This behavior is not explicitly defined in the mlock() > > documentation, which could lead to confusion. > > This is more of a problem of the cgroup configurations where different > resource domains are sharing resources. This is not much diffent when > other resources (e.g. shmem) are shared accross unrelated cgroups. However, we have yet to address even a single one of these issues or reach a consensus on a solution, correct? > > > To clarify, I propose updating the mlock() documentation as follows: > > This is not really possible because you are effectively breaking an > existing userspace. This behavior is neither mandatory nor the default. You are not obligated to use it if you prefer not to. -- Regards Yafang
