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.
