Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1326337

Re: [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live pages

Show all headers | View raw

On Wed, 3 Feb 2016, Johannes Weiner wrote:

> CCing Hugh and Greg, they have worked on the memcg migration code most
> recently. AFAIK the only reason newpage->mem_cgroup had to be set up
> that early in migration was because of the way dirty accounting used
> to work. But Hugh took memcg out of the equation there, so moving
> mem_cgroup_migrate() to the end should be safe, as long as the pages
> are still locked and off the LRU.

Yes, that should be safe now: Vladimir's patch looks okay to me,
fixing the immediate irq issue.

But it would be nicer, if mem_cgroup_migrate() were called solely
from migrate_page_copy() - deleting the other calls in mm/migrate.c,
including that from migrate_misplaced_transhuge_page() (which does
some rewinding on error after its migrate_page_copy(): but just as
you now let a successfully migrated old page be uncharged when it's
freed, so you can leave a failed new_page to be uncharged when it's
freed, no extra code needed).

And (even more off-topic), I'm slightly sad to see that the lrucare
arg which mem_cgroup_migrate() used to have (before I renamed it and
you renamed it back!) has gone, so mem_cgroup_migrate() now always
demands lrucare of commit_charge().  I'd hoped that with your
separation of new from old charge, mem_cgroup_migrate() would never
need lrucare; but that's not true for the fuse case, though true
for everyone else.  Maybe just not worth bothering about?  Or the
reintroduction of some unnecessary zone->lru_lock-ing in page
migration, which we ought to try to avoid?

Or am I wrong, and even fuse doesn't need it?  That early return
"if (newpage->mem_cgroup)": isn't mem_cgroup_migrate() a no-op for
fuse, or is there some corner case by which newpage can be on LRU
but its mem_cgroup unset?

Hugh

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/3] mm: memcontrol: simplify page->mem_cgroup pinning Johannes Weiner <hannes@cmpxchg.org> - 2016-01-30 00:30 +0100
  [PATCH 3/3] mm: remove unnecessary uses of lock_page_memcg() Johannes Weiner <hannes@cmpxchg.org> - 2016-01-30 00:30 +0100
    Re: [PATCH 3/3] mm: remove unnecessary uses of lock_page_memcg() Vladimir Davydov <vdavydov@virtuozzo.com> - 2016-02-03 10:30 +0100
  [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live pages Johannes Weiner <hannes@cmpxchg.org> - 2016-01-30 00:30 +0100
    Re: [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live  pages Vladimir Davydov <vdavydov@virtuozzo.com> - 2016-02-03 10:30 +0100
    Re: [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live  pages Mateusz Guzik <mguzik@redhat.com> - 2016-02-03 14:20 +0100
      Re: [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live  pages Vladimir Davydov <vdavydov@virtuozzo.com> - 2016-02-03 15:10 +0100
        Re: [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live  pages Johannes Weiner <hannes@cmpxchg.org> - 2016-02-03 19:40 +0100
          Re: [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live  pages Hugh Dickins <hughd@google.com> - 2016-02-04 02:40 +0100
            Re: [PATCH 1/3] mm: migrate: do not touch page->mem_cgroup of live  pages Johannes Weiner <hannes@cmpxchg.org> - 2016-02-04 21:00 +0100

csiph-web