On 10/12/20 6:28 AM, Johannes Weiner wrote:
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@xxxxxxxxxx> 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@xxxxxxxxxxx>
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.
Thanks for the Acked-by and the comments.
I assume Andrew will update the tags when queuing this up unless he wants me to resend.
