From: Dan Williams <dan.j.williams@intel.com> To: jhubbard@nvidia.com Cc: "Jan Kara" <jack@suse.cz>, "Christoph Hellwig" <hch@lst.de>, "Jérôme Glisse" <jglisse@redhat.com>, linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: [PATCH] mm: Cleanup __put_devmap_managed_page() vs ->page_free() Date: Wed, 13 Nov 2019 16:07:22 -0800 [thread overview] Message-ID: <157368992671.2974225.13512647385398246617.stgit@dwillia2-desk3.amr.corp.intel.com> (raw) 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 <jack@suse.cz> Cc: Christoph Hellwig <hch@lst.de> Cc: Ira Weiny <ira.weiny@intel.com> Cc: Jérôme Glisse <jglisse@redhat.com> Cc: John Hubbard <jhubbard@nvidia.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- 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 -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com> To: jhubbard@nvidia.com Cc: "Jan Kara" <jack@suse.cz>, "Christoph Hellwig" <hch@lst.de>, "Ira Weiny" <ira.weiny@intel.com>, "Jérôme Glisse" <jglisse@redhat.com>, linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: [PATCH] mm: Cleanup __put_devmap_managed_page() vs ->page_free() Date: Wed, 13 Nov 2019 16:07:22 -0800 [thread overview] Message-ID: <157368992671.2974225.13512647385398246617.stgit@dwillia2-desk3.amr.corp.intel.com> (raw) 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 <jack@suse.cz> Cc: Christoph Hellwig <hch@lst.de> Cc: Ira Weiny <ira.weiny@intel.com> Cc: Jérôme Glisse <jglisse@redhat.com> Cc: John Hubbard <jhubbard@nvidia.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- 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); }
next reply other threads:[~2019-11-14 0:21 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-14 0:07 Dan Williams [this message] 2019-11-14 0:07 ` [PATCH] mm: Cleanup __put_devmap_managed_page() vs ->page_free() Dan Williams 2019-11-14 0:39 ` John Hubbard 2019-11-14 0:39 ` John Hubbard 2019-11-14 0:47 ` Dan Williams 2019-11-14 0:47 ` Dan Williams 2019-11-14 7:23 ` Christoph Hellwig 2019-11-14 7:23 ` Christoph Hellwig 2019-11-14 7:26 ` John Hubbard 2019-11-14 7:26 ` John Hubbard 2019-11-14 1:24 ` Jerome Glisse 2019-11-14 1:24 ` Jerome Glisse 2019-11-14 7:19 ` Christoph Hellwig 2019-11-14 7:19 ` Christoph Hellwig 2019-11-14 7:25 ` Dan Williams 2019-11-14 7:25 ` Dan Williams
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=157368992671.2974225.13512647385398246617.stgit@dwillia2-desk3.amr.corp.intel.com \ --to=dan.j.williams@intel.com \ --cc=hch@lst.de \ --cc=jack@suse.cz \ --cc=jglisse@redhat.com \ --cc=jhubbard@nvidia.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-nvdimm@lists.01.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.