All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeelb@google.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,  Kernel Team <kernel-team@fb.com>,
	Tejun Heo <tj@kernel.org>, Johannes Weiner <hannes@cmpxchg.org>,
	 Chris Down <chris@chrisdown.name>,
	Cgroups <cgroups@vger.kernel.org>,
	 Michal Hocko <mhocko@kernel.org>
Subject: Re: [PATCH mm v3 3/3] mm: automatically penalize tasks with high swap use
Date: Mon, 18 May 2020 12:58:01 -0700	[thread overview]
Message-ID: <CALvZod7AMhBKCGXB4FuZYTzYcgeMypCXSWqyzJSzyQDGqm8bOA@mail.gmail.com> (raw)
In-Reply-To: <20200518124210.37335d0b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Mon, May 18, 2020 at 12:42 PM Jakub Kicinski <kuba@kernel.org> 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.


WARNING: multiple messages have this Message-ID (diff)
From: Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
To: Jakub Kicinski <kuba-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Linux MM <linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org>,
	Kernel Team <kernel-team-b10kYP2dOMg@public.gmane.org>,
	Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>,
	Chris Down <chris-6Bi1550iOqEnzZ6mRAm98g@public.gmane.org>,
	Cgroups <cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH mm v3 3/3] mm: automatically penalize tasks with high swap use
Date: Mon, 18 May 2020 12:58:01 -0700	[thread overview]
Message-ID: <CALvZod7AMhBKCGXB4FuZYTzYcgeMypCXSWqyzJSzyQDGqm8bOA@mail.gmail.com> (raw)
In-Reply-To: <20200518124210.37335d0b-CbGlpYbfshD2+ikPA6JUzH8MC5kE+Go+7OQnqPW8cmTNHmdVIy2bbWItS4zQEDct@public.gmane.org>

On Mon, May 18, 2020 at 12:42 PM Jakub Kicinski <kuba-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 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.

  reply	other threads:[~2020-05-18 19:58 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15 20:20 [PATCH mm v3 0/3] memcg: Slow down swap allocation as the available space gets depleted Jakub Kicinski
2020-05-15 20:20 ` Jakub Kicinski
2020-05-15 20:20 ` [PATCH mm v3 1/3] mm: prepare for swap over-high accounting and penalty calculation Jakub Kicinski
2020-05-15 20:20   ` Jakub Kicinski
2020-05-15 20:20 ` [PATCH mm v3 2/3] mm: move penalty delay clamping out of calculate_high_delay() Jakub Kicinski
2020-05-15 20:20   ` Jakub Kicinski
2020-05-15 20:20 ` [PATCH mm v3 3/3] mm: automatically penalize tasks with high swap use Jakub Kicinski
2020-05-15 20:20   ` Jakub Kicinski
2020-05-17 13:44   ` Shakeel Butt
2020-05-17 13:44     ` Shakeel Butt
2020-05-18 19:42     ` Jakub Kicinski
2020-05-18 19:42       ` Jakub Kicinski
2020-05-18 19:58       ` Shakeel Butt [this message]
2020-05-18 19:58         ` Shakeel Butt
2020-05-19  0:42     ` Jakub Kicinski
2020-05-19  0:42       ` Jakub Kicinski
2020-05-19  1:10       ` Shakeel Butt
2020-05-19  1:10         ` Shakeel Butt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALvZod7AMhBKCGXB4FuZYTzYcgeMypCXSWqyzJSzyQDGqm8bOA@mail.gmail.com \
    --to=shakeelb@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=chris@chrisdown.name \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@fb.com \
    --cc=kuba@kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.