On Wed, Nov 13, 2019 at 04:07:22PM -0800, Dan Williams wrote: > After the removal of the device-public infrastructure there are only 2 > ->page_free() call backs in the kernel. One of those is a device-private > callback in the nouveau driver, the other is a generic wakeup needed in > the DAX case. In the hopes that all ->page_free() callbacks can be > migrated to common core kernel functionality, move the device-private > specific actions in __put_devmap_managed_page() under the > is_device_private_page() conditional, including the ->page_free() > callback. For the other page types just open-code the generic wakeup. > > Yes, the wakeup is only needed in the MEMORY_DEVICE_FSDAX case, but it > does no harm in the MEMORY_DEVICE_DEVDAX and MEMORY_DEVICE_PCI_P2PDMA > case. > > Cc: Jan Kara <[email protected]> > Cc: Christoph Hellwig <[email protected]> > Cc: Ira Weiny <[email protected]> > Cc: Jérôme Glisse <[email protected]> > Cc: John Hubbard <[email protected]> > Signed-off-by: Dan Williams <[email protected]>
All looks good to me. Reviewed-by: Jérôme Glisse <[email protected]> > --- > Hi John, > > This applies on top of today's linux-next and passes my nvdimm unit > tests. That testing noticed that devmap_managed_enable_get() needed a > small fixup as well. > > drivers/nvdimm/pmem.c | 6 ------ > mm/memremap.c | 22 ++++++++++++---------- > 2 files changed, 12 insertions(+), 16 deletions(-) > > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > index f9f76f6ba07b..21db1ce8c0ae 100644 > --- a/drivers/nvdimm/pmem.c > +++ b/drivers/nvdimm/pmem.c > @@ -338,13 +338,7 @@ static void pmem_release_disk(void *__pmem) > put_disk(pmem->disk); > } > > -static void pmem_pagemap_page_free(struct page *page) > -{ > - wake_up_var(&page->_refcount); > -} > - > static const struct dev_pagemap_ops fsdax_pagemap_ops = { > - .page_free = pmem_pagemap_page_free, > .kill = pmem_pagemap_kill, > .cleanup = pmem_pagemap_cleanup, > }; > diff --git a/mm/memremap.c b/mm/memremap.c > index 022e78e68ea0..6e6f3d6fdb73 100644 > --- a/mm/memremap.c > +++ b/mm/memremap.c > @@ -27,7 +27,8 @@ static void devmap_managed_enable_put(void) > > static int devmap_managed_enable_get(struct dev_pagemap *pgmap) > { > - if (!pgmap->ops || !pgmap->ops->page_free) { > + if (!pgmap->ops || (pgmap->type == MEMORY_DEVICE_PRIVATE > + && !pgmap->ops->page_free)) { > WARN(1, "Missing page_free method\n"); > return -EINVAL; > } > @@ -449,12 +450,6 @@ void __put_devmap_managed_page(struct page *page) > * holds a reference on the page. > */ > if (count == 1) { > - /* Clear Active bit in case of parallel mark_page_accessed */ > - __ClearPageActive(page); > - __ClearPageWaiters(page); > - > - mem_cgroup_uncharge(page); > - > /* > * When a device_private page is freed, the page->mapping field > * may still contain a (stale) mapping value. For example, the > @@ -476,10 +471,17 @@ void __put_devmap_managed_page(struct page *page) > * handled differently or not done at all, so there is no need > * to clear page->mapping. > */ > - if (is_device_private_page(page)) > - page->mapping = NULL; > + if (is_device_private_page(page)) { > + /* Clear Active bit in case of parallel > mark_page_accessed */ > + __ClearPageActive(page); > + __ClearPageWaiters(page); > > - page->pgmap->ops->page_free(page); > + mem_cgroup_uncharge(page); > + > + page->mapping = NULL; > + page->pgmap->ops->page_free(page); > + } else > + wake_up_var(&page->_refcount); > } else if (!count) > __put_page(page); > } > _______________________________________________ Linux-nvdimm mailing list -- [email protected] To unsubscribe send an email to [email protected]
