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