From: Johannes Weiner <hannes@cmpxchg.org>
To: Ralph Campbell <rcampbell@nvidia.com>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-kernel@vger.kernel.org, "Michal Hocko" <mhocko@kernel.org>,
"Vladimir Davydov" <vdavydov.dev@gmail.com>,
"Jérôme Glisse" <jglisse@redhat.com>,
"Balbir Singh" <bsingharora@gmail.com>,
"Ira Weiny" <ira.weiny@intel.com>,
stable@vger.kernel.org
Subject: Re: [PATCH] mm/memcg: fix device private memcg accounting
Date: Mon, 12 Oct 2020 09:28:59 -0400 [thread overview]
Message-ID: <20201012132859.GD163830@cmpxchg.org> (raw)
In-Reply-To: <d1aab0b0-4327-38da-6587-98f1740228fd@nvidia.com>
On Fri, Oct 09, 2020 at 05:00:37PM -0700, Ralph Campbell wrote:
>
> On 10/9/20 3:50 PM, Andrew Morton wrote:
> > On Fri, 9 Oct 2020 14:59:52 -0700 Ralph Campbell <rcampbell@nvidia.com> wrote:
> >
> > > The code in mc_handle_swap_pte() checks for non_swap_entry() and returns
> > > NULL before checking is_device_private_entry() so device private pages
> > > are never handled.
> > > Fix this by checking for non_swap_entry() after handling device private
> > > swap PTEs.
The fix looks good to me.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > But this makes me suspect the answer is "there aren't any that we know
> > of". Are you sure a cc:stable is warranted?
> >
>
> I assume the memory cgroup accounting would be off somehow when moving
> a process to another memory cgroup.
> Currently, the device private page is charged like a normal anonymous page
> when allocated and is uncharged when the page is freed so I think that path is OK.
> Maybe someone who knows more about memory cgroup accounting can comment?
As for whether to CC stable, I'm leaning toward no:
- When moving tasks, we'd leave their device pages behind in the old
cgroup. This isn't great, but it doesn't cause counter imbalances or
corruption or anything - we also skip locked pages, we used to skip
pages mapped by more than one pte, the user can select whether to
move pages along tasks at all, and if so, whether only anon or file.
- Charge moving itself is a bit of a questionable feature, and users
have been moving away from it. Leaving tasks in a cgroup and
changing the configuration is a heck of a lot cheaper than moving
potentially gigabytes of pages to another configuration domain.
- According to the Fixes tag, this isn't a regression, either. Since
their inception, we have never migrated device pages.
next prev parent reply other threads:[~2020-10-12 13:30 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-09 21:59 Ralph Campbell
2020-10-09 22:50 ` Andrew Morton
2020-10-10 0:00 ` Ralph Campbell
2020-10-12 13:28 ` Johannes Weiner [this message]
2020-10-12 17:11 ` Ralph Campbell
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=20201012132859.GD163830@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=bsingharora@gmail.com \
--cc=cgroups@vger.kernel.org \
--cc=ira.weiny@intel.com \
--cc=jglisse@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=rcampbell@nvidia.com \
--cc=stable@vger.kernel.org \
--cc=vdavydov.dev@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox