search for: page_free

...>> cal init_page_count() or set_page_count() since get_page() calls >> VM_BUG_ON_PAGE() if refcount == 0. >> I'll move set_page_refcounted() since that is what the page allocator >> uses and seems named for the purpose. > > I don't think any of the three ->page_free instances even cares about > the page refcount. > Not true. The page_free() callback records the page is free by setting a bit or putting the page on a free list but when it allocates a free device private struct page to be used with migrate_vma_setup(), it needs to increment the refcount....

...reeze_queue(struct percpu_ref *ref) +static void pmem_kill(struct percpu_ref *ref) { struct request_queue *q; @@ -339,19 +339,27 @@ static void pmem_release_pgmap_ops(void *__pgmap) dev_pagemap_put_ops(); } -static void fsdax_pagefree(struct page *page, void *data) +static void pmem_fsdax_page_free(struct page *page, void *data) { wake_up_var(&page->_refcount); } +static const struct dev_pagemap_ops fsdax_pagemap_ops = { + .page_free = pmem_fsdax_page_free, + .kill = pmem_kill, +}; + +static const struct dev_pagemap_ops pmem_legacy_pagemap_ops = { + .kill = pmem_kill, +}; +...

...e(struct percpu_ref *ref) +static void pmem_pagemap_kill(struct percpu_ref *ref) { struct request_queue *q; @@ -347,19 +347,24 @@ static void pmem_release_pgmap_ops(void *__pgmap) dev_pagemap_put_ops(); } -static void fsdax_pagefree(struct page *page, void *data) +static void pmem_pagemap_page_free(struct page *page, void *data) { 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, +}; + static int setup_pagemap_fsdax(struct device *d...

...ges never >> - * reach 0 refcount unless there is a refcount bug. This allows the >> - * device driver to implement its own memory management.) >> + * Called once the page refcount reaches 0. The reference count is >> + * reset to 1 before calling page_free(). This allows the >> + * device driver to implement its own memory management. > > I'd clarify the order events / responsibility of the common core > page_free() and the device specific page_free(). At the same time, why > not update drivers to expect that the page is...

On 9/15/20 10:36 PM, Christoph Hellwig wrote: > On Tue, Sep 15, 2020 at 09:39:47AM -0700, Ralph Campbell wrote: >>> I don't think any of the three ->page_free instances even cares about >>> the page refcount. >>> >> Not true. The page_free() callback records the page is free by setting >> a bit or putting the page on a free list but when it allocates a free >> device private struct page to be used with migrate_vma_setu...

...* Called once the page refcount reaches 1. (ZONE_DEVICE pages never - * reach 0 refcount unless there is a refcount bug. This allows the - * device driver to implement its own memory management.) + * Called once the page refcount reaches 0. The reference count is + * reset to 1 before calling page_free(). This allows the + * device driver to implement its own memory management. */ void (*page_free)(struct page *page); diff --git a/include/linux/mm.h b/include/linux/mm.h index 517751310dd2..5a82037a4b26 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1093,34 +1093,6 @@ static...

On Wed, Jun 26, 2019 at 02:27:10PM +0200, Christoph Hellwig wrote: > Just check if there is a ->page_free operation set and take care of the > static key enable, as well as the put using device managed resources. > Also check that a ->page_free is provided for the pgmaps types that > require it, and check for a valid type as well while we are at it. > > Note that this also fixes the...

Just check if there is a ->page_free operation set and take care of the static key enable, as well as the put using device managed resources. Also check that a ->page_free is provided for the pgmaps types that require it, and check for a valid type as well while we are at it. Note that this also fixes the fact that hmm never calle...

...ions(+), 8 deletions(-) diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 66837eed6375..847d1b2bc10e 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -334,7 +334,7 @@ static void pmem_release_disk(void *__pmem) put_disk(pmem->disk); } -static void pmem_fsdax_page_free(struct page *page, void *data) +static void pmem_fsdax_page_free(struct page *page) { wake_up_var(&page->_refcount); } diff --git a/include/linux/memremap.h b/include/linux/memremap.h index 03a4099be701..75b80de6394a 100644 --- a/include/linux/memremap.h +++ b/include/linux/memremap.h @@...

Just check if there is a ->page_free operation set and take care of the static key enable, as well as the put using device managed resources. Also check that a ->page_free is provided for the pgmaps types that require it, and check for a valid type as well while we are at it. Note that this also fixes the fact that hmm never calle...

On Fri, Jun 28, 2019 at 11:52 AM Christoph Hellwig <hch at lst.de> wrote: > > On Fri, Jun 28, 2019 at 11:44:35AM -0700, Dan Williams wrote: > > There is a problem with the series in CH's tree. It removes the > > ->page_free() callback from the release_pages() path because it goes > > too far and removes the put_devmap_managed_page() call. > > release_pages only called put_devmap_managed_page for device public > pages. So I can't see how that is in any way a problem. It's a bug that the call to...

...(ZONE_DEVICE pages never > - * reach 0 refcount unless there is a refcount bug. This allows the > - * device driver to implement its own memory management.) > + * Called once the page refcount reaches 0. The reference count is > + * reset to 1 before calling page_free(). This allows the > + * device driver to implement its own memory management. I'd clarify the order events / responsibility of the common core page_free() and the device specific page_free(). At the same time, why not update drivers to expect that the page is already refcount==0 on...

...mm.git it will go after CH's > series (and Ira will have to rebase/repost it), so I think there is > nothing to do at this moment - unless you are saying there is a > problem with the series in CH's git tree? There is a problem with the series in CH's tree. It removes the ->page_free() callback from the release_pages() path because it goes too far and removes the put_devmap_managed_page() call.

Just check if there is a ->page_free operation set and take care of the static key enable, as well as the put using device managed resources. Signed-off-by: Christoph Hellwig <hch at lst.de> --- drivers/nvdimm/pmem.c | 23 +++-------------- include/linux/mm.h | 10 -------- kernel/memremap.c | 59 +++++++++++++++++++++++...

On Tue, Sep 15, 2020 at 09:39:47AM -0700, Ralph Campbell wrote: >> I don't think any of the three ->page_free instances even cares about >> the page refcount. >> > Not true. The page_free() callback records the page is free by setting > a bit or putting the page on a free list but when it allocates a free > device private struct page to be used with migrate_vma_setup(), it needs to &gt...

...T_SYMBOL(devmap_managed_key); - -static void devmap_managed_enable_put(void) -{ - static_branch_dec(&devmap_managed_key); -} - -static int devmap_managed_enable_get(struct dev_pagemap *pgmap) -{ - if (pgmap->type == MEMORY_DEVICE_PRIVATE && - (!pgmap->ops || !pgmap->ops->page_free)) { - WARN(1, "Missing page_free method\n"); - return -EINVAL; - } - - static_branch_inc(&devmap_managed_key); - return 0; -} -#else -static int devmap_managed_enable_get(struct dev_pagemap *pgmap) -{ - return -EINVAL; -} -static void devmap_managed_enable_put(void) -{ -} -#endif /*...

...void devmap_managed_enable_put(void) > -{ > - static_branch_dec(&devmap_managed_key); > -} > - > -static int devmap_managed_enable_get(struct dev_pagemap *pgmap) > -{ > - if (pgmap->type == MEMORY_DEVICE_PRIVATE && > - (!pgmap->ops || !pgmap->ops->page_free)) { > - WARN(1, "Missing page_free method\n"); > - return -EINVAL; > - } > - > - static_branch_inc(&devmap_managed_key); > - return 0; > -} > -#else > -static int devmap_managed_enable_get(struct dev_pagemap *pgmap) > -{ > - return -EINVAL; > -} &...

...* device driver to implement its own memory management.) + * Called once the page refcount reaches 0. The reference count + * should be reset to one with init_page_count(page) before reusing + * the page. This allows the device driver to implement its own + * memory management. */ void (*page_free)(struct page *page); diff --git a/include/linux/mm.h b/include/linux/mm.h index 27c64d0d7520..3d71a820ae38 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1100,39 +1100,6 @@ static inline bool is_zone_device_page(const struct page *page) } #endif -#ifdef CONFIG_DEV_PAGEMAP_OPS -v...

...address range covered by @ref * @ref: reference count that pins the devm_memremap_pages() mapping + * @internal_ref: internal reference if @ref is not provided by the caller + * @done: completion for @internal_ref * @dev: host device of the mapping for debug * @data: private data pointer for page_free() * @type: memory type: see MEMORY_* in memory_hotplug.h @@ -105,6 +107,8 @@ struct dev_pagemap { struct vmem_altmap altmap; struct resource res; struct percpu_ref *ref; + struct percpu_ref internal_ref; + struct completion done; struct device *dev; enum memory_type type; unsigned int...

Hi Dan, Jérôme and Jason, below is a series that cleans up the dev_pagemap interface so that it is more easily usable, which removes the need to wrap it in hmm and thus allowing to kill a lot of code Note: this series is on top of the rdma/hmm branch + the dev_pagemap releas fix series from Dan that went into 5.2-rc5. Git tree: git://git.infradead.org/users/hch/misc.git