On Mon, May 18, 2020 at 12:42 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > On Sun, 17 May 2020 06:44:52 -0700 Shakeel Butt wrote: > > > Use one counter for number of pages allocated under pressure > > > to save struct task space and avoid two separate hierarchy > > > walks on the hot path. > > > > The above para seems out of place. It took some time to realize you > > are talking about current->memcg_nr_pages_over_high. IMO instead of > > this para, a comment in code would be much better. > > Where would you like to see the comment? In struct task or where > counter is bumped? > I think the place where the counter is bumped. > > > Take the new high limit into account when determining if swap > > > is "full". Borrowing the explanation from Johannes: > > > > > > The idea behind "swap full" is that as long as the workload has plenty > > > of swap space available and it's not changing its memory contents, it > > > makes sense to generously hold on to copies of data in the swap > > > device, even after the swapin. A later reclaim cycle can drop the page > > > without any IO. Trading disk space for IO. > > > > > > But the only two ways to reclaim a swap slot is when they're faulted > > > in and the references go away, or by scanning the virtual address space > > > like swapoff does - which is very expensive (one could argue it's too > > > expensive even for swapoff, it's often more practical to just reboot). > > > > > > So at some point in the fill level, we have to start freeing up swap > > > slots on fault/swapin. > > > > swap.high allows the user to force the kernel to start freeing swap > > slots before half-full heuristic, right? > > I'd say that the definition of full is extended to include swap.high.