On 11/13/19 2:55 PM, Dan Williams wrote:
On Wed, Nov 13, 2019 at 2:49 PM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
On 11/13/19 2:00 PM, Dan Williams wrote:
...
Ugh, when did all this HMM specific manipulation sneak into the
generic ZONE_DEVICE path? It used to be gated by pgmap type with its
own put_zone_device_private_page(). For example it's certainly
unnecessary and might be broken (would need to check) to call
mem_cgroup_uncharge() on a DAX page. ZONE_DEVICE users are not a
monolith and the HMM use case leaks pages into code paths that DAX
explicitly avoids.
It's been this way for a while and I did not react previously,
apologies for that. I think __ClearPageActive, __ClearPageWaiters, and
mem_cgroup_uncharge, belong behind a device-private conditional. The
history here is:
Move some, but not all HMM specifics to hmm_devmem_free():
2fa147bdbf67 mm, dev_pagemap: Do not clear ->mapping on final put
Remove the clearing of mapping since no upstream consumers needed it:
b7a523109fb5 mm: don't clear ->mapping in hmm_devmem_free
Add it back in once an upstream consumer arrived:
7ab0ad0e74f8 mm/hmm: fix ZONE_DEVICE anon page mapping reuse
We're now almost entirely free of ->page_free callbacks except for
that weird nouveau case, can that FIXME in nouveau_dmem_page_free()
also result in killing the ->page_free() callback altogether? In the
meantime I'm proposing a cleanup like this:
OK, assuming this is acceptable (no obvious problems jump out at me,
and we can also test it with HMM), then how would you like to proceed, as
far as patches go: add such a patch as part of this series here, or as a
stand-alone patch either before or after this series? Or something else?
And did you plan on sending it out as such?
I think it makes sense to include it in your series since you're
looking to refactor the implementation. I can send you one based on
current linux-next as a lead-in cleanup before the refactor. Does that
work for you?
That would be perfect!
Also, the diffs didn't quite make it through intact to my "git apply", so
I'm re-posting the diff in hopes that this time it survives:
Apologies for that. For quick "how about this" patch examples, I just
copy and paste into gmail and it sometimes clobbers it.
No problem at all, I do the same thing and *usually* it works. ha. And
as you say, it's good enough for discussions.
thanks,
--
John Hubbard
NVIDIA
