* [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Jérôme Glisse @ 2017-05-22 16:51 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse,
Kirill A . Shutemov, Dan Williams, Ross Zwisler
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
have any user. For device private pages this is important to catch
and thus we need to special case put_page() for this.
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
---
include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
kernel/memremap.c | 1 -
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a825dab..11f7bac 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -23,6 +23,7 @@
#include <linux/page_ext.h>
#include <linux/err.h>
#include <linux/page_ref.h>
+#include <linux/memremap.h>
struct mempolicy;
struct anon_vma;
@@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
return ((page_zonenum(page) == ZONE_DEVICE) &&
(page->pgmap->type == MEMORY_DEVICE_PRIVATE));
}
+
+static inline void put_zone_device_private_page(struct page *page)
+{
+ int count = page_ref_dec_return(page);
+
+ /*
+ * If refcount is 1 then page is freed and refcount is stable as nobody
+ * holds a reference on the page.
+ */
+ if (count == 1)
+ page->pgmap->page_free(page, page->pgmap->data);
+ else if (!count)
+ __put_page(page);
+}
#else
static inline bool is_zone_device_page(const struct page *page)
{
@@ -805,6 +820,10 @@ static inline bool is_device_private_page(const struct page *page)
{
return false;
}
+
+static inline void put_zone_device_private_page(struct page *page)
+{
+}
#endif
static inline void get_page(struct page *page)
@@ -822,6 +841,17 @@ static inline void put_page(struct page *page)
{
page = compound_head(page);
+ /*
+ * For private device pages we need to catch refcount transition from
+ * 2 to 1, when refcount reach one it means the private device page is
+ * free and we need to inform the device driver through callback. See
+ * include/linux/memremap.h and HMM for details.
+ */
+ if (unlikely(is_device_private_page(page))) {
+ put_zone_device_private_page(page);
+ return;
+ }
+
if (put_page_testzero(page))
__put_page(page);
}
diff --git a/kernel/memremap.c b/kernel/memremap.c
index dbdb656..71f6f28 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -11,7 +11,6 @@
* General Public License for more details.
*/
#include <linux/radix-tree.h>
-#include <linux/memremap.h>
#include <linux/device.h>
#include <linux/types.h>
#include <linux/pfn_t.h>
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [HMM 09/15] mm/hmm/devmem: device memory hotplug using ZONE_DEVICE v4
From: Jérôme Glisse @ 2017-05-22 16:52 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse,
Evgeny Baskakov, Mark Hairgrove, Sherry Cheung, Subhash Gutti
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
This introduce a simple struct and associated helpers for device driver
to use when hotpluging un-addressable device memory as ZONE_DEVICE. It
will find a unuse physical address range and trigger memory hotplug for
it which allocates and initialize struct page for the device memory.
Changed since v3:
- s/device unaddressable/device private/
Changed since v2:
- s/SECTION_SIZE/PA_SECTION_SIZE
Changed since v1:
- change to adapt to new add_pages() helper
- make this x86-64 only for now
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Signed-off-by: Evgeny Baskakov <ebaskakov@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
Signed-off-by: Sherry Cheung <SCheung@nvidia.com>
Signed-off-by: Subhash Gutti <sgutti@nvidia.com>
---
include/linux/hmm.h | 114 +++++++++++++++
mm/Kconfig | 9 ++
mm/hmm.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 536 insertions(+), 1 deletion(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 248a6e0..0865afd 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -72,6 +72,11 @@
#if IS_ENABLED(CONFIG_HMM)
+#include <linux/migrate.h>
+#include <linux/memremap.h>
+#include <linux/completion.h>
+
+
struct hmm;
/*
@@ -322,6 +327,115 @@ int hmm_vma_fault(struct vm_area_struct *vma,
#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
+#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+struct hmm_devmem;
+
+struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
+ unsigned long addr);
+
+/*
+ * struct hmm_devmem_ops - callback for ZONE_DEVICE memory events
+ *
+ * @free: call when refcount on page reach 1 and thus is no longer use
+ * @fault: call when there is a page fault to unaddressable memory
+ */
+struct hmm_devmem_ops {
+ void (*free)(struct hmm_devmem *devmem, struct page *page);
+ int (*fault)(struct hmm_devmem *devmem,
+ struct vm_area_struct *vma,
+ unsigned long addr,
+ struct page *page,
+ unsigned int flags,
+ pmd_t *pmdp);
+};
+
+/*
+ * struct hmm_devmem - track device memory
+ *
+ * @completion: completion object for device memory
+ * @pfn_first: first pfn for this resource (set by hmm_devmem_add())
+ * @pfn_last: last pfn for this resource (set by hmm_devmem_add())
+ * @resource: IO resource reserved for this chunk of memory
+ * @pagemap: device page map for that chunk
+ * @device: device to bind resource to
+ * @ops: memory operations callback
+ * @ref: per CPU refcount
+ *
+ * This an helper structure for device drivers that do not wish to implement
+ * the gory details related to hotplugging new memoy and allocating struct
+ * pages.
+ *
+ * Device drivers can directly use ZONE_DEVICE memory on their own if they
+ * wish to do so.
+ */
+struct hmm_devmem {
+ struct completion completion;
+ unsigned long pfn_first;
+ unsigned long pfn_last;
+ struct resource *resource;
+ struct device *device;
+ struct dev_pagemap pagemap;
+ const struct hmm_devmem_ops *ops;
+ struct percpu_ref ref;
+};
+
+/*
+ * To add (hotplug) device memory, HMM assumes that there is no real resource
+ * that reserves a range in the physical address space (this is intended to be
+ * use by unaddressable device memory). It will reserve a physical range big
+ * enough and allocate struct page for it.
+ *
+ * The device driver can wrap the hmm_devmem struct inside a private device
+ * driver struct. The device driver must call hmm_devmem_remove() before the
+ * device goes away and before freeing the hmm_devmem struct memory.
+ */
+struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
+ struct device *device,
+ unsigned long size);
+void hmm_devmem_remove(struct hmm_devmem *devmem);
+
+int hmm_devmem_fault_range(struct hmm_devmem *devmem,
+ struct vm_area_struct *vma,
+ const struct migrate_vma_ops *ops,
+ unsigned long *src,
+ unsigned long *dst,
+ unsigned long start,
+ unsigned long addr,
+ unsigned long end,
+ void *private);
+
+/*
+ * hmm_devmem_page_set_drvdata - set per-page driver data field
+ *
+ * @page: pointer to struct page
+ * @data: driver data value to set
+ *
+ * Because page can not be on lru we have an unsigned long that driver can use
+ * to store a per page field. This just a simple helper to do that.
+ */
+static inline void hmm_devmem_page_set_drvdata(struct page *page,
+ unsigned long data)
+{
+ unsigned long *drvdata = (unsigned long *)&page->pgmap;
+
+ drvdata[1] = data;
+}
+
+/*
+ * hmm_devmem_page_get_drvdata - get per page driver data field
+ *
+ * @page: pointer to struct page
+ * Return: driver data value
+ */
+static inline unsigned long hmm_devmem_page_get_drvdata(struct page *page)
+{
+ unsigned long *drvdata = (unsigned long *)&page->pgmap;
+
+ return drvdata[1];
+}
+#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
+
+
/* Below are for HMM internal use only! Not to be used by device driver! */
void hmm_mm_destroy(struct mm_struct *mm);
diff --git a/mm/Kconfig b/mm/Kconfig
index d49d816..feb1eb2 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -314,6 +314,15 @@ config HMM_MIRROR
page tables (at PAGE_SIZE granularity), and must be able to recover from
the resulting potential page faults.
+config HMM_DEVMEM
+ bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
+ depends on ARCH_HAS_HMM
+ select HMM
+ help
+ HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
+ feature. This is just to avoid having device drivers to replicating a lot
+ of boiler plate code. See Documentation/vm/hmm.txt.
+
config PHYS_ADDR_T_64BIT
def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
diff --git a/mm/hmm.c b/mm/hmm.c
index ed97051..939a62e 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -23,9 +23,15 @@
#include <linux/swap.h>
#include <linux/slab.h>
#include <linux/sched.h>
+#include <linux/mmzone.h>
+#include <linux/pagemap.h>
#include <linux/swapops.h>
#include <linux/hugetlb.h>
+#include <linux/memremap.h>
#include <linux/mmu_notifier.h>
+#include <linux/memory_hotplug.h>
+
+#define PA_SECTION_SIZE (1UL << PA_SECTION_SHIFT)
static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
@@ -427,7 +433,15 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
* This is a special swap entry, ignore migration, use
* device and report anything else as error.
*/
- if (is_migration_entry(entry)) {
+ if (is_device_private_entry(entry)) {
+ pfns[i] = hmm_pfn_t_from_pfn(swp_offset(entry));
+ if (is_write_device_private_entry(entry)) {
+ pfns[i] |= HMM_PFN_WRITE;
+ } else if (write_fault)
+ goto fault;
+ pfns[i] |= HMM_PFN_DEVICE_UNADDRESSABLE;
+ pfns[i] |= flag;
+ } else if (is_migration_entry(entry)) {
if (hmm_vma_walk->fault) {
pte_unmap(ptep);
hmm_vma_walk->last = addr;
@@ -721,3 +735,401 @@ int hmm_vma_fault(struct vm_area_struct *vma,
}
EXPORT_SYMBOL(hmm_vma_fault);
#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
+
+
+#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
+ unsigned long addr)
+{
+ struct page *page;
+
+ page = alloc_page_vma(GFP_HIGHUSER, vma, addr);
+ if (!page)
+ return NULL;
+ lock_page(page);
+ return page;
+}
+EXPORT_SYMBOL(hmm_vma_alloc_locked_page);
+
+
+static void hmm_devmem_ref_release(struct percpu_ref *ref)
+{
+ struct hmm_devmem *devmem;
+
+ devmem = container_of(ref, struct hmm_devmem, ref);
+ complete(&devmem->completion);
+}
+
+static void hmm_devmem_ref_exit(void *data)
+{
+ struct percpu_ref *ref = data;
+ struct hmm_devmem *devmem;
+
+ devmem = container_of(ref, struct hmm_devmem, ref);
+ percpu_ref_exit(ref);
+ devm_remove_action(devmem->device, &hmm_devmem_ref_exit, data);
+}
+
+static void hmm_devmem_ref_kill(void *data)
+{
+ struct percpu_ref *ref = data;
+ struct hmm_devmem *devmem;
+
+ devmem = container_of(ref, struct hmm_devmem, ref);
+ percpu_ref_kill(ref);
+ wait_for_completion(&devmem->completion);
+ devm_remove_action(devmem->device, &hmm_devmem_ref_kill, data);
+}
+
+static int hmm_devmem_fault(struct vm_area_struct *vma,
+ unsigned long addr,
+ struct page *page,
+ unsigned int flags,
+ pmd_t *pmdp)
+{
+ struct hmm_devmem *devmem = page->pgmap->data;
+
+ return devmem->ops->fault(devmem, vma, addr, page, flags, pmdp);
+}
+
+static void hmm_devmem_free(struct page *page, void *data)
+{
+ struct hmm_devmem *devmem = data;
+
+ devmem->ops->free(devmem, page);
+}
+
+static DEFINE_MUTEX(hmm_devmem_lock);
+static RADIX_TREE(hmm_devmem_radix, GFP_KERNEL);
+
+static void hmm_devmem_radix_release(struct resource *resource)
+{
+ resource_size_t key, align_start, align_size, align_end;
+
+ align_start = resource->start & ~(PA_SECTION_SIZE - 1);
+ align_size = ALIGN(resource_size(resource), PA_SECTION_SIZE);
+ align_end = align_start + align_size - 1;
+
+ mutex_lock(&hmm_devmem_lock);
+ for (key = resource->start;
+ key <= resource->end;
+ key += PA_SECTION_SIZE)
+ radix_tree_delete(&hmm_devmem_radix, key >> PA_SECTION_SHIFT);
+ mutex_unlock(&hmm_devmem_lock);
+}
+
+static void hmm_devmem_release(struct device *dev, void *data)
+{
+ struct hmm_devmem *devmem = data;
+ struct resource *resource = devmem->resource;
+ unsigned long start_pfn, npages;
+ struct zone *zone;
+ struct page *page;
+
+ if (percpu_ref_tryget_live(&devmem->ref)) {
+ dev_WARN(dev, "%s: page mapping is still live!\n", __func__);
+ percpu_ref_put(&devmem->ref);
+ }
+
+ /* pages are dead and unused, undo the arch mapping */
+ start_pfn = (resource->start & ~(PA_SECTION_SIZE - 1)) >> PAGE_SHIFT;
+ npages = ALIGN(resource_size(resource), PA_SECTION_SIZE) >> PAGE_SHIFT;
+
+ page = pfn_to_page(start_pfn);
+ zone = page_zone(page);
+
+ mem_hotplug_begin();
+ __remove_pages(zone, start_pfn, npages);
+ mem_hotplug_done();
+
+ hmm_devmem_radix_release(resource);
+}
+
+static struct hmm_devmem *hmm_devmem_find(resource_size_t phys)
+{
+ WARN_ON_ONCE(!rcu_read_lock_held());
+
+ return radix_tree_lookup(&hmm_devmem_radix, phys >> PA_SECTION_SHIFT);
+}
+
+static int hmm_devmem_pages_create(struct hmm_devmem *devmem)
+{
+ resource_size_t key, align_start, align_size, align_end;
+ struct device *device = devmem->device;
+ int ret, nid, is_ram;
+ unsigned long pfn;
+
+ align_start = devmem->resource->start & ~(PA_SECTION_SIZE - 1);
+ align_size = ALIGN(devmem->resource->start +
+ resource_size(devmem->resource),
+ PA_SECTION_SIZE) - align_start;
+
+ is_ram = region_intersects(align_start, align_size,
+ IORESOURCE_SYSTEM_RAM,
+ IORES_DESC_NONE);
+ if (is_ram == REGION_MIXED) {
+ WARN_ONCE(1, "%s attempted on mixed region %pr\n",
+ __func__, devmem->resource);
+ return -ENXIO;
+ }
+ if (is_ram == REGION_INTERSECTS)
+ return -ENXIO;
+
+ devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
+ devmem->pagemap.res = devmem->resource;
+ devmem->pagemap.page_fault = hmm_devmem_fault;
+ devmem->pagemap.page_free = hmm_devmem_free;
+ devmem->pagemap.dev = devmem->device;
+ devmem->pagemap.ref = &devmem->ref;
+ devmem->pagemap.data = devmem;
+
+ mutex_lock(&hmm_devmem_lock);
+ align_end = align_start + align_size - 1;
+ for (key = align_start; key <= align_end; key += PA_SECTION_SIZE) {
+ struct hmm_devmem *dup;
+
+ rcu_read_lock();
+ dup = hmm_devmem_find(key);
+ rcu_read_unlock();
+ if (dup) {
+ dev_err(device, "%s: collides with mapping for %s\n",
+ __func__, dev_name(dup->device));
+ mutex_unlock(&hmm_devmem_lock);
+ ret = -EBUSY;
+ goto error;
+ }
+ ret = radix_tree_insert(&hmm_devmem_radix,
+ key >> PA_SECTION_SHIFT,
+ devmem);
+ if (ret) {
+ dev_err(device, "%s: failed: %d\n", __func__, ret);
+ mutex_unlock(&hmm_devmem_lock);
+ goto error_radix;
+ }
+ }
+ mutex_unlock(&hmm_devmem_lock);
+
+ nid = dev_to_node(device);
+ if (nid < 0)
+ nid = numa_mem_id();
+
+ mem_hotplug_begin();
+ ret = add_pages(nid, align_start >> PAGE_SHIFT,
+ align_size >> PAGE_SHIFT, false);
+ if (ret) {
+ mem_hotplug_done();
+ goto error_add_memory;
+ }
+ move_pfn_range_to_zone(&NODE_DATA(nid)->node_zones[ZONE_DEVICE],
+ align_start >> PAGE_SHIFT,
+ align_size >> PAGE_SHIFT);
+ mem_hotplug_done();
+
+ for (pfn = devmem->pfn_first; pfn < devmem->pfn_last; pfn++) {
+ struct page *page = pfn_to_page(pfn);
+
+ /*
+ * ZONE_DEVICE pages union ->lru with a ->pgmap back
+ * pointer. It is a bug if a ZONE_DEVICE page is ever
+ * freed or placed on a driver-private list. Therefore,
+ * seed the storage with LIST_POISON* values.
+ */
+ list_del(&page->lru);
+ page->pgmap = &devmem->pagemap;
+ }
+ return 0;
+
+error_add_memory:
+ untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
+error_radix:
+ hmm_devmem_radix_release(devmem->resource);
+error:
+ return ret;
+}
+
+static int hmm_devmem_match(struct device *dev, void *data, void *match_data)
+{
+ struct hmm_devmem *devmem = data;
+
+ return devmem->resource == match_data;
+}
+
+static void hmm_devmem_pages_remove(struct hmm_devmem *devmem)
+{
+ devres_release(devmem->device, &hmm_devmem_release,
+ &hmm_devmem_match, devmem->resource);
+}
+
+/*
+ * hmm_devmem_add() - hotplug ZONE_DEVICE memory for device memory
+ *
+ * @ops: memory event device driver callback (see struct hmm_devmem_ops)
+ * @device: device struct to bind the resource too
+ * @size: size in bytes of the device memory to add
+ * Returns: pointer to new hmm_devmem struct ERR_PTR otherwise
+ *
+ * This first finds an empty range of physical address big enough to contain the
+ * new resource, and then hotplugs it as ZONE_DEVICE memory, which in turn
+ * allocates struct pages. It does not do anything beyond that; all events
+ * affecting the memory will go through the various callbacks provided by
+ * hmm_devmem_ops struct.
+ */
+struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
+ struct device *device,
+ unsigned long size)
+{
+ struct hmm_devmem *devmem;
+ resource_size_t addr;
+ int ret;
+
+ devmem = devres_alloc_node(&hmm_devmem_release, sizeof(*devmem),
+ GFP_KERNEL, dev_to_node(device));
+ if (!devmem)
+ return ERR_PTR(-ENOMEM);
+
+ init_completion(&devmem->completion);
+ devmem->pfn_first = -1UL;
+ devmem->pfn_last = -1UL;
+ devmem->resource = NULL;
+ devmem->device = device;
+ devmem->ops = ops;
+
+ ret = percpu_ref_init(&devmem->ref, &hmm_devmem_ref_release,
+ 0, GFP_KERNEL);
+ if (ret)
+ goto error_percpu_ref;
+
+ ret = devm_add_action(device, hmm_devmem_ref_exit, &devmem->ref);
+ if (ret)
+ goto error_devm_add_action;
+
+ size = ALIGN(size, PA_SECTION_SIZE);
+ addr = min((unsigned long)iomem_resource.end, 1UL << MAX_PHYSMEM_BITS);
+ addr = addr - size + 1UL;
+
+ /*
+ * FIXME add a new helper to quickly walk resource tree and find free
+ * range
+ *
+ * FIXME what about ioport_resource resource ?
+ */
+ for (; addr > size && addr >= iomem_resource.start; addr -= size) {
+ ret = region_intersects(addr, size, 0, IORES_DESC_NONE);
+ if (ret != REGION_DISJOINT)
+ continue;
+
+ devmem->resource = devm_request_mem_region(device, addr, size,
+ dev_name(device));
+ if (!devmem->resource) {
+ ret = -ENOMEM;
+ goto error_no_resource;
+ }
+ break;
+ }
+ if (!devmem->resource) {
+ ret = -ERANGE;
+ goto error_no_resource;
+ }
+
+ devmem->resource->desc = IORES_DESC_DEVICE_PRIVATE_MEMORY;
+ devmem->pfn_first = devmem->resource->start >> PAGE_SHIFT;
+ devmem->pfn_last = devmem->pfn_first +
+ (resource_size(devmem->resource) >> PAGE_SHIFT);
+
+ ret = hmm_devmem_pages_create(devmem);
+ if (ret)
+ goto error_pages;
+
+ devres_add(device, devmem);
+
+ ret = devm_add_action(device, hmm_devmem_ref_kill, &devmem->ref);
+ if (ret) {
+ hmm_devmem_remove(devmem);
+ return ERR_PTR(ret);
+ }
+
+ return devmem;
+
+error_pages:
+ devm_release_mem_region(device, devmem->resource->start,
+ resource_size(devmem->resource));
+error_no_resource:
+error_devm_add_action:
+ hmm_devmem_ref_kill(&devmem->ref);
+ hmm_devmem_ref_exit(&devmem->ref);
+error_percpu_ref:
+ devres_free(devmem);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(hmm_devmem_add);
+
+/*
+ * hmm_devmem_remove() - remove device memory (kill and free ZONE_DEVICE)
+ *
+ * @devmem: hmm_devmem struct use to track and manage the ZONE_DEVICE memory
+ *
+ * This will hot-unplug memory that was hotplugged by hmm_devmem_add on behalf
+ * of the device driver. It will free struct page and remove the resource that
+ * reserved the physical address range for this device memory.
+ */
+void hmm_devmem_remove(struct hmm_devmem *devmem)
+{
+ resource_size_t start, size;
+ struct device *device;
+
+ if (!devmem)
+ return;
+
+ device = devmem->device;
+ start = devmem->resource->start;
+ size = resource_size(devmem->resource);
+
+ hmm_devmem_ref_kill(&devmem->ref);
+ hmm_devmem_ref_exit(&devmem->ref);
+ hmm_devmem_pages_remove(devmem);
+
+ devm_release_mem_region(device, start, size);
+}
+EXPORT_SYMBOL(hmm_devmem_remove);
+
+/*
+ * hmm_devmem_fault_range() - migrate back a virtual range of memory
+ *
+ * @devmem: hmm_devmem struct use to track and manage the ZONE_DEVICE memory
+ * @vma: virtual memory area containing the range to be migrated
+ * @ops: migration callback for allocating destination memory and copying
+ * @src: array of unsigned long containing source pfns
+ * @dst: array of unsigned long containing destination pfns
+ * @start: start address of the range to migrate (inclusive)
+ * @addr: fault address (must be inside the range)
+ * @end: end address of the range to migrate (exclusive)
+ * @private: pointer passed back to each of the callback
+ * Returns: 0 on success, VM_FAULT_SIGBUS on error
+ *
+ * This is a wrapper around migrate_vma() which checks the migration status
+ * for a given fault address and returns the corresponding page fault handler
+ * status. That will be 0 on success, or VM_FAULT_SIGBUS if migration failed
+ * for the faulting address.
+ *
+ * This is a helper intendend to be used by the ZONE_DEVICE fault handler.
+ */
+int hmm_devmem_fault_range(struct hmm_devmem *devmem,
+ struct vm_area_struct *vma,
+ const struct migrate_vma_ops *ops,
+ unsigned long *src,
+ unsigned long *dst,
+ unsigned long start,
+ unsigned long addr,
+ unsigned long end,
+ void *private)
+{
+ if (migrate_vma(ops, vma, start, end, src, dst, private))
+ return VM_FAULT_SIGBUS;
+
+ if (dst[(addr - start) >> PAGE_SHIFT] & MIGRATE_PFN_ERROR)
+ return VM_FAULT_SIGBUS;
+
+ return 0;
+}
+EXPORT_SYMBOL(hmm_devmem_fault_range);
+#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [HMM 10/15] mm/hmm/devmem: dummy HMM device for ZONE_DEVICE memory v3
From: Jérôme Glisse @ 2017-05-22 16:52 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse,
Evgeny Baskakov, Mark Hairgrove, Sherry Cheung, Subhash Gutti
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
This introduce a dummy HMM device class so device driver can use it to
create hmm_device for the sole purpose of registering device memory.
It is useful to device driver that want to manage multiple physical
device memory under same struct device umbrella.
Changed since v2:
- use device_initcall() and drop everything that is module specific
Changed since v1:
- Improve commit message
- Add drvdata parameter to set on struct device
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Signed-off-by: Evgeny Baskakov <ebaskakov@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
Signed-off-by: Sherry Cheung <SCheung@nvidia.com>
Signed-off-by: Subhash Gutti <sgutti@nvidia.com>
---
include/linux/hmm.h | 22 +++++++++++++-
mm/hmm.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 0865afd..2f2a6ff 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -72,11 +72,11 @@
#if IS_ENABLED(CONFIG_HMM)
+#include <linux/device.h>
#include <linux/migrate.h>
#include <linux/memremap.h>
#include <linux/completion.h>
-
struct hmm;
/*
@@ -433,6 +433,26 @@ static inline unsigned long hmm_devmem_page_get_drvdata(struct page *page)
return drvdata[1];
}
+
+
+/*
+ * struct hmm_device - fake device to hang device memory onto
+ *
+ * @device: device struct
+ * @minor: device minor number
+ */
+struct hmm_device {
+ struct device device;
+ unsigned int minor;
+};
+
+/*
+ * A device driver that wants to handle multiple devices memory through a
+ * single fake device can use hmm_device to do so. This is purely a helper and
+ * it is not strictly needed, in order to make use of any HMM functionality.
+ */
+struct hmm_device *hmm_device_new(void *drvdata);
+void hmm_device_put(struct hmm_device *hmm_device);
#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
diff --git a/mm/hmm.c b/mm/hmm.c
index 939a62e..d1816dbe 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -19,6 +19,7 @@
*/
#include <linux/mm.h>
#include <linux/hmm.h>
+#include <linux/init.h>
#include <linux/rmap.h>
#include <linux/swap.h>
#include <linux/slab.h>
@@ -1132,4 +1133,91 @@ int hmm_devmem_fault_range(struct hmm_devmem *devmem,
return 0;
}
EXPORT_SYMBOL(hmm_devmem_fault_range);
+
+/*
+ * A device driver that wants to handle multiple devices memory through a
+ * single fake device can use hmm_device to do so. This is purely a helper
+ * and it is not needed to make use of any HMM functionality.
+ */
+#define HMM_DEVICE_MAX 256
+
+static DECLARE_BITMAP(hmm_device_mask, HMM_DEVICE_MAX);
+static DEFINE_SPINLOCK(hmm_device_lock);
+static struct class *hmm_device_class;
+static dev_t hmm_device_devt;
+
+static void hmm_device_release(struct device *device)
+{
+ struct hmm_device *hmm_device;
+
+ hmm_device = container_of(device, struct hmm_device, device);
+ spin_lock(&hmm_device_lock);
+ clear_bit(hmm_device->minor, hmm_device_mask);
+ spin_unlock(&hmm_device_lock);
+
+ kfree(hmm_device);
+}
+
+struct hmm_device *hmm_device_new(void *drvdata)
+{
+ struct hmm_device *hmm_device;
+ int ret;
+
+ hmm_device = kzalloc(sizeof(*hmm_device), GFP_KERNEL);
+ if (!hmm_device)
+ return ERR_PTR(-ENOMEM);
+
+ ret = alloc_chrdev_region(&hmm_device->device.devt, 0, 1, "hmm_device");
+ if (ret < 0) {
+ kfree(hmm_device);
+ return NULL;
+ }
+
+ spin_lock(&hmm_device_lock);
+ hmm_device->minor = find_first_zero_bit(hmm_device_mask, HMM_DEVICE_MAX);
+ if (hmm_device->minor >= HMM_DEVICE_MAX) {
+ spin_unlock(&hmm_device_lock);
+ kfree(hmm_device);
+ return NULL;
+ }
+ set_bit(hmm_device->minor, hmm_device_mask);
+ spin_unlock(&hmm_device_lock);
+
+ dev_set_name(&hmm_device->device, "hmm_device%d", hmm_device->minor);
+ hmm_device->device.devt = MKDEV(MAJOR(hmm_device_devt),
+ hmm_device->minor);
+ hmm_device->device.release = hmm_device_release;
+ dev_set_drvdata(&hmm_device->device, drvdata);
+ hmm_device->device.class = hmm_device_class;
+ device_initialize(&hmm_device->device);
+
+ return hmm_device;
+}
+EXPORT_SYMBOL(hmm_device_new);
+
+void hmm_device_put(struct hmm_device *hmm_device)
+{
+ put_device(&hmm_device->device);
+}
+EXPORT_SYMBOL(hmm_device_put);
+
+static int __init hmm_init(void)
+{
+ int ret;
+
+ ret = alloc_chrdev_region(&hmm_device_devt, 0,
+ HMM_DEVICE_MAX,
+ "hmm_device");
+ if (ret)
+ return ret;
+
+ hmm_device_class = class_create(THIS_MODULE, "hmm_device");
+ if (IS_ERR(hmm_device_class)) {
+ unregister_chrdev_region(hmm_device_devt, HMM_DEVICE_MAX);
+ return PTR_ERR(hmm_device_class);
+ }
+ return 0;
+}
+
+device_initcall(hmm_init);
#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [HMM 11/15] mm/migrate: new migrate mode MIGRATE_SYNC_NO_COPY
From: Jérôme Glisse @ 2017-05-22 16:52 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
Introduce a new migration mode that allow to offload the copy to
a device DMA engine. This changes the workflow of migration and
not all address_space migratepage callback can support this. So
it needs to be tested in those cases.
This is intended to be use by migrate_vma() which itself is use
for thing like HMM (see include/linux/hmm.h).
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
---
fs/aio.c | 8 +++++++
fs/f2fs/data.c | 5 ++++-
fs/hugetlbfs/inode.c | 5 ++++-
fs/ubifs/file.c | 5 ++++-
include/linux/migrate.h | 5 +++++
include/linux/migrate_mode.h | 5 +++++
mm/balloon_compaction.c | 8 +++++++
mm/migrate.c | 52 ++++++++++++++++++++++++++++++++++----------
mm/zsmalloc.c | 8 +++++++
9 files changed, 86 insertions(+), 15 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index f52d925..e51351e 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -373,6 +373,14 @@ static int aio_migratepage(struct address_space *mapping, struct page *new,
pgoff_t idx;
int rc;
+ /*
+ * We cannot support the _NO_COPY case here, because copy needs to
+ * happen under the ctx->completion_lock. That does not work with the
+ * migration workflow of MIGRATE_SYNC_NO_COPY.
+ */
+ if (mode == MIGRATE_SYNC_NO_COPY)
+ return -EINVAL;
+
rc = 0;
/* mapping->private_lock here protects against the kioctx teardown. */
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 1602b4b..7a56446 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2091,7 +2091,10 @@ int f2fs_migrate_page(struct address_space *mapping,
SetPagePrivate(newpage);
set_page_private(newpage, page_private(page));
- migrate_page_copy(newpage, page);
+ if (mode != MIGRATE_SYNC_NO_COPY)
+ migrate_page_copy(newpage, page);
+ else
+ migrate_page_states(newpage, page);
return MIGRATEPAGE_SUCCESS;
}
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index dde8613..c02ff56 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -846,7 +846,10 @@ static int hugetlbfs_migrate_page(struct address_space *mapping,
rc = migrate_huge_page_move_mapping(mapping, newpage, page);
if (rc != MIGRATEPAGE_SUCCESS)
return rc;
- migrate_page_copy(newpage, page);
+ if (mode != MIGRATE_SYNC_NO_COPY)
+ migrate_page_copy(newpage, page);
+ else
+ migrate_page_states(newpage, page);
return MIGRATEPAGE_SUCCESS;
}
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index d9ae86f..c08cbcc 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1482,7 +1482,10 @@ static int ubifs_migrate_page(struct address_space *mapping,
SetPagePrivate(newpage);
}
- migrate_page_copy(newpage, page);
+ if (mode != MIGRATE_SYNC_NO_COPY)
+ migrate_page_copy(newpage, page);
+ else
+ migrate_page_states(newpage, page);
return MIGRATEPAGE_SUCCESS;
}
#endif
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 48e2484..78a0fdc 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -43,6 +43,7 @@ extern void putback_movable_page(struct page *page);
extern int migrate_prep(void);
extern int migrate_prep_local(void);
+extern void migrate_page_states(struct page *newpage, struct page *page);
extern void migrate_page_copy(struct page *newpage, struct page *page);
extern int migrate_huge_page_move_mapping(struct address_space *mapping,
struct page *newpage, struct page *page);
@@ -63,6 +64,10 @@ static inline int isolate_movable_page(struct page *page, isolate_mode_t mode)
static inline int migrate_prep(void) { return -ENOSYS; }
static inline int migrate_prep_local(void) { return -ENOSYS; }
+static inline void migrate_page_states(struct page *newpage, struct page *page)
+{
+}
+
static inline void migrate_page_copy(struct page *newpage,
struct page *page) {}
diff --git a/include/linux/migrate_mode.h b/include/linux/migrate_mode.h
index ebf3d89..bdf66af 100644
--- a/include/linux/migrate_mode.h
+++ b/include/linux/migrate_mode.h
@@ -6,11 +6,16 @@
* on most operations but not ->writepage as the potential stall time
* is too significant
* MIGRATE_SYNC will block when migrating pages
+ * MIGRATE_SYNC_NO_COPY will block when migrating pages but will not copy pages
+ * with the CPU. Instead, page copy happens outside the migratepage()
+ * callback and is likely using a DMA engine. See migrate_vma() and HMM
+ * (mm/hmm.c) for users of this mode.
*/
enum migrate_mode {
MIGRATE_ASYNC,
MIGRATE_SYNC_LIGHT,
MIGRATE_SYNC,
+ MIGRATE_SYNC_NO_COPY,
};
#endif /* MIGRATE_MODE_H_INCLUDED */
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index da91df5..145b903 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -139,6 +139,14 @@ int balloon_page_migrate(struct address_space *mapping,
{
struct balloon_dev_info *balloon = balloon_page_device(page);
+ /*
+ * We can not easily support the no copy case here so ignore it as it
+ * is unlikely to be use with ballon pages. See include/linux/hmm.h for
+ * user of the MIGRATE_SYNC_NO_COPY mode.
+ */
+ if (mode == MIGRATE_SYNC_NO_COPY)
+ return -EINVAL;
+
VM_BUG_ON_PAGE(!PageLocked(page), page);
VM_BUG_ON_PAGE(!PageLocked(newpage), newpage);
diff --git a/mm/migrate.c b/mm/migrate.c
index 051cc15..66410fc 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -603,15 +603,10 @@ static void copy_huge_page(struct page *dst, struct page *src)
/*
* Copy the page to its new location
*/
-void migrate_page_copy(struct page *newpage, struct page *page)
+void migrate_page_states(struct page *newpage, struct page *page)
{
int cpupid;
- if (PageHuge(page) || PageTransHuge(page))
- copy_huge_page(newpage, page);
- else
- copy_highpage(newpage, page);
-
if (PageError(page))
SetPageError(newpage);
if (PageReferenced(page))
@@ -665,6 +660,17 @@ void migrate_page_copy(struct page *newpage, struct page *page)
mem_cgroup_migrate(page, newpage);
}
+EXPORT_SYMBOL(migrate_page_states);
+
+void migrate_page_copy(struct page *newpage, struct page *page)
+{
+ if (PageHuge(page) || PageTransHuge(page))
+ copy_huge_page(newpage, page);
+ else
+ copy_highpage(newpage, page);
+
+ migrate_page_states(newpage, page);
+}
EXPORT_SYMBOL(migrate_page_copy);
/************************************************************
@@ -690,7 +696,10 @@ int migrate_page(struct address_space *mapping,
if (rc != MIGRATEPAGE_SUCCESS)
return rc;
- migrate_page_copy(newpage, page);
+ if (mode != MIGRATE_SYNC_NO_COPY)
+ migrate_page_copy(newpage, page);
+ else
+ migrate_page_states(newpage, page);
return MIGRATEPAGE_SUCCESS;
}
EXPORT_SYMBOL(migrate_page);
@@ -740,12 +749,15 @@ int buffer_migrate_page(struct address_space *mapping,
SetPagePrivate(newpage);
- migrate_page_copy(newpage, page);
+ if (mode != MIGRATE_SYNC_NO_COPY)
+ migrate_page_copy(newpage, page);
+ else
+ migrate_page_states(newpage, page);
bh = head;
do {
unlock_buffer(bh);
- put_bh(bh);
+ put_bh(bh);
bh = bh->b_this_page;
} while (bh != head);
@@ -804,8 +816,13 @@ static int fallback_migrate_page(struct address_space *mapping,
{
if (PageDirty(page)) {
/* Only writeback pages in full synchronous migration */
- if (mode != MIGRATE_SYNC)
+ switch (mode) {
+ case MIGRATE_SYNC:
+ case MIGRATE_SYNC_NO_COPY:
+ break;
+ default:
return -EBUSY;
+ }
return writeout(mapping, page);
}
@@ -942,7 +959,11 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
* the retry loop is too short and in the sync-light case,
* the overhead of stalling is too much
*/
- if (mode != MIGRATE_SYNC) {
+ switch (mode) {
+ case MIGRATE_SYNC:
+ case MIGRATE_SYNC_NO_COPY:
+ break;
+ default:
rc = -EBUSY;
goto out_unlock;
}
@@ -1212,8 +1233,15 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
return -ENOMEM;
if (!trylock_page(hpage)) {
- if (!force || mode != MIGRATE_SYNC)
+ if (!force)
goto out;
+ switch (mode) {
+ case MIGRATE_SYNC:
+ case MIGRATE_SYNC_NO_COPY:
+ break;
+ default:
+ goto out;
+ }
lock_page(hpage);
}
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index d41edd2..aeea3a5 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1983,6 +1983,14 @@ int zs_page_migrate(struct address_space *mapping, struct page *newpage,
unsigned int obj_idx;
int ret = -EAGAIN;
+ /*
+ * We cannot support the _NO_COPY case here, because copy needs to
+ * happen under the zs lock, which does not work with
+ * MIGRATE_SYNC_NO_COPY workflow.
+ */
+ if (mode == MIGRATE_SYNC_NO_COPY)
+ return -EINVAL;
+
VM_BUG_ON_PAGE(!PageMovable(page), page);
VM_BUG_ON_PAGE(!PageIsolated(page), page);
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [HMM 12/15] mm/migrate: new memory migration helper for use with device memory v4
From: Jérôme Glisse @ 2017-05-22 16:52 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse,
Evgeny Baskakov, Mark Hairgrove, Sherry Cheung, Subhash Gutti
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
This patch add a new memory migration helpers, which migrate memory
backing a range of virtual address of a process to different memory
(which can be allocated through special allocator). It differs from
numa migration by working on a range of virtual address and thus by
doing migration in chunk that can be large enough to use DMA engine
or special copy offloading engine.
Expected users are any one with heterogeneous memory where different
memory have different characteristics (latency, bandwidth, ...). As
an example IBM platform with CAPI bus can make use of this feature
to migrate between regular memory and CAPI device memory. New CPU
architecture with a pool of high performance memory not manage as
cache but presented as regular memory (while being faster and with
lower latency than DDR) will also be prime user of this patch.
Migration to private device memory will be useful for device that
have large pool of such like GPU, NVidia plans to use HMM for that.
Changes since v3:
- Rebase
Changes since v2:
- droped HMM prefix and HMM specific code
Changes since v1:
- typos fix
- split early unmap optimization for page with single mapping
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Signed-off-by: Evgeny Baskakov <ebaskakov@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
Signed-off-by: Sherry Cheung <SCheung@nvidia.com>
Signed-off-by: Subhash Gutti <sgutti@nvidia.com>
---
include/linux/migrate.h | 104 ++++++++++++
mm/migrate.c | 444 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 548 insertions(+)
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 78a0fdc..576b3f5 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -127,4 +127,108 @@ static inline int migrate_misplaced_transhuge_page(struct mm_struct *mm,
}
#endif /* CONFIG_NUMA_BALANCING && CONFIG_TRANSPARENT_HUGEPAGE*/
+
+#ifdef CONFIG_MIGRATION
+
+#define MIGRATE_PFN_VALID (1UL << 0)
+#define MIGRATE_PFN_MIGRATE (1UL << 1)
+#define MIGRATE_PFN_LOCKED (1UL << 2)
+#define MIGRATE_PFN_WRITE (1UL << 3)
+#define MIGRATE_PFN_ERROR (1UL << 4)
+#define MIGRATE_PFN_SHIFT 5
+
+static inline struct page *migrate_pfn_to_page(unsigned long mpfn)
+{
+ if (!(mpfn & MIGRATE_PFN_VALID))
+ return NULL;
+ return pfn_to_page(mpfn >> MIGRATE_PFN_SHIFT);
+}
+
+static inline unsigned long migrate_pfn(unsigned long pfn)
+{
+ return (pfn << MIGRATE_PFN_SHIFT) | MIGRATE_PFN_VALID;
+}
+
+/*
+ * struct migrate_vma_ops - migrate operation callback
+ *
+ * @alloc_and_copy: alloc destination memory and copy source memory to it
+ * @finalize_and_map: allow caller to map the successfully migrated pages
+ *
+ *
+ * The alloc_and_copy() callback happens once all source pages have been locked,
+ * unmapped and checked (checked whether pinned or not). All pages that can be
+ * migrated will have an entry in the src array set with the pfn value of the
+ * page and with the MIGRATE_PFN_VALID and MIGRATE_PFN_MIGRATE flag set (other
+ * flags might be set but should be ignored by the callback).
+ *
+ * The alloc_and_copy() callback can then allocate destination memory and copy
+ * source memory to it for all those entries (ie with MIGRATE_PFN_VALID and
+ * MIGRATE_PFN_MIGRATE flag set). Once these are allocated and copied, the
+ * callback must update each corresponding entry in the dst array with the pfn
+ * value of the destination page and with the MIGRATE_PFN_VALID and
+ * MIGRATE_PFN_LOCKED flags set (destination pages must have their struct pages
+ * locked, via lock_page()).
+ *
+ * At this point the alloc_and_copy() callback is done and returns.
+ *
+ * Note that the callback does not have to migrate all the pages that are
+ * marked with MIGRATE_PFN_MIGRATE flag in src array unless this is a migration
+ * from device memory to system memory (ie the MIGRATE_PFN_DEVICE flag is also
+ * set in the src array entry). If the device driver cannot migrate a device
+ * page back to system memory, then it must set the corresponding dst array
+ * entry to MIGRATE_PFN_ERROR. This will trigger a SIGBUS if CPU tries to
+ * access any of the virtual addresses originally backed by this page. Because
+ * a SIGBUS is such a severe result for the userspace process, the device
+ * driver should avoid setting MIGRATE_PFN_ERROR unless it is really in an
+ * unrecoverable state.
+ *
+ * THE alloc_and_copy() CALLBACK MUST NOT CHANGE ANY OF THE SRC ARRAY ENTRIES
+ * OR BAD THINGS WILL HAPPEN !
+ *
+ *
+ * The finalize_and_map() callback happens after struct page migration from
+ * source to destination (destination struct pages are the struct pages for the
+ * memory allocated by the alloc_and_copy() callback). Migration can fail, and
+ * thus the finalize_and_map() allows the driver to inspect which pages were
+ * successfully migrated, and which were not. Successfully migrated pages will
+ * have the MIGRATE_PFN_MIGRATE flag set for their src array entry.
+ *
+ * It is safe to update device page table from within the finalize_and_map()
+ * callback because both destination and source page are still locked, and the
+ * mmap_sem is held in read mode (hence no one can unmap the range being
+ * migrated).
+ *
+ * Once callback is done cleaning up things and updating its page table (if it
+ * chose to do so, this is not an obligation) then it returns. At this point,
+ * the HMM core will finish up the final steps, and the migration is complete.
+ *
+ * THE finalize_and_map() CALLBACK MUST NOT CHANGE ANY OF THE SRC OR DST ARRAY
+ * ENTRIES OR BAD THINGS WILL HAPPEN !
+ */
+struct migrate_vma_ops {
+ void (*alloc_and_copy)(struct vm_area_struct *vma,
+ const unsigned long *src,
+ unsigned long *dst,
+ unsigned long start,
+ unsigned long end,
+ void *private);
+ void (*finalize_and_map)(struct vm_area_struct *vma,
+ const unsigned long *src,
+ const unsigned long *dst,
+ unsigned long start,
+ unsigned long end,
+ void *private);
+};
+
+int migrate_vma(const struct migrate_vma_ops *ops,
+ struct vm_area_struct *vma,
+ unsigned long start,
+ unsigned long end,
+ unsigned long *src,
+ unsigned long *dst,
+ void *private);
+
+#endif /* CONFIG_MIGRATION */
+
#endif /* _LINUX_MIGRATE_H */
diff --git a/mm/migrate.c b/mm/migrate.c
index 66410fc..12063f3 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -397,6 +397,14 @@ int migrate_page_move_mapping(struct address_space *mapping,
int expected_count = 1 + extra_count;
void **pslot;
+ /*
+ * ZONE_DEVICE pages have 1 refcount always held by their device
+ *
+ * Note that DAX memory will never reach that point as it does not have
+ * the MEMORY_DEVICE_ALLOW_MIGRATE flag set (see memory_hotplug.h).
+ */
+ expected_count += is_zone_device_page(page);
+
if (!mapping) {
/* Anonymous page without mapping */
if (page_count(page) != expected_count)
@@ -2077,3 +2085,439 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
#endif /* CONFIG_NUMA_BALANCING */
#endif /* CONFIG_NUMA */
+
+
+struct migrate_vma {
+ struct vm_area_struct *vma;
+ unsigned long *dst;
+ unsigned long *src;
+ unsigned long cpages;
+ unsigned long npages;
+ unsigned long start;
+ unsigned long end;
+};
+
+static int migrate_vma_collect_hole(unsigned long start,
+ unsigned long end,
+ struct mm_walk *walk)
+{
+ struct migrate_vma *migrate = walk->private;
+ unsigned long addr, next;
+
+ for (addr = start & PAGE_MASK; addr < end; addr += PAGE_SIZE) {
+ migrate->dst[migrate->npages] = 0;
+ migrate->src[migrate->npages++] = 0;
+ }
+
+ return 0;
+}
+
+static int migrate_vma_collect_pmd(pmd_t *pmdp,
+ unsigned long start,
+ unsigned long end,
+ struct mm_walk *walk)
+{
+ struct migrate_vma *migrate = walk->private;
+ struct mm_struct *mm = walk->vma->vm_mm;
+ unsigned long addr = start;
+ spinlock_t *ptl;
+ pte_t *ptep;
+
+ if (pmd_none(*pmdp) || pmd_trans_unstable(pmdp)) {
+ /* FIXME support THP */
+ return migrate_vma_collect_hole(start, end, walk);
+ }
+
+ ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
+ for (; addr < end; addr += PAGE_SIZE, ptep++) {
+ unsigned long mpfn, pfn;
+ struct page *page;
+ pte_t pte;
+
+ pte = *ptep;
+ pfn = pte_pfn(pte);
+
+ if (!pte_present(pte)) {
+ mpfn = pfn = 0;
+ goto next;
+ }
+
+ /* FIXME support THP */
+ page = vm_normal_page(migrate->vma, addr, pte);
+ if (!page || !page->mapping || PageTransCompound(page)) {
+ mpfn = pfn = 0;
+ goto next;
+ }
+
+ /*
+ * By getting a reference on the page we pin it and that blocks
+ * any kind of migration. Side effect is that it "freezes" the
+ * pte.
+ *
+ * We drop this reference after isolating the page from the lru
+ * for non device page (device page are not on the lru and thus
+ * can't be dropped from it).
+ */
+ get_page(page);
+ migrate->cpages++;
+ mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
+ mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
+
+next:
+ migrate->src[migrate->npages++] = mpfn;
+ }
+ pte_unmap_unlock(ptep - 1, ptl);
+
+ return 0;
+}
+
+/*
+ * migrate_vma_collect() - collect pages over a range of virtual addresses
+ * @migrate: migrate struct containing all migration information
+ *
+ * This will walk the CPU page table. For each virtual address backed by a
+ * valid page, it updates the src array and takes a reference on the page, in
+ * order to pin the page until we lock it and unmap it.
+ */
+static void migrate_vma_collect(struct migrate_vma *migrate)
+{
+ struct mm_walk mm_walk;
+
+ mm_walk.pmd_entry = migrate_vma_collect_pmd;
+ mm_walk.pte_entry = NULL;
+ mm_walk.pte_hole = migrate_vma_collect_hole;
+ mm_walk.hugetlb_entry = NULL;
+ mm_walk.test_walk = NULL;
+ mm_walk.vma = migrate->vma;
+ mm_walk.mm = migrate->vma->vm_mm;
+ mm_walk.private = migrate;
+
+ walk_page_range(migrate->start, migrate->end, &mm_walk);
+
+ migrate->end = migrate->start + (migrate->npages << PAGE_SHIFT);
+}
+
+/*
+ * migrate_vma_check_page() - check if page is pinned or not
+ * @page: struct page to check
+ *
+ * Pinned pages cannot be migrated. This is the same test as in
+ * migrate_page_move_mapping(), except that here we allow migration of a
+ * ZONE_DEVICE page.
+ */
+static bool migrate_vma_check_page(struct page *page)
+{
+ /*
+ * One extra ref because caller holds an extra reference, either from
+ * isolate_lru_page() for a regular page, or migrate_vma_collect() for
+ * a device page.
+ */
+ int extra = 1;
+
+ /*
+ * FIXME support THP (transparent huge page), it is bit more complex to
+ * check them than regular pages, because they can be mapped with a pmd
+ * or with a pte (split pte mapping).
+ */
+ if (PageCompound(page))
+ return false;
+
+ if ((page_count(page) - extra) > page_mapcount(page))
+ return false;
+
+ return true;
+}
+
+/*
+ * migrate_vma_prepare() - lock pages and isolate them from the lru
+ * @migrate: migrate struct containing all migration information
+ *
+ * This locks pages that have been collected by migrate_vma_collect(). Once each
+ * page is locked it is isolated from the lru (for non-device pages). Finally,
+ * the ref taken by migrate_vma_collect() is dropped, as locked pages cannot be
+ * migrated by concurrent kernel threads.
+ */
+static void migrate_vma_prepare(struct migrate_vma *migrate)
+{
+ const unsigned long npages = migrate->npages;
+ const unsigned long start = migrate->start;
+ unsigned long addr, i, restore = 0;
+ bool allow_drain = true;
+
+ lru_add_drain();
+
+ for (i = 0; i < npages; i++) {
+ struct page *page = migrate_pfn_to_page(migrate->src[i]);
+
+ if (!page)
+ continue;
+
+ lock_page(page);
+ migrate->src[i] |= MIGRATE_PFN_LOCKED;
+
+ if (!PageLRU(page) && allow_drain) {
+ /* Drain CPU's pagevec */
+ lru_add_drain_all();
+ allow_drain = false;
+ }
+
+ if (isolate_lru_page(page)) {
+ migrate->src[i] = 0;
+ unlock_page(page);
+ migrate->cpages--;
+ put_page(page);
+ continue;
+ }
+
+ if (!migrate_vma_check_page(page)) {
+ migrate->src[i] = 0;
+ unlock_page(page);
+ migrate->cpages--;
+
+ putback_lru_page(page);
+ }
+ }
+}
+
+/*
+ * migrate_vma_unmap() - replace page mapping with special migration pte entry
+ * @migrate: migrate struct containing all migration information
+ *
+ * Replace page mapping (CPU page table pte) with a special migration pte entry
+ * and check again if it has been pinned. Pinned pages are restored because we
+ * cannot migrate them.
+ *
+ * This is the last step before we call the device driver callback to allocate
+ * destination memory and copy contents of original page over to new page.
+ */
+static void migrate_vma_unmap(struct migrate_vma *migrate)
+{
+ int flags = TTU_MIGRATION | TTU_IGNORE_MLOCK | TTU_IGNORE_ACCESS;
+ const unsigned long npages = migrate->npages;
+ const unsigned long start = migrate->start;
+ unsigned long addr, i, restore = 0;
+
+ for (i = 0; i < npages; i++) {
+ struct page *page = migrate_pfn_to_page(migrate->src[i]);
+
+ if (!page || !(migrate->src[i] & MIGRATE_PFN_MIGRATE))
+ continue;
+
+ try_to_unmap(page, flags);
+ if (page_mapped(page) || !migrate_vma_check_page(page)) {
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ migrate->cpages--;
+ restore++;
+ }
+ }
+
+ for (addr = start, i = 0; i < npages && restore; addr += PAGE_SIZE, i++) {
+ struct page *page = migrate_pfn_to_page(migrate->src[i]);
+
+ if (!page || (migrate->src[i] & MIGRATE_PFN_MIGRATE))
+ continue;
+
+ remove_migration_ptes(page, page, false);
+
+ migrate->src[i] = 0;
+ unlock_page(page);
+ restore--;
+
+ putback_lru_page(page);
+ }
+}
+
+/*
+ * migrate_vma_pages() - migrate meta-data from src page to dst page
+ * @migrate: migrate struct containing all migration information
+ *
+ * This migrates struct page meta-data from source struct page to destination
+ * struct page. This effectively finishes the migration from source page to the
+ * destination page.
+ */
+static void migrate_vma_pages(struct migrate_vma *migrate)
+{
+ const unsigned long npages = migrate->npages;
+ const unsigned long start = migrate->start;
+ unsigned long addr, i;
+
+ for (i = 0, addr = start; i < npages; addr += PAGE_SIZE, i++) {
+ struct page *newpage = migrate_pfn_to_page(migrate->dst[i]);
+ struct page *page = migrate_pfn_to_page(migrate->src[i]);
+ struct address_space *mapping;
+ int r;
+
+ if (!page || !newpage)
+ continue;
+ if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE))
+ continue;
+
+ mapping = page_mapping(page);
+
+ r = migrate_page(mapping, newpage, page, MIGRATE_SYNC_NO_COPY);
+ if (r != MIGRATEPAGE_SUCCESS)
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ }
+}
+
+/*
+ * migrate_vma_finalize() - restore CPU page table entry
+ * @migrate: migrate struct containing all migration information
+ *
+ * This replaces the special migration pte entry with either a mapping to the
+ * new page if migration was successful for that page, or to the original page
+ * otherwise.
+ *
+ * This also unlocks the pages and puts them back on the lru, or drops the extra
+ * refcount, for device pages.
+ */
+static void migrate_vma_finalize(struct migrate_vma *migrate)
+{
+ const unsigned long npages = migrate->npages;
+ unsigned long i;
+
+ for (i = 0; i < npages; i++) {
+ struct page *newpage = migrate_pfn_to_page(migrate->dst[i]);
+ struct page *page = migrate_pfn_to_page(migrate->src[i]);
+
+ if (!page)
+ continue;
+ if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE) || !newpage) {
+ if (newpage) {
+ unlock_page(newpage);
+ put_page(newpage);
+ }
+ newpage = page;
+ }
+
+ remove_migration_ptes(page, newpage, false);
+ unlock_page(page);
+ migrate->cpages--;
+
+ putback_lru_page(page);
+
+ if (newpage != page) {
+ unlock_page(newpage);
+ putback_lru_page(newpage);
+ }
+ }
+}
+
+/*
+ * migrate_vma() - migrate a range of memory inside vma
+ *
+ * @ops: migration callback for allocating destination memory and copying
+ * @vma: virtual memory area containing the range to be migrated
+ * @start: start address of the range to migrate (inclusive)
+ * @end: end address of the range to migrate (exclusive)
+ * @src: array of hmm_pfn_t containing source pfns
+ * @dst: array of hmm_pfn_t containing destination pfns
+ * @private: pointer passed back to each of the callback
+ * Returns: 0 on success, error code otherwise
+ *
+ * This function tries to migrate a range of memory virtual address range, using
+ * callbacks to allocate and copy memory from source to destination. First it
+ * collects all the pages backing each virtual address in the range, saving this
+ * inside the src array. Then it locks those pages and unmaps them. Once the pages
+ * are locked and unmapped, it checks whether each page is pinned or not. Pages
+ * that aren't pinned have the MIGRATE_PFN_MIGRATE flag set (by this function)
+ * in the corresponding src array entry. It then restores any pages that are
+ * pinned, by remapping and unlocking those pages.
+ *
+ * At this point it calls the alloc_and_copy() callback. For documentation on
+ * what is expected from that callback, see struct migrate_vma_ops comments in
+ * include/linux/migrate.h
+ *
+ * After the alloc_and_copy() callback, this function goes over each entry in
+ * the src array that has the MIGRATE_PFN_VALID and MIGRATE_PFN_MIGRATE flag
+ * set. If the corresponding entry in dst array has MIGRATE_PFN_VALID flag set,
+ * then the function tries to migrate struct page information from the source
+ * struct page to the destination struct page. If it fails to migrate the struct
+ * page information, then it clears the MIGRATE_PFN_MIGRATE flag in the src
+ * array.
+ *
+ * At this point all successfully migrated pages have an entry in the src
+ * array with MIGRATE_PFN_VALID and MIGRATE_PFN_MIGRATE flag set and the dst
+ * array entry with MIGRATE_PFN_VALID flag set.
+ *
+ * It then calls the finalize_and_map() callback. See comments for "struct
+ * migrate_vma_ops", in include/linux/migrate.h for details about
+ * finalize_and_map() behavior.
+ *
+ * After the finalize_and_map() callback, for successfully migrated pages, this
+ * function updates the CPU page table to point to new pages, otherwise it
+ * restores the CPU page table to point to the original source pages.
+ *
+ * Function returns 0 after the above steps, even if no pages were migrated
+ * (The function only returns an error if any of the arguments are invalid.)
+ *
+ * Both src and dst array must be big enough for (end - start) >> PAGE_SHIFT
+ * unsigned long entries.
+ */
+int migrate_vma(const struct migrate_vma_ops *ops,
+ struct vm_area_struct *vma,
+ unsigned long start,
+ unsigned long end,
+ unsigned long *src,
+ unsigned long *dst,
+ void *private)
+{
+ struct migrate_vma migrate;
+
+ /* Sanity check the arguments */
+ start &= PAGE_MASK;
+ end &= PAGE_MASK;
+ if (!vma || is_vm_hugetlb_page(vma) || (vma->vm_flags & VM_SPECIAL))
+ return -EINVAL;
+ if (start < vma->vm_start || start >= vma->vm_end)
+ return -EINVAL;
+ if (end <= vma->vm_start || end > vma->vm_end)
+ return -EINVAL;
+ if (!ops || !src || !dst || start >= end)
+ return -EINVAL;
+
+ memset(src, 0, sizeof(*src) * ((end - start) >> PAGE_SHIFT));
+ migrate.src = src;
+ migrate.dst = dst;
+ migrate.start = start;
+ migrate.npages = 0;
+ migrate.cpages = 0;
+ migrate.end = end;
+ migrate.vma = vma;
+
+ /* Collect, and try to unmap source pages */
+ migrate_vma_collect(&migrate);
+ if (!migrate.cpages)
+ return 0;
+
+ /* Lock and isolate page */
+ migrate_vma_prepare(&migrate);
+ if (!migrate.cpages)
+ return 0;
+
+ /* Unmap pages */
+ migrate_vma_unmap(&migrate);
+ if (!migrate.cpages)
+ return 0;
+
+ /*
+ * At this point pages are locked and unmapped, and thus they have
+ * stable content and can safely be copied to destination memory that
+ * is allocated by the callback.
+ *
+ * Note that migration can fail in migrate_vma_struct_page() for each
+ * individual page.
+ */
+ ops->alloc_and_copy(vma, src, dst, start, end, private);
+
+ /* This does the real migration of struct page */
+ migrate_vma_pages(&migrate);
+
+ ops->finalize_and_map(vma, src, dst, start, end, private);
+
+ /* Unlock and remap pages */
+ migrate_vma_finalize(&migrate);
+
+ return 0;
+}
+EXPORT_SYMBOL(migrate_vma);
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [HMM 13/15] mm/migrate: migrate_vma() unmap page from vma while collecting pages
From: Jérôme Glisse @ 2017-05-22 16:52 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse,
Evgeny Baskakov, Mark Hairgrove, Sherry Cheung, Subhash Gutti
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
Common case for migration of virtual address range is page are map
only once inside the vma in which migration is taking place. Because
we already walk the CPU page table for that range we can directly do
the unmap there and setup special migration swap entry.
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Signed-off-by: Evgeny Baskakov <ebaskakov@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
Signed-off-by: Sherry Cheung <SCheung@nvidia.com>
Signed-off-by: Subhash Gutti <sgutti@nvidia.com>
---
mm/migrate.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 98 insertions(+), 16 deletions(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index 12063f3..1f2bc61 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2119,7 +2119,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
{
struct migrate_vma *migrate = walk->private;
struct mm_struct *mm = walk->vma->vm_mm;
- unsigned long addr = start;
+ unsigned long addr = start, unmapped = 0;
spinlock_t *ptl;
pte_t *ptep;
@@ -2129,9 +2129,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
}
ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
+ arch_enter_lazy_mmu_mode();
+
for (; addr < end; addr += PAGE_SIZE, ptep++) {
unsigned long mpfn, pfn;
struct page *page;
+ swp_entry_t entry;
pte_t pte;
pte = *ptep;
@@ -2163,11 +2166,44 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
+ /*
+ * Optimize for the common case where page is only mapped once
+ * in one process. If we can lock the page, then we can safely
+ * set up a special migration page table entry now.
+ */
+ if (trylock_page(page)) {
+ pte_t swp_pte;
+
+ mpfn |= MIGRATE_PFN_LOCKED;
+ ptep_get_and_clear(mm, addr, ptep);
+
+ /* Setup special migration page table entry */
+ entry = make_migration_entry(page, pte_write(pte));
+ swp_pte = swp_entry_to_pte(entry);
+ if (pte_soft_dirty(pte))
+ swp_pte = pte_swp_mksoft_dirty(swp_pte);
+ set_pte_at(mm, addr, ptep, swp_pte);
+
+ /*
+ * This is like regular unmap: we remove the rmap and
+ * drop page refcount. Page won't be freed, as we took
+ * a reference just above.
+ */
+ page_remove_rmap(page, false);
+ put_page(page);
+ unmapped++;
+ }
+
next:
migrate->src[migrate->npages++] = mpfn;
}
+ arch_leave_lazy_mmu_mode();
pte_unmap_unlock(ptep - 1, ptl);
+ /* Only flush the TLB if we actually modified any entries */
+ if (unmapped)
+ flush_tlb_range(walk->vma, start, end);
+
return 0;
}
@@ -2192,7 +2228,13 @@ static void migrate_vma_collect(struct migrate_vma *migrate)
mm_walk.mm = migrate->vma->vm_mm;
mm_walk.private = migrate;
+ mmu_notifier_invalidate_range_start(mm_walk.mm,
+ migrate->start,
+ migrate->end);
walk_page_range(migrate->start, migrate->end, &mm_walk);
+ mmu_notifier_invalidate_range_end(mm_walk.mm,
+ migrate->start,
+ migrate->end);
migrate->end = migrate->start + (migrate->npages << PAGE_SHIFT);
}
@@ -2248,12 +2290,16 @@ static void migrate_vma_prepare(struct migrate_vma *migrate)
for (i = 0; i < npages; i++) {
struct page *page = migrate_pfn_to_page(migrate->src[i]);
+ bool remap = true;
if (!page)
continue;
- lock_page(page);
- migrate->src[i] |= MIGRATE_PFN_LOCKED;
+ if (!(migrate->src[i] & MIGRATE_PFN_LOCKED)) {
+ remap = false;
+ lock_page(page);
+ migrate->src[i] |= MIGRATE_PFN_LOCKED;
+ }
if (!PageLRU(page) && allow_drain) {
/* Drain CPU's pagevec */
@@ -2262,21 +2308,50 @@ static void migrate_vma_prepare(struct migrate_vma *migrate)
}
if (isolate_lru_page(page)) {
- migrate->src[i] = 0;
- unlock_page(page);
- migrate->cpages--;
- put_page(page);
+ if (remap) {
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ migrate->cpages--;
+ restore++;
+ } else {
+ migrate->src[i] = 0;
+ unlock_page(page);
+ migrate->cpages--;
+ put_page(page);
+ }
continue;
}
if (!migrate_vma_check_page(page)) {
- migrate->src[i] = 0;
- unlock_page(page);
- migrate->cpages--;
+ if (remap) {
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ migrate->cpages--;
+ restore++;
- putback_lru_page(page);
+ get_page(page);
+ putback_lru_page(page);
+ } else {
+ migrate->src[i] = 0;
+ unlock_page(page);
+ migrate->cpages--;
+
+ putback_lru_page(page);
+ }
}
}
+
+ for (i = 0, addr = start; i < npages && restore; i++, addr += PAGE_SIZE) {
+ struct page *page = migrate_pfn_to_page(migrate->src[i]);
+
+ if (!page || (migrate->src[i] & MIGRATE_PFN_MIGRATE))
+ continue;
+
+ remove_migration_pte(page, migrate->vma, addr, page);
+
+ migrate->src[i] = 0;
+ unlock_page(page);
+ put_page(page);
+ restore--;
+ }
}
/*
@@ -2303,12 +2378,19 @@ static void migrate_vma_unmap(struct migrate_vma *migrate)
if (!page || !(migrate->src[i] & MIGRATE_PFN_MIGRATE))
continue;
- try_to_unmap(page, flags);
- if (page_mapped(page) || !migrate_vma_check_page(page)) {
- migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
- migrate->cpages--;
- restore++;
+ if (page_mapped(page)) {
+ try_to_unmap(page, flags);
+ if (page_mapped(page))
+ goto restore;
}
+
+ if (migrate_vma_check_page(page))
+ continue;
+
+restore:
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ migrate->cpages--;
+ restore++;
}
for (addr = start, i = 0; i < npages && restore; addr += PAGE_SIZE, i++) {
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [HMM 14/15] mm/migrate: support un-addressable ZONE_DEVICE page in migration v2
From: Jérôme Glisse @ 2017-05-22 16:52 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse,
Kirill A . Shutemov
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
Allow to unmap and restore special swap entry of un-addressable
ZONE_DEVICE memory.
Changed since v1:
- s/device unaddressable/device private/
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
include/linux/migrate.h | 10 +++-
mm/migrate.c | 134 ++++++++++++++++++++++++++++++++++++++----------
mm/page_vma_mapped.c | 10 ++++
mm/rmap.c | 25 +++++++++
4 files changed, 150 insertions(+), 29 deletions(-)
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 576b3f5..7dd875a 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -130,12 +130,18 @@ static inline int migrate_misplaced_transhuge_page(struct mm_struct *mm,
#ifdef CONFIG_MIGRATION
+/*
+ * Watch out for PAE architecture, which has an unsigned long, and might not
+ * have enough bits to store all physical address and flags. So far we have
+ * enough room for all our flags.
+ */
#define MIGRATE_PFN_VALID (1UL << 0)
#define MIGRATE_PFN_MIGRATE (1UL << 1)
#define MIGRATE_PFN_LOCKED (1UL << 2)
#define MIGRATE_PFN_WRITE (1UL << 3)
-#define MIGRATE_PFN_ERROR (1UL << 4)
-#define MIGRATE_PFN_SHIFT 5
+#define MIGRATE_PFN_DEVICE (1UL << 4)
+#define MIGRATE_PFN_ERROR (1UL << 5)
+#define MIGRATE_PFN_SHIFT 6
static inline struct page *migrate_pfn_to_page(unsigned long mpfn)
{
diff --git a/mm/migrate.c b/mm/migrate.c
index 1f2bc61..9e68399 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -36,6 +36,7 @@
#include <linux/hugetlb.h>
#include <linux/hugetlb_cgroup.h>
#include <linux/gfp.h>
+#include <linux/memremap.h>
#include <linux/balloon_compaction.h>
#include <linux/mmu_notifier.h>
#include <linux/page_idle.h>
@@ -227,7 +228,15 @@ static bool remove_migration_pte(struct page *page, struct vm_area_struct *vma,
if (is_write_migration_entry(entry))
pte = maybe_mkwrite(pte, vma);
- flush_dcache_page(new);
+ if (unlikely(is_zone_device_page(new)) &&
+ is_device_private_page(new)) {
+ entry = make_device_private_entry(new, pte_write(pte));
+ pte = swp_entry_to_pte(entry);
+ if (pte_swp_soft_dirty(*pvmw.pte))
+ pte = pte_mksoft_dirty(pte);
+ } else
+ flush_dcache_page(new);
+
#ifdef CONFIG_HUGETLB_PAGE
if (PageHuge(new)) {
pte = pte_mkhuge(pte);
@@ -2140,17 +2149,40 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
pte = *ptep;
pfn = pte_pfn(pte);
- if (!pte_present(pte)) {
+ if (pte_none(pte)) {
mpfn = pfn = 0;
goto next;
}
+ if (!pte_present(pte)) {
+ mpfn = pfn = 0;
+
+ /*
+ * Only care about unaddressable device page special
+ * page table entry. Other special swap entries are not
+ * migratable, and we ignore regular swapped page.
+ */
+ entry = pte_to_swp_entry(pte);
+ if (!is_device_private_entry(entry))
+ goto next;
+
+ page = device_private_entry_to_page(entry);
+ mpfn = migrate_pfn(page_to_pfn(page))|
+ MIGRATE_PFN_DEVICE | MIGRATE_PFN_MIGRATE;
+ if (is_write_device_private_entry(entry))
+ mpfn |= MIGRATE_PFN_WRITE;
+ } else {
+ page = vm_normal_page(migrate->vma, addr, pte);
+ mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
+ mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
+ }
+
/* FIXME support THP */
- page = vm_normal_page(migrate->vma, addr, pte);
if (!page || !page->mapping || PageTransCompound(page)) {
mpfn = pfn = 0;
goto next;
}
+ pfn = page_to_pfn(page);
/*
* By getting a reference on the page we pin it and that blocks
@@ -2163,8 +2195,6 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
*/
get_page(page);
migrate->cpages++;
- mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
- mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
/*
* Optimize for the common case where page is only mapped once
@@ -2195,6 +2225,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
}
next:
+ migrate->dst[migrate->npages] = 0;
migrate->src[migrate->npages++] = mpfn;
}
arch_leave_lazy_mmu_mode();
@@ -2264,6 +2295,15 @@ static bool migrate_vma_check_page(struct page *page)
if (PageCompound(page))
return false;
+ /* Page from ZONE_DEVICE have one extra reference */
+ if (is_zone_device_page(page)) {
+ if (is_device_private_page(page)) {
+ extra++;
+ } else
+ /* Other ZONE_DEVICE memory type are not supported */
+ return false;
+ }
+
if ((page_count(page) - extra) > page_mapcount(page))
return false;
@@ -2301,24 +2341,30 @@ static void migrate_vma_prepare(struct migrate_vma *migrate)
migrate->src[i] |= MIGRATE_PFN_LOCKED;
}
- if (!PageLRU(page) && allow_drain) {
- /* Drain CPU's pagevec */
- lru_add_drain_all();
- allow_drain = false;
- }
+ /* ZONE_DEVICE pages are not on LRU */
+ if (!is_zone_device_page(page)) {
+ if (!PageLRU(page) && allow_drain) {
+ /* Drain CPU's pagevec */
+ lru_add_drain_all();
+ allow_drain = false;
+ }
- if (isolate_lru_page(page)) {
- if (remap) {
- migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
- migrate->cpages--;
- restore++;
- } else {
- migrate->src[i] = 0;
- unlock_page(page);
- migrate->cpages--;
- put_page(page);
+ if (isolate_lru_page(page)) {
+ if (remap) {
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ migrate->cpages--;
+ restore++;
+ } else {
+ migrate->src[i] = 0;
+ unlock_page(page);
+ migrate->cpages--;
+ put_page(page);
+ }
+ continue;
}
- continue;
+
+ /* Drop the reference we took in collect */
+ put_page(page);
}
if (!migrate_vma_check_page(page)) {
@@ -2327,14 +2373,19 @@ static void migrate_vma_prepare(struct migrate_vma *migrate)
migrate->cpages--;
restore++;
- get_page(page);
- putback_lru_page(page);
+ if (!is_zone_device_page(page)) {
+ get_page(page);
+ putback_lru_page(page);
+ }
} else {
migrate->src[i] = 0;
unlock_page(page);
migrate->cpages--;
- putback_lru_page(page);
+ if (!is_zone_device_page(page))
+ putback_lru_page(page);
+ else
+ put_page(page);
}
}
}
@@ -2405,7 +2456,10 @@ static void migrate_vma_unmap(struct migrate_vma *migrate)
unlock_page(page);
restore--;
- putback_lru_page(page);
+ if (is_zone_device_page(page))
+ put_page(page);
+ else
+ putback_lru_page(page);
}
}
@@ -2436,6 +2490,26 @@ static void migrate_vma_pages(struct migrate_vma *migrate)
mapping = page_mapping(page);
+ if (is_zone_device_page(newpage)) {
+ if (is_device_private_page(newpage)) {
+ /*
+ * For now only support private anonymous when
+ * migrating to un-addressable device memory.
+ */
+ if (mapping) {
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ continue;
+ }
+ } else {
+ /*
+ * Other types of ZONE_DEVICE page are not
+ * supported.
+ */
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
+ continue;
+ }
+ }
+
r = migrate_page(mapping, newpage, page, MIGRATE_SYNC_NO_COPY);
if (r != MIGRATEPAGE_SUCCESS)
migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
@@ -2476,11 +2550,17 @@ static void migrate_vma_finalize(struct migrate_vma *migrate)
unlock_page(page);
migrate->cpages--;
- putback_lru_page(page);
+ if (is_zone_device_page(page))
+ put_page(page);
+ else
+ putback_lru_page(page);
if (newpage != page) {
unlock_page(newpage);
- putback_lru_page(newpage);
+ if (is_zone_device_page(newpage))
+ put_page(newpage);
+ else
+ putback_lru_page(newpage);
}
}
}
diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index de9c40d..f95765c 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -48,6 +48,7 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
if (!is_swap_pte(*pvmw->pte))
return false;
entry = pte_to_swp_entry(*pvmw->pte);
+
if (!is_migration_entry(entry))
return false;
if (migration_entry_to_page(entry) - pvmw->page >=
@@ -60,6 +61,15 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
WARN_ON_ONCE(1);
#endif
} else {
+ if (is_swap_pte(*pvmw->pte)) {
+ swp_entry_t entry;
+
+ entry = pte_to_swp_entry(*pvmw->pte);
+ if (is_device_private_entry(entry) &&
+ device_private_entry_to_page(entry) == pvmw->page)
+ return true;
+ }
+
if (!pte_present(*pvmw->pte))
return false;
diff --git a/mm/rmap.c b/mm/rmap.c
index 3ff241f..e442a80 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -63,6 +63,7 @@
#include <linux/hugetlb.h>
#include <linux/backing-dev.h>
#include <linux/page_idle.h>
+#include <linux/memremap.h>
#include <asm/tlbflush.h>
@@ -1308,6 +1309,10 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
if ((flags & TTU_MUNLOCK) && !(vma->vm_flags & VM_LOCKED))
return true;
+ if (IS_ENABLED(CONFIG_MIGRATION) && (flags & TTU_MIGRATION) &&
+ is_zone_device_page(page) && !is_device_private_page(page))
+ return true;
+
if (flags & TTU_SPLIT_HUGE_PMD) {
split_huge_pmd_address(vma, address,
flags & TTU_MIGRATION, page);
@@ -1343,6 +1348,26 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
subpage = page - page_to_pfn(page) + pte_pfn(*pvmw.pte);
address = pvmw.address;
+ if (IS_ENABLED(CONFIG_MIGRATION) &&
+ (flags & TTU_MIGRATION) &&
+ is_zone_device_page(page)) {
+ swp_entry_t entry;
+ pte_t swp_pte;
+
+ pteval = ptep_get_and_clear(mm, address, pvmw.pte);
+
+ /*
+ * Store the pfn of the page in a special migration
+ * pte. do_swap_page() will wait until the migration
+ * pte is removed and then restart fault handling.
+ */
+ entry = make_migration_entry(page, 0);
+ swp_pte = swp_entry_to_pte(entry);
+ if (pte_soft_dirty(pteval))
+ swp_pte = pte_swp_mksoft_dirty(swp_pte);
+ set_pte_at(mm, address, pvmw.pte, swp_pte);
+ goto discard;
+ }
if (!(flags & TTU_IGNORE_ACCESS)) {
if (ptep_clear_flush_young_notify(vma, address,
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [HMM 15/15] mm/migrate: allow migrate_vma() to alloc new page on empty entry v2
From: Jérôme Glisse @ 2017-05-22 16:52 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm
Cc: John Hubbard, David Nellans, Jérôme Glisse
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>
This allow caller of migrate_vma() to allocate new page for empty CPU
page table entry. It only support anoymous memory and it won't allow
new page to be instance if userfaultfd is armed.
This is useful to device driver that want to migrate a range of virtual
address and would rather allocate new memory than having to fault later
on.
Changed since v1:
- 5 level page table fix
Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
---
mm/migrate.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 131 insertions(+), 4 deletions(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index 9e68399..d7c4db6 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -37,6 +37,7 @@
#include <linux/hugetlb_cgroup.h>
#include <linux/gfp.h>
#include <linux/memremap.h>
+#include <linux/userfaultfd_k.h>
#include <linux/balloon_compaction.h>
#include <linux/mmu_notifier.h>
#include <linux/page_idle.h>
@@ -2111,9 +2112,10 @@ static int migrate_vma_collect_hole(unsigned long start,
struct mm_walk *walk)
{
struct migrate_vma *migrate = walk->private;
- unsigned long addr, next;
+ unsigned long addr;
for (addr = start & PAGE_MASK; addr < end; addr += PAGE_SIZE) {
+ migrate->cpages++;
migrate->dst[migrate->npages] = 0;
migrate->src[migrate->npages++] = 0;
}
@@ -2150,6 +2152,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
pfn = pte_pfn(pte);
if (pte_none(pte)) {
+ migrate->cpages++;
mpfn = pfn = 0;
goto next;
}
@@ -2463,6 +2466,118 @@ static void migrate_vma_unmap(struct migrate_vma *migrate)
}
}
+static void migrate_vma_insert_page(struct migrate_vma *migrate,
+ unsigned long addr,
+ struct page *page,
+ unsigned long *src,
+ unsigned long *dst)
+{
+ struct vm_area_struct *vma = migrate->vma;
+ struct mm_struct *mm = vma->vm_mm;
+ struct mem_cgroup *memcg;
+ spinlock_t *ptl;
+ pgd_t *pgdp;
+ p4d_t *p4dp;
+ pud_t *pudp;
+ pmd_t *pmdp;
+ pte_t *ptep;
+ pte_t entry;
+
+ /* Only allow populating anonymous memory */
+ if (!vma_is_anonymous(vma))
+ goto abort;
+
+ pgdp = pgd_offset(mm, addr);
+ p4dp = p4d_alloc(mm, pgdp, addr);
+ if (!p4dp)
+ goto abort;
+ pudp = pud_alloc(mm, p4dp, addr);
+ if (!pudp)
+ goto abort;
+ pmdp = pmd_alloc(mm, pudp, addr);
+ if (!pmdp)
+ goto abort;
+
+ if (pmd_trans_unstable(pmdp) || pmd_devmap(*pmdp))
+ goto abort;
+
+ /*
+ * Use pte_alloc() instead of pte_alloc_map(). We can't run
+ * pte_offset_map() on pmds where a huge pmd might be created
+ * from a different thread.
+ *
+ * pte_alloc_map() is safe to use under down_write(mmap_sem) or when
+ * parallel threads are excluded by other means.
+ *
+ * Here we only have down_read(mmap_sem).
+ */
+ if (pte_alloc(mm, pmdp, addr))
+ goto abort;
+
+ /* See the comment in pte_alloc_one_map() */
+ if (unlikely(pmd_trans_unstable(pmdp)))
+ goto abort;
+
+ if (unlikely(anon_vma_prepare(vma)))
+ goto abort;
+ if (mem_cgroup_try_charge(page, vma->vm_mm, GFP_KERNEL, &memcg, false))
+ goto abort;
+
+ /*
+ * The memory barrier inside __SetPageUptodate makes sure that
+ * preceding stores to the page contents become visible before
+ * the set_pte_at() write.
+ */
+ __SetPageUptodate(page);
+
+ if (is_zone_device_page(page) && is_device_private_page(page)) {
+ swp_entry_t swp_entry;
+
+ swp_entry = make_device_private_entry(page, vma->vm_flags & VM_WRITE);
+ entry = swp_entry_to_pte(swp_entry);
+ } else {
+ entry = mk_pte(page, vma->vm_page_prot);
+ if (vma->vm_flags & VM_WRITE)
+ entry = pte_mkwrite(pte_mkdirty(entry));
+ }
+
+ ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
+ if (!pte_none(*ptep)) {
+ pte_unmap_unlock(ptep, ptl);
+ mem_cgroup_cancel_charge(page, memcg, false);
+ goto abort;
+ }
+
+ /*
+ * Check for usefaultfd but do not deliver the fault. Instead,
+ * just back off.
+ */
+ if (userfaultfd_missing(vma)) {
+ pte_unmap_unlock(ptep, ptl);
+ mem_cgroup_cancel_charge(page, memcg, false);
+ goto abort;
+ }
+
+ inc_mm_counter(mm, MM_ANONPAGES);
+ page_add_new_anon_rmap(page, vma, addr, false);
+ mem_cgroup_commit_charge(page, memcg, false, false);
+ if (!is_zone_device_page(page))
+ lru_cache_add_active_or_unevictable(page, vma);
+ set_pte_at(mm, addr, ptep, entry);
+
+ /* Take a reference on the page */
+ get_page(page);
+
+ /* No need to invalidate - it was non-present before */
+ update_mmu_cache(vma, addr, ptep);
+ pte_unmap_unlock(ptep, ptl);
+ *src = MIGRATE_PFN_MIGRATE;
+ return;
+
+abort:
+ *src &= ~MIGRATE_PFN_MIGRATE;
+}
+
/*
* migrate_vma_pages() - migrate meta-data from src page to dst page
* @migrate: migrate struct containing all migration information
@@ -2483,10 +2598,16 @@ static void migrate_vma_pages(struct migrate_vma *migrate)
struct address_space *mapping;
int r;
- if (!page || !newpage)
+ if (!newpage) {
+ migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
continue;
- if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE))
+ } else if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE)) {
+ if (!page)
+ migrate_vma_insert_page(migrate, addr, newpage,
+ &migrate->src[i],
+ &migrate->dst[i]);
continue;
+ }
mapping = page_mapping(page);
@@ -2536,8 +2657,14 @@ static void migrate_vma_finalize(struct migrate_vma *migrate)
struct page *newpage = migrate_pfn_to_page(migrate->dst[i]);
struct page *page = migrate_pfn_to_page(migrate->src[i]);
- if (!page)
+ if (!page) {
+ if (newpage) {
+ unlock_page(newpage);
+ put_page(newpage);
+ }
continue;
+ }
+
if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE) || !newpage) {
if (newpage) {
unlock_page(newpage);
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: mm, something wring in page_lock_anon_vma_read()?
From: Vlastimil Babka @ 2017-05-22 16:51 UTC (permalink / raw)
To: zhong jiang, Hugh Dickins
Cc: Xishi Qiu, Andrew Morton, Tejun Heo, Michal Hocko,
Johannes Weiner, Mel Gorman, Michal Hocko, Minchan Kim,
David Rientjes, Joonsoo Kim, aarcange, sumeet.keswani,
Rik van Riel, Linux MM, LKML
In-Reply-To: <591FB173.4020409@huawei.com>
On 05/20/2017 05:01 AM, zhong jiang wrote:
> On 2017/5/20 10:40, Hugh Dickins wrote:
>> On Sat, 20 May 2017, Xishi Qiu wrote:
>>> Here is a bug report form redhat: https://bugzilla.redhat.com/show_bug.cgi?id=1305620
>>> And I meet the bug too. However it is hard to reproduce, and
>>> 624483f3ea82598("mm: rmap: fix use-after-free in __put_anon_vma") is not help.
>>>
>>> From the vmcore, it seems that the page is still mapped(_mapcount=0 and _count=2),
>>> and the value of mapping is a valid address(mapping = 0xffff8801b3e2a101),
>>> but anon_vma has been corrupted.
>>>
>>> Any ideas?
>> Sorry, no. I assume that _mapcount has been misaccounted, for example
>> a pte mapped in on top of another pte; but cannot begin tell you where
>> in Red Hat's kernel-3.10.0-229.4.2.el7 that might happen.
>>
>> Hugh
>>
>> .
>>
> Hi, Hugh
>
> I find the following message from the dmesg.
>
> [26068.316592] BUG: Bad rss-counter state mm:ffff8800a7de2d80 idx:1 val:1
>
> I can prove that the __mapcount is misaccount. when task is exited. the rmap
> still exist.
Check if the kernel in question contains this commit: ad33bb04b2a6 ("mm:
thp: fix SMP race condition between THP page fault and MADV_DONTNEED")
> Thanks
> zhongjiang
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v2 12/17] cgroup: Remove cgroup v2 no internal process constraint
From: Waiman Long @ 2017-05-22 16:56 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170519203824.GC15279@wtj.duckdns.org>
On 05/19/2017 04:38 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:11AM -0400, Waiman Long wrote:
>> The rationale behind the cgroup v2 no internal process constraint is
>> to avoid resouorce competition between internal processes and child
>> cgroups. However, not all controllers have problem with internal
>> process competiton. Enforcing this rule may lead to unnatural process
>> hierarchy and unneeded levels for those controllers.
> This isn't necessarily something we can determine by looking at the
> current state of controllers. It's true that some controllers - pid
> and perf - inherently only care about membership of each task but at
> the same time neither really suffers from the constraint either. CPU
> which is the problematic one here and currently only cares about tasks
> actually distributes resources which have parts which are specific to
> domain rather than threads and we don't want to declare that CPU isn't
> domain aware resource because it inherently is.
I agree that it is hard to decide which controller should be regarded as
domain aware and which should not be. That is why I don't attempt to do
that in the v2 patchset.
Unlike my v1 patch where each controller has to be specifically marked
as being a resource domain and hence has special directory for internal
process resource control knobs, the v2 patch leaves the decision up to
the userland. Depending on the context, any controllers can now have
special resource control knobs for internal processes in the
cgroup.resource_domain directory by writing the controller name to the
cgroup.resource_control file. So even the CPU controller can be regarded
as domain aware, if necessary. This is all part of my move to give as
much freedom and flexibility to the userland.
>> This patch removes the no internal process contraint by enabling those
>> controllers that don't like internal process competition to have a
>> separate set of control knobs just for internal processes in a cgroup.
>>
>> A new control file "cgroup.resource_control" is added. Enabling a
>> controller with a "+" prefix will create a separate set of control
>> knobs for that controller in the special "cgroup.resource_domain"
>> sub-directory for all the internal processes. The existing control
>> knobs in the cgroup will then be used to manage resource distribution
>> between internal processes as a group and other child cgroups.
> We would need to declare all major resource controllers to be needing
> that special sub-directory. That'd work around the
> no-internal-process constraint but I don't think it is solving any
> real problems. It's just the kernel doing something that userland can
> do with ease and more context.
All controllers can use the special sub-directory if userland chooses to
do so. The problem that I am trying to address in this patch is to allow
more natural hierarchy that reflect a certain purpose, like the task
classification done by systemd. Restricting tasks only to leaf nodes
makes the hierarchy unnatural and probably difficult to manage.
Regards,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH] mm, oom: cgroup-aware OOM-killer
From: Roman Gushchin @ 2017-05-22 17:01 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Johannes Weiner, Tejun Heo, Li Zefan, Michal Hocko, Tetsuo Handa,
kernel-team, cgroups, linux-doc, linux-kernel, linux-mm
In-Reply-To: <20170520183729.GA3195@esperanza>
On Sat, May 20, 2017 at 09:37:29PM +0300, Vladimir Davydov wrote:
> Hello Roman,
Hi Vladimir!
>
> On Thu, May 18, 2017 at 05:28:04PM +0100, Roman Gushchin wrote:
> ...
> > +5-2-4. Cgroup-aware OOM Killer
> > +
> > +Cgroup v2 memory controller implements a cgroup-aware OOM killer.
> > +It means that it treats memory cgroups as memory consumers
> > +rather then individual processes. Under the OOM conditions it tries
> > +to find an elegible leaf memory cgroup, and kill all processes
> > +in this cgroup. If it's not possible (e.g. all processes belong
> > +to the root cgroup), it falls back to the traditional per-process
> > +behaviour.
>
> I agree that the current OOM victim selection algorithm is totally
> unfair in a system using containers and it has been crying for rework
> for the last few years now, so it's great to see this finally coming.
>
> However, I don't reckon that killing a whole leaf cgroup is always the
> best practice. It does make sense when cgroups are used for
> containerizing services or applications, because a service is unlikely
> to remain operational after one of its processes is gone, but one can
> also use cgroups to containerize processes started by a user. Kicking a
> user out for one of her process has gone mad doesn't sound right to me.
I agree, that it's not always a best practise, if you're not allowed
to change the cgroup configuration (e.g. create new cgroups).
IMHO, this case is mostly covered by using the v1 cgroup interface,
which remains unchanged.
If you do have control over cgroups, you can put processes into
separate cgroups, and obtain control over OOM victim selection and killing.
> Another example when the policy you're suggesting fails in my opinion is
> in case a service (cgroup) consists of sub-services (sub-cgroups) that
> run processes. The main service may stop working normally if one of its
> sub-services is killed. So it might make sense to kill not just an
> individual process or a leaf cgroup, but the whole main service with all
> its sub-services.
I agree, although I do not pretend for solving all possible
userspace problems caused by an OOM.
How to react on an OOM - is definitely a policy, which depends
on the workload. Nothing is changing here from how it's working now,
except now kernel will choose a victim cgroup, and kill the victim cgroup
rather than a process.
> And both kinds of workloads (services/applications and individual
> processes run by users) can co-exist on the same host - consider the
> default systemd setup, for instance.
>
> IMHO it would be better to give users a choice regarding what they
> really want for a particular cgroup in case of OOM - killing the whole
> cgroup or one of its descendants. For example, we could introduce a
> per-cgroup flag that would tell the kernel whether the cgroup can
> tolerate killing a descendant or not. If it can, the kernel will pick
> the fattest sub-cgroup or process and check it. If it cannot, it will
> kill the whole cgroup and all its processes and sub-cgroups.
The last thing we want to do, is to compare processes with cgroups.
I agree, that we can have some option to disable the cgroup-aware OOM at all,
mostly for backward-compatibility. But I don't think it should be a
per-cgroup configuration option, which we will support forever.
>
> > +
> > +The memory controller tries to make the best choise of a victim cgroup.
> > +In general, it tries to select the largest cgroup, matching given
> > +node/zone requirements, but the concrete algorithm is not defined,
> > +and may be changed later.
> > +
> > +This affects both system- and cgroup-wide OOMs. For a cgroup-wide OOM
> > +the memory controller considers only cgroups belonging to a sub-tree
> > +of the OOM-ing cgroup, including itself.
> ...
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index c131f7e..8d07481 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2625,6 +2625,75 @@ static inline bool memcg_has_children(struct mem_cgroup *memcg)
> > return ret;
> > }
> >
> > +bool mem_cgroup_select_oom_victim(struct oom_control *oc)
> > +{
> > + struct mem_cgroup *iter;
> > + unsigned long chosen_memcg_points;
> > +
> > + oc->chosen_memcg = NULL;
> > +
> > + if (mem_cgroup_disabled())
> > + return false;
> > +
> > + if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> > + return false;
> > +
> > + pr_info("Choosing a victim memcg because of %s",
> > + oc->memcg ?
> > + "memory limit reached of cgroup " :
> > + "out of memory\n");
> > + if (oc->memcg) {
> > + pr_cont_cgroup_path(oc->memcg->css.cgroup);
> > + pr_cont("\n");
> > + }
> > +
> > + chosen_memcg_points = 0;
> > +
> > + for_each_mem_cgroup_tree(iter, oc->memcg) {
> > + unsigned long points;
> > + int nid;
> > +
> > + if (mem_cgroup_is_root(iter))
> > + continue;
> > +
> > + if (memcg_has_children(iter))
> > + continue;
> > +
> > + points = 0;
> > + for_each_node_state(nid, N_MEMORY) {
> > + if (oc->nodemask && !node_isset(nid, *oc->nodemask))
> > + continue;
> > + points += mem_cgroup_node_nr_lru_pages(iter, nid,
> > + LRU_ALL_ANON | BIT(LRU_UNEVICTABLE));
> > + }
> > + points += mem_cgroup_get_nr_swap_pages(iter);
>
> I guess we should also take into account kmem as well (unreclaimable
> slabs, kernel stacks, socket buffers).
Added to v2 (I'll post it soon).
Good idea, thanks!
--
Roman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] Patch for remapping pages around the fault page
From: kbuild test robot @ 2017-05-22 17:07 UTC (permalink / raw)
To: Sarunya Pumma
Cc: kbuild-all, rppt, linux-mm, akpm, kirill.shutemov, jack,
ross.zwisler, mhocko, aneesh.kumar, lstoakes, dave.jiang
In-Reply-To: <1495379520-23752-1-git-send-email-sarunya@vt.edu>
[-- Attachment #1: Type: text/plain, Size: 989 bytes --]
Hi Sarunya,
[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.12-rc2 next-20170522]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sarunya-Pumma/Patch-for-remapping-pages-around-the-fault-page/20170522-211816
base: git://git.cmpxchg.org/linux-mmotm.git master
config: c6x-evmc6678_defconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=c6x
All errors (new ones prefixed by >>):
>> kernel/built-in.o:(.fardata+0x1b2c): undefined reference to `vm_nr_remapping'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 5427 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v2 11/17] cgroup: Implement new thread mode semantics
From: Waiman Long @ 2017-05-22 17:13 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170519202624.GA15279@wtj.duckdns.org>
On 05/19/2017 04:26 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:10AM -0400, Waiman Long wrote:
>> Now we could have something like
>>
>> R -- A -- B
>> \
>> T1 -- T2
>>
>> where R is the thread root, A and B are non-threaded cgroups, T1 and
>> T2 are threaded cgroups. The cgroups R, T1, T2 form a threaded subtree
>> where all the non-threaded resources are accounted for in R. The no
>> internal process constraint does not apply in the threaded subtree.
>> Non-threaded controllers need to properly handle the competition
>> between internal processes and child cgroups at the thread root.
>>
>> This model will be flexible enough to support the need of the threaded
>> controllers.
> Maybe I'm misunderstanding the design, but this seems to push the
> processes which belong to the threaded subtree to the parent which is
> part of the usual resource domain hierarchy thus breaking the no
> internal competition constraint. I'm not sure this is something we'd
> want. Given that the limitation of the original threaded mode was the
> required nesting below root and that we treat root special anyway
> (exactly in the way necessary), I wonder whether it'd be better to
> simply allow root to be both domain and thread root.
Yes, root can be both domain and thread root. I haven't placed any
restriction on that.
>
> Specific review points below but we'd probably want to discuss the
> overall design first.
>
>> +static inline bool cgroup_is_threaded(const struct cgroup *cgrp)
>> +{
>> + return cgrp->proc_cgrp && (cgrp->proc_cgrp != cgrp);
>> +}
>> +
>> +static inline bool cgroup_is_thread_root(const struct cgroup *cgrp)
>> +{
>> + return cgrp->proc_cgrp == cgrp;
>> +}
> Maybe add a bit of comments explaining what's going on with
> ->proc_cgrp?
Sure, will do that.
>> /**
>> + * threaded_children_count - returns # of threaded children
>> + * @cgrp: cgroup to be tested
>> + *
>> + * cgroup_mutex must be held by the caller.
>> + */
>> +static int threaded_children_count(struct cgroup *cgrp)
>> +{
>> + struct cgroup *child;
>> + int count = 0;
>> +
>> + lockdep_assert_held(&cgroup_mutex);
>> + cgroup_for_each_live_child(child, cgrp)
>> + if (cgroup_is_threaded(child))
>> + count++;
>> + return count;
>> +}
> It probably would be a good idea to keep track of the count so that we
> don't have to count them each time. There are cases where people end
> up creating a very high number of cgroups and we've already been
> bitten a couple times with silly complexity issues.
Thanks for the suggestion, I can keep a count in the cgroup strcture to
avoid doing that repetitively.
>
>> @@ -2982,22 +3010,48 @@ static int cgroup_enable_threaded(struct cgroup *cgrp)
>> LIST_HEAD(csets);
>> struct cgrp_cset_link *link;
>> struct css_set *cset, *cset_next;
>> + struct cgroup *child;
>> int ret;
>> + u16 ss_mask;
>>
>> lockdep_assert_held(&cgroup_mutex);
>>
>> /* noop if already threaded */
>> - if (cgrp->proc_cgrp)
>> + if (cgroup_is_threaded(cgrp))
>> return 0;
>>
>> - /* allow only if there are neither children or enabled controllers */
>> - if (css_has_online_children(&cgrp->self) || cgrp->subtree_control)
>> + /*
>> + * Allow only if it is not the root and there are:
>> + * 1) no children,
>> + * 2) no non-threaded controllers are enabled, and
>> + * 3) no attached tasks.
>> + *
>> + * With no attached tasks, it is assumed that no css_sets will be
>> + * linked to the current cgroup. This may not be true if some dead
>> + * css_sets linger around due to task_struct leakage, for example.
>> + */
> It doesn't look like the code is actually making this (incorrect)
> assumption. I suppose the comment is from before
> cgroup_is_populated() was added?
Yes, it is a bug. I should have checked the tasks_count instead of using
cgroup_is_populated. Thanks for catching that.
>
>> spin_lock_irq(&css_set_lock);
>> list_for_each_entry(link, &cgrp->cset_links, cset_link) {
>> cset = link->cset;
>> + if (cset->dead)
>> + continue;
> Hmm... is this a bug fix which is necessary regardless of whether we
> change the threadroot semantics or not?
That is true. I put it there because the the reference counting bug
fixed in patch 6 caused a lot of dead csets hanging around before the
fix. I can pull this out as a separate patch.
Cheers,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v2 11/17] cgroup: Implement new thread mode semantics
From: Waiman Long @ 2017-05-22 17:32 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <b1d02881-f522-8baa-5ebe-9b1ad74a03e4@redhat.com>
On 05/22/2017 01:13 PM, Waiman Long wrote:
> On 05/19/2017 04:26 PM, Tejun Heo wrote:
>>> @@ -2982,22 +3010,48 @@ static int cgroup_enable_threaded(struct cgroup *cgrp)
>>> LIST_HEAD(csets);
>>> struct cgrp_cset_link *link;
>>> struct css_set *cset, *cset_next;
>>> + struct cgroup *child;
>>> int ret;
>>> + u16 ss_mask;
>>>
>>> lockdep_assert_held(&cgroup_mutex);
>>>
>>> /* noop if already threaded */
>>> - if (cgrp->proc_cgrp)
>>> + if (cgroup_is_threaded(cgrp))
>>> return 0;
>>>
>>> - /* allow only if there are neither children or enabled controllers */
>>> - if (css_has_online_children(&cgrp->self) || cgrp->subtree_control)
>>> + /*
>>> + * Allow only if it is not the root and there are:
>>> + * 1) no children,
>>> + * 2) no non-threaded controllers are enabled, and
>>> + * 3) no attached tasks.
>>> + *
>>> + * With no attached tasks, it is assumed that no css_sets will be
>>> + * linked to the current cgroup. This may not be true if some dead
>>> + * css_sets linger around due to task_struct leakage, for example.
>>> + */
>> It doesn't look like the code is actually making this (incorrect)
>> assumption. I suppose the comment is from before
>> cgroup_is_populated() was added?
> Yes, it is a bug. I should have checked the tasks_count instead of using
> cgroup_is_populated. Thanks for catching that.
Sorry, I would like to take it back. I think cgroup_is_populated() will
be set if there is any task attached to the cgroup. So I think it is
doing the right thing with regard to (3).
Cheers,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] mm: kmemleak: Treat vm_struct as alternative reference to vmalloc'ed objects
From: Catalin Marinas @ 2017-05-22 17:35 UTC (permalink / raw)
To: linux-mm; +Cc: linux-kernel, Michal Hocko, Andy Lutomirski, Luis R. Rodriguez
Kmemleak requires that vmalloc'ed objects have a minimum reference count
of 2: one in the corresponding vm_struct object and the other owned by
the vmalloc() caller. There are cases, however, where the original
vmalloc() returned pointer is lost and, instead, a pointer to vm_struct
is stored (see free_thread_stack()). Kmemleak currently reports such
objects as leaks.
This patch adds support for treating any surplus references to an object
as additional references to a specified object. It introduces the
kmemleak_vmalloc() API function which takes a vm_struct pointer and sets
its surplus reference passing to the actual vmalloc() returned pointer.
The __vmalloc_node_range() calling site has been modified accordingly.
An unrelated minor change is included in this patch to change the type
of kmemleak_object.flags to unsigned int (previously unsigned long).
Reported-by: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
Hi,
As per [1], I added support to use pointers to vm_struct as an
alternative way to avoid false positives when the original vmalloc()
pointer has been lost. This is slightly harder to reason about but it
seems to work for this use-case. I'm not aware of other cases (than
free_thread_stack()) where the original vmalloc() pointer is removed in
favour of a vm_struct one.
An alternative implementation (simpler to understand), if preferred, is
to annotate alloc_thread_stack_node() and free_thread_stack() with
kmemleak_unignore()/kmemleak_ignore() calls and proper comments.
Feedback welcome, I'm fine with either option.
Thanks.
[1] http://lkml.kernel.org/r/20170517110922.GA18716@e104818-lin.cambridge.arm.com
Documentation/dev-tools/kmemleak.rst | 1 +
include/linux/kmemleak.h | 7 ++
mm/kmemleak.c | 136 +++++++++++++++++++++++++++++------
mm/vmalloc.c | 7 +-
4 files changed, 122 insertions(+), 29 deletions(-)
diff --git a/Documentation/dev-tools/kmemleak.rst b/Documentation/dev-tools/kmemleak.rst
index b2391b829169..cb8862659178 100644
--- a/Documentation/dev-tools/kmemleak.rst
+++ b/Documentation/dev-tools/kmemleak.rst
@@ -150,6 +150,7 @@ See the include/linux/kmemleak.h header for the functions prototype.
- ``kmemleak_init`` - initialize kmemleak
- ``kmemleak_alloc`` - notify of a memory block allocation
- ``kmemleak_alloc_percpu`` - notify of a percpu memory block allocation
+- ``kmemleak_vmalloc`` - notify of a vmalloc() memory allocation
- ``kmemleak_free`` - notify of a memory block freeing
- ``kmemleak_free_part`` - notify of a partial memory block freeing
- ``kmemleak_free_percpu`` - notify of a percpu memory block freeing
diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
index 1c2a32829620..590343f6c1b1 100644
--- a/include/linux/kmemleak.h
+++ b/include/linux/kmemleak.h
@@ -22,6 +22,7 @@
#define __KMEMLEAK_H
#include <linux/slab.h>
+#include <linux/vmalloc.h>
#ifdef CONFIG_DEBUG_KMEMLEAK
@@ -30,6 +31,8 @@ extern void kmemleak_alloc(const void *ptr, size_t size, int min_count,
gfp_t gfp) __ref;
extern void kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
gfp_t gfp) __ref;
+extern void kmemleak_vmalloc(const struct vm_struct *area, size_t size,
+ gfp_t gfp) __ref;
extern void kmemleak_free(const void *ptr) __ref;
extern void kmemleak_free_part(const void *ptr, size_t size) __ref;
extern void kmemleak_free_percpu(const void __percpu *ptr) __ref;
@@ -81,6 +84,10 @@ static inline void kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
gfp_t gfp)
{
}
+static inline void kmemleak_vmalloc(const struct vm_struct *area, size_t size,
+ gfp_t gfp)
+{
+}
static inline void kmemleak_free(const void *ptr)
{
}
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 20036d4f9f13..11ab654502fd 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -150,7 +150,7 @@ struct kmemleak_scan_area {
*/
struct kmemleak_object {
spinlock_t lock;
- unsigned long flags; /* object status flags */
+ unsigned int flags; /* object status flags */
struct list_head object_list;
struct list_head gray_list;
struct rb_node rb_node;
@@ -159,6 +159,8 @@ struct kmemleak_object {
atomic_t use_count;
unsigned long pointer;
size_t size;
+ /* pass surplus references to this pointer */
+ unsigned long excess_ref;
/* minimum number of a pointers found before it is considered leak */
int min_count;
/* the total number of pointers found pointing to this object */
@@ -253,7 +255,8 @@ enum {
KMEMLEAK_NOT_LEAK,
KMEMLEAK_IGNORE,
KMEMLEAK_SCAN_AREA,
- KMEMLEAK_NO_SCAN
+ KMEMLEAK_NO_SCAN,
+ KMEMLEAK_SET_EXCESS_REF
};
/*
@@ -262,9 +265,12 @@ enum {
*/
struct early_log {
int op_type; /* kmemleak operation type */
- const void *ptr; /* allocated/freed memory block */
- size_t size; /* memory block size */
int min_count; /* minimum reference count */
+ const void *ptr; /* allocated/freed memory block */
+ union {
+ size_t size; /* memory block size */
+ unsigned long excess_ref; /* surplus reference passing */
+ };
unsigned long trace[MAX_TRACE]; /* stack trace */
unsigned int trace_len; /* stack trace length */
};
@@ -393,7 +399,7 @@ static void dump_object_info(struct kmemleak_object *object)
object->comm, object->pid, object->jiffies);
pr_notice(" min_count = %d\n", object->min_count);
pr_notice(" count = %d\n", object->count);
- pr_notice(" flags = 0x%lx\n", object->flags);
+ pr_notice(" flags = 0x%x\n", object->flags);
pr_notice(" checksum = %u\n", object->checksum);
pr_notice(" backtrace:\n");
print_stack_trace(&trace, 4);
@@ -562,6 +568,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
object->flags = OBJECT_ALLOCATED;
object->pointer = ptr;
object->size = size;
+ object->excess_ref = 0;
object->min_count = min_count;
object->count = 0; /* white color initially */
object->jiffies = jiffies;
@@ -795,6 +802,30 @@ static void add_scan_area(unsigned long ptr, size_t size, gfp_t gfp)
}
/*
+ * Any surplus references (object already gray) to 'ptr' are passed to
+ * 'excess_ref'. This is used in the vmalloc() case where a pointer to
+ * vm_struct may be used as an alternative reference to the vmalloc'ed object
+ * (see free_thread_stack()).
+ */
+static void object_set_excess_ref(unsigned long ptr, unsigned long excess_ref)
+{
+ unsigned long flags;
+ struct kmemleak_object *object;
+
+ object = find_and_get_object(ptr, 0);
+ if (!object) {
+ kmemleak_warn("Setting excess_ref on unknown object at 0x%08lx\n",
+ ptr);
+ return;
+ }
+
+ spin_lock_irqsave(&object->lock, flags);
+ object->excess_ref = excess_ref;
+ spin_unlock_irqrestore(&object->lock, flags);
+ put_object(object);
+}
+
+/*
* Set the OBJECT_NO_SCAN flag for the object corresponding to the give
* pointer. Such object will not be scanned by kmemleak but references to it
* are searched.
@@ -908,7 +939,7 @@ static void early_alloc_percpu(struct early_log *log)
* @gfp: kmalloc() flags used for kmemleak internal memory allocations
*
* This function is called from the kernel allocators when a new object
- * (memory block) is allocated (kmem_cache_alloc, kmalloc, vmalloc etc.).
+ * (memory block) is allocated (kmem_cache_alloc, kmalloc etc.).
*/
void __ref kmemleak_alloc(const void *ptr, size_t size, int min_count,
gfp_t gfp)
@@ -952,6 +983,36 @@ void __ref kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
EXPORT_SYMBOL_GPL(kmemleak_alloc_percpu);
/**
+ * kmemleak_vmalloc - register a newly vmalloc'ed object
+ * @area: pointer to vm_struct
+ * @size: size of the object
+ * @gfp: __vmalloc() flags used for kmemleak internal memory allocations
+ *
+ * This function is called from the vmalloc() kernel allocator when a new
+ * object (memory block) is allocated.
+ */
+void __ref kmemleak_vmalloc(const struct vm_struct *area, size_t size, gfp_t gfp)
+{
+ pr_debug("%s(0x%p, %zu)\n", __func__, area, size);
+
+ /*
+ * A min_count = 2 is needed because vm_struct contains a reference to
+ * the virtual address of the vmalloc'ed block.
+ */
+ if (kmemleak_enabled) {
+ create_object((unsigned long)area->addr, size, 2, gfp);
+ object_set_excess_ref((unsigned long)area,
+ (unsigned long)area->addr);
+ } else if (kmemleak_early_log) {
+ log_early(KMEMLEAK_ALLOC, area->addr, size, 2);
+ /* reusing early_log.size for storing area->addr */
+ log_early(KMEMLEAK_SET_EXCESS_REF,
+ area, (unsigned long)area->addr, 0);
+ }
+}
+EXPORT_SYMBOL_GPL(kmemleak_vmalloc);
+
+/**
* kmemleak_free - unregister a previously registered object
* @ptr: pointer to beginning of the object
*
@@ -1188,6 +1249,30 @@ static bool update_checksum(struct kmemleak_object *object)
}
/*
+ * Update an object's references. object->lock must be held by the caller.
+ */
+static void update_refs(struct kmemleak_object *object)
+{
+ if (!color_white(object)) {
+ /* non-orphan, ignored or new */
+ return;
+ }
+
+ /*
+ * Increase the object's reference count (number of pointers to the
+ * memory block). If this count reaches the required minimum, the
+ * object's color will become gray and it will be added to the
+ * gray_list.
+ */
+ object->count++;
+ if (color_gray(object)) {
+ /* put_object() called when removing from gray_list */
+ WARN_ON(!get_object(object));
+ list_add_tail(&object->gray_list, &gray_list);
+ }
+}
+
+/*
* Memory scanning is a long process and it needs to be interruptable. This
* function checks whether such interrupt condition occurred.
*/
@@ -1224,6 +1309,7 @@ static void scan_block(void *_start, void *_end,
for (ptr = start; ptr < end; ptr++) {
struct kmemleak_object *object;
unsigned long pointer;
+ unsigned long excess_ref;
if (scan_should_stop())
break;
@@ -1259,25 +1345,25 @@ static void scan_block(void *_start, void *_end,
* enclosed by scan_mutex.
*/
spin_lock_nested(&object->lock, SINGLE_DEPTH_NESTING);
- if (!color_white(object)) {
- /* non-orphan, ignored or new */
- spin_unlock(&object->lock);
- continue;
- }
+ /* only pass surplus references (object already gray) */
+ if (color_gray(object))
+ excess_ref = object->excess_ref;
+ else
+ excess_ref = 0;
+ update_refs(object);
+ spin_unlock(&object->lock);
- /*
- * Increase the object's reference count (number of pointers
- * to the memory block). If this count reaches the required
- * minimum, the object's color will become gray and it will be
- * added to the gray_list.
- */
- object->count++;
- if (color_gray(object)) {
- /* put_object() called when removing from gray_list */
- WARN_ON(!get_object(object));
- list_add_tail(&object->gray_list, &gray_list);
+ if (excess_ref) {
+ object = lookup_object(excess_ref, 0);
+ if (!object)
+ continue;
+ if (object == scanned)
+ /* circular reference, ignore */
+ continue;
+ spin_lock_nested(&object->lock, SINGLE_DEPTH_NESTING);
+ update_refs(object);
+ spin_unlock(&object->lock);
}
- spin_unlock(&object->lock);
}
read_unlock_irqrestore(&kmemleak_lock, flags);
}
@@ -1980,6 +2066,10 @@ void __init kmemleak_init(void)
case KMEMLEAK_NO_SCAN:
kmemleak_no_scan(log->ptr);
break;
+ case KMEMLEAK_SET_EXCESS_REF:
+ object_set_excess_ref((unsigned long)log->ptr,
+ log->excess_ref);
+ break;
default:
kmemleak_warn("Unknown early log operation: %d\n",
log->op_type);
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 34a1c3e46ed7..b805cc5ecca0 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1759,12 +1759,7 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align,
*/
clear_vm_uninitialized_flag(area);
- /*
- * A ref_count = 2 is needed because vm_struct allocated in
- * __get_vm_area_node() contains a reference to the virtual address of
- * the vmalloc'ed block.
- */
- kmemleak_alloc(addr, real_size, 2, gfp_mask);
+ kmemleak_vmalloc(area, size, gfp_mask);
return addr;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE
From: Mike Rapoport @ 2017-05-22 17:51 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Michal Hocko, Kirill A. Shutemov, Andrew Morton, Arnd Bergmann,
Kirill A. Shutemov, Andrea Arcangeli, Pavel Emelyanov, linux-mm,
lkml
In-Reply-To: <a9e74c22-1a07-f49a-42b5-497fee85e9c9@suse.cz>
On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
> > On Mon, May 22, 2017 at 03:55:48PM +0200, Michal Hocko wrote:
> >> On Mon 22-05-17 16:36:00, Mike Rapoport wrote:
> >>> On Mon, May 22, 2017 at 02:42:43PM +0300, Kirill A. Shutemov wrote:
> >>>> On Mon, May 22, 2017 at 09:12:42AM +0300, Mike Rapoport wrote:
> >>>>> Currently applications can explicitly enable or disable THP for a memory
> >>>>> region using MADV_HUGEPAGE or MADV_NOHUGEPAGE. However, once either of
> >>>>> these advises is used, the region will always have
> >>>>> VM_HUGEPAGE/VM_NOHUGEPAGE flag set in vma->vm_flags.
> >>>>> The MADV_CLR_HUGEPAGE resets both these flags and allows managing THP in
> >>>>> the region according to system-wide settings.
> >>>>
> >>>> Seems reasonable. But could you describe an use-case when it's useful in
> >>>> real world.
> >>>
> >>> My use-case was combination of pre- and post-copy migration of containers
> >>> with CRIU.
> >>> In this case we populate a part of a memory region with data that was saved
> >>> during the pre-copy stage. Afterwards, the region is registered with
> >>> userfaultfd and we expect to get page faults for the parts of the region
> >>> that were not yet populated. However, khugepaged collapses the pages and
> >>> the page faults we would expect do not occur.
> >>
> >> I am not sure I undestand the problem. Do I get it right that the
> >> khugepaged will effectivelly corrupt the memory by collapsing a range
> >> which is not yet fully populated? If yes shouldn't that be fixed in
> >> khugepaged rather than adding yet another madvise command? Also how do
> >> you prevent on races? (say you VM_NOHUGEPAGE, khugepaged would be in the
> >> middle of the operation and sees a collapsable vma and you get the same
> >> result)
> >
> > Probably I didn't explained it too well.
> >
> > The range is intentionally not populated. When we combine pre- and
> > post-copy for process migration, we create memory pre-dump without stopping
> > the process, then we freeze the process without dumping the pages it has
> > dirtied between pre-dump and freeze, and then, during restore, we populate
> > the dirtied pages using userfaultfd.
> >
> > When CRIU restores a process in such scenario, it does something like:
> >
> > * mmap() memory region
> > * fill in the pages that were collected during the pre-dump
> > * do some other stuff
> > * register memory region with userfaultfd
> > * populate the missing memory on demand
> >
> > khugepaged collapses the pages in the partially populated regions before we
> > have a chance to register these regions with userfaultfd, which would
> > prevent the collapse.
> >
> > We could have used MADV_NOHUGEPAGE right after the mmap() call, and then
> > there would be no race because there would be nothing for khugepaged to
> > collapse at that point. But the problem is that we have no way to reset
> > *HUGEPAGE flags after the memory restore is complete.
>
> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
> this scenario is indeed impossible?
>
> - you do the mmap
> - khugepaged will choose the process' mm to scan
> - khugepaged will get to the vma in question, it doesn't have
> MADV_NOHUGEPAGE yet
> - you set MADV_NOHUGEPAGE on the vma
> - you start populating the vma
> - khugepaged sees the vma is non-empty, collapses
>
> unless I'm wrong, the racers will have mmap_sem for reading only when
> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.
madvise(MADV_*HUGEPAGE) takes mmap_sem for writing, so it is safe.
> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
> nonzero value, the flag is set, otherwise it is cleared." says the
> manpage. Do it before the mmap and you avoid the race as well?
I've missed that one, thanks Vlastimil!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: dm ioctl: Restore __GFP_HIGH in copy_params()
From: Mike Snitzer @ 2017-05-22 18:04 UTC (permalink / raw)
To: Michal Hocko
Cc: Mikulas Patocka, Junaid Shahid, David Rientjes, Alasdair Kergon,
Andrew Morton, linux-mm, andreslc, gthelen, vbabka, linux-kernel
In-Reply-To: <20170522150321.GM8509@dhcp22.suse.cz>
On Mon, May 22 2017 at 11:03am -0400,
Michal Hocko <mhocko@kernel.org> wrote:
> On Mon 22-05-17 10:52:44, Mikulas Patocka wrote:
> >
> >
> > On Mon, 22 May 2017, Michal Hocko wrote:
> [...]
> > > I am not sure I understand. OOM killer is invoked for _all_ allocations
> > > <= PAGE_ALLOC_COSTLY_ORDER that do not have __GFP_NORETRY as long as the
> > > OOM killer is not disabled (oom_killer_disable) and that only happens
> > > from the PM suspend path which makes sure that no userspace is active at
> > > the time. AFAIU this is a userspace triggered path and so the later
> > > shouldn't apply to it and GFP_KERNEL should be therefore sufficient.
> > > Relying to a portion of memory reserves to prevent from deadlock seems
> > > fundamentaly broken to me.
> > >
> >
> > The lvm2 was designed this way - it is broken, but there is not much that
> > can be done about it - fixing this would mean major rewrite. The only
> > thing we can do about it is to lower the deadlock probability with
> > __GFP_HIGH (or PF_MEMALLOC that was used some times ago).
Yes, lvm2 was originally designed to to have access to memory reserves
to ensure forward progress. But if the mm subsystem has improved to
allow for the required progress without lvm2 trying to stake a claim on
those reserves then we'll gladly avoid (ab)using them.
> But let me repeat. GFP_KERNEL allocation for order-0 page will not fail.
OK, but will it be serviced immediately? Not failing isn't useful if it
never completes.
> If you need non-failing semantic then just make it clear by adding
> __GFP_NOFAIL rather than __GFP_HIGH. Memory reserves are a scarce
> resource and there are users which might really need it from atomic
> contexts.
While adding the __GFP_NOFAIL flag would serve to document expectations
I'm left unconvinced that the memory allocator will _not fail_ for an
order-0 page -- as Mikulas said most ioctls don't need more than 4K.
(Apologies if you've already covered _why_ we can have confidence in the
mm subsystem's ability to ensure forward progress for these allocations).
Mike
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm/oom_kill: count global and memory cgroup oom kills
From: Roman Guschin @ 2017-05-22 18:05 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: linux-mm, Andrew Morton, Tejun Heo, cgroups, linux-kernel,
Vlastimil Babka, Michal Hocko, Johannes Weiner
In-Reply-To: <ecd4a7ea-06c0-f549-a1bf-6d2d3c0af719@yandex-team.ru>
2017-05-22 10:11 GMT+01:00 Konstantin Khlebnikov <khlebnikov@yandex-team.ru>:
>
>
> On 19.05.2017 19:34, Roman Guschin wrote:
>>
>> 2017-05-19 15:22 GMT+01:00 Konstantin Khlebnikov
>> <khlebnikov@yandex-team.ru>:
>> From a user's point of view the difference between "oom" and "max"
>> becomes really vague here,
>> assuming that "max" is described almost in the same words:
>>
>> "The number of times the cgroup's memory usage was
>> about to go over the max boundary. If direct reclaim
>> fails to bring it down, the OOM killer is invoked."
>>
>> I wonder, if it's better to fix the existing "oom" value to show what
>> it has to show, according to docs,
>> rather than to introduce a new one?
>>
>
> Nope, they are different. I think we should rephase documentation somehow
>
> low - count of reclaims below low level
> high - count of post-allocation reclaims above high level
> max - count of direct reclaims
> oom - count of failed direct reclaims
> oom_kill - count of oom killer invocations and killed processes
Definitely worth it.
Also, I would prefer to reserve "oom" for number of oom victims,
and introduce something like "reclaim_failed".
It will be consistent with existing vmstat.
Thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: kmemleak: Treat vm_struct as alternative reference to vmalloc'ed objects
From: Andy Lutomirski @ 2017-05-22 18:19 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Michal Hocko,
Luis R. Rodriguez
In-Reply-To: <1495474514-24425-1-git-send-email-catalin.marinas@arm.com>
On Mon, May 22, 2017 at 10:35 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> Kmemleak requires that vmalloc'ed objects have a minimum reference count
> of 2: one in the corresponding vm_struct object and the other owned by
> the vmalloc() caller. There are cases, however, where the original
> vmalloc() returned pointer is lost and, instead, a pointer to vm_struct
> is stored (see free_thread_stack()). Kmemleak currently reports such
> objects as leaks.
>
> This patch adds support for treating any surplus references to an object
> as additional references to a specified object. It introduces the
> kmemleak_vmalloc() API function which takes a vm_struct pointer and sets
> its surplus reference passing to the actual vmalloc() returned pointer.
> The __vmalloc_node_range() calling site has been modified accordingly.
>
> An unrelated minor change is included in this patch to change the type
> of kmemleak_object.flags to unsigned int (previously unsigned long).
>
> Reported-by: "Luis R. Rodriguez" <mcgrof@kernel.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>
> Hi,
>
> As per [1], I added support to use pointers to vm_struct as an
> alternative way to avoid false positives when the original vmalloc()
> pointer has been lost. This is slightly harder to reason about but it
> seems to work for this use-case. I'm not aware of other cases (than
> free_thread_stack()) where the original vmalloc() pointer is removed in
> favour of a vm_struct one.
>
> An alternative implementation (simpler to understand), if preferred, is
> to annotate alloc_thread_stack_node() and free_thread_stack() with
> kmemleak_unignore()/kmemleak_ignore() calls and proper comments.
>
I personally prefer the option in this patch. It keeps the special
case in kmemleak and the allocation code rather than putting it in the
consumer code.
Also, I want to add an API at some point that vmallocs some memory and
returns the vm_struct directly. That won't work with explicit
annotations in the caller because kmemleak might think it's leaked
before the caller can execute the annotations.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: mm, something wring in page_lock_anon_vma_read()?
From: Hugh Dickins @ 2017-05-22 19:26 UTC (permalink / raw)
To: Xishi Qiu
Cc: Hugh Dickins, Andrew Morton, Tejun Heo, Michal Hocko,
Johannes Weiner, Mel Gorman, Michal Hocko, Vlastimil Babka,
Minchan Kim, David Rientjes, Joonsoo Kim, aarcange,
sumeet.keswani, Rik van Riel, Linux MM, LKML, zhong jiang
In-Reply-To: <5922B3D4.1030700@huawei.com>
On Mon, 22 May 2017, Xishi Qiu wrote:
> On 2017/5/20 10:40, Hugh Dickins wrote:
> > On Sat, 20 May 2017, Xishi Qiu wrote:
> >>
> >> Here is a bug report form redhat: https://bugzilla.redhat.com/show_bug.cgi?id=1305620
> >> And I meet the bug too. However it is hard to reproduce, and
> >> 624483f3ea82598("mm: rmap: fix use-after-free in __put_anon_vma") is not help.
> >>
> >> From the vmcore, it seems that the page is still mapped(_mapcount=0 and _count=2),
> >> and the value of mapping is a valid address(mapping = 0xffff8801b3e2a101),
> >> but anon_vma has been corrupted.
> >>
> >> Any ideas?
> >
> > Sorry, no. I assume that _mapcount has been misaccounted, for example
> > a pte mapped in on top of another pte; but cannot begin tell you where
>
> Hi Hugh,
>
> What does "a pte mapped in on top of another pte" mean? Could you give more info?
I mean, there are various places in mm/memory.c which decide what they
intend to do based on orig_pte, then take pte lock, then check that
pte_same(pte, orig_pte) before taking it any further. If a pte_same()
check were missing (I do not know of any such case), then two racing
tasks might install the same pte, one on top of the other - page
mapcount being incremented twice, but decremented only once when
that pte is finally unmapped later.
Please see similar discussion in the earlier thread at
marc.info/?l=linux-mm&m=148222656211837&w=2
Hugh
>
> Thanks,
> Xishi Qiu
>
> > in Red Hat's kernel-3.10.0-229.4.2.el7 that might happen.
> >
> > Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Dan Williams @ 2017-05-22 19:29 UTC (permalink / raw)
To: Jérôme Glisse
Cc: Andrew Morton, linux-kernel@vger.kernel.org, Linux MM,
John Hubbard, David Nellans, Kirill A . Shutemov, Ross Zwisler
In-Reply-To: <20170522165206.6284-9-jglisse@redhat.com>
On Mon, May 22, 2017 at 9:51 AM, Jérôme Glisse <jglisse@redhat.com> wrote:
> A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
> have any user. For device private pages this is important to catch
> and thus we need to special case put_page() for this.
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
> include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
> kernel/memremap.c | 1 -
> 2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a825dab..11f7bac 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -23,6 +23,7 @@
> #include <linux/page_ext.h>
> #include <linux/err.h>
> #include <linux/page_ref.h>
> +#include <linux/memremap.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
> return ((page_zonenum(page) == ZONE_DEVICE) &&
> (page->pgmap->type == MEMORY_DEVICE_PRIVATE));
> }
> +
> +static inline void put_zone_device_private_page(struct page *page)
> +{
> + int count = page_ref_dec_return(page);
> +
> + /*
> + * If refcount is 1 then page is freed and refcount is stable as nobody
> + * holds a reference on the page.
> + */
> + if (count == 1)
> + page->pgmap->page_free(page, page->pgmap->data);
> + else if (!count)
> + __put_page(page);
> +}
> #else
> static inline bool is_zone_device_page(const struct page *page)
> {
> @@ -805,6 +820,10 @@ static inline bool is_device_private_page(const struct page *page)
> {
> return false;
> }
> +
> +static inline void put_zone_device_private_page(struct page *page)
> +{
> +}
> #endif
>
> static inline void get_page(struct page *page)
> @@ -822,6 +841,17 @@ static inline void put_page(struct page *page)
> {
> page = compound_head(page);
>
> + /*
> + * For private device pages we need to catch refcount transition from
> + * 2 to 1, when refcount reach one it means the private device page is
> + * free and we need to inform the device driver through callback. See
> + * include/linux/memremap.h and HMM for details.
> + */
> + if (unlikely(is_device_private_page(page))) {
Since I presume HMM is a niche use case can we make this a
"static_branch_unlikely(&hmm_key) && is_device_private_page(page))"?
That way non-hmm platforms see minimal overhead.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 2/2] dax: Fix race between colliding PMD & PTE entries
From: Ross Zwisler @ 2017-05-22 19:43 UTC (permalink / raw)
To: Jan Kara
Cc: Ross Zwisler, Andrew Morton, linux-kernel, Darrick J. Wong,
Alexander Viro, Christoph Hellwig, Dan Williams, Dave Hansen,
Matthew Wilcox, linux-fsdevel, linux-mm, linux-nvdimm,
Kirill A . Shutemov, Pawel Lebioda, Dave Jiang, Xiong Zhou,
Eryu Guan, stable
In-Reply-To: <20170522144457.GE25118@quack2.suse.cz>
On Mon, May 22, 2017 at 04:44:57PM +0200, Jan Kara wrote:
> On Wed 17-05-17 11:16:39, Ross Zwisler wrote:
> > We currently have two related PMD vs PTE races in the DAX code. These can
> > both be easily triggered by having two threads reading and writing
> > simultaneously to the same private mapping, with the key being that private
> > mapping reads can be handled with PMDs but private mapping writes are
> > always handled with PTEs so that we can COW.
> >
> > Here is the first race:
> >
> > CPU 0 CPU 1
> >
> > (private mapping write)
> > __handle_mm_fault()
> > create_huge_pmd() - FALLBACK
> > handle_pte_fault()
> > passes check for pmd_devmap()
> >
> > (private mapping read)
> > __handle_mm_fault()
> > create_huge_pmd()
> > dax_iomap_pmd_fault() inserts PMD
> >
> > dax_iomap_pte_fault() does a PTE fault, but we already have a DAX PMD
> > installed in our page tables at this spot.
> >
> > Here's the second race:
> >
> > CPU 0 CPU 1
> >
> > (private mapping write)
> > __handle_mm_fault()
> > create_huge_pmd() - FALLBACK
> > (private mapping read)
> > __handle_mm_fault()
> > passes check for pmd_none()
> > create_huge_pmd()
> >
> > handle_pte_fault()
> > dax_iomap_pte_fault() inserts PTE
> > dax_iomap_pmd_fault() inserts PMD,
> > but we already have a PTE at
> > this spot.
> >
> > The core of the issue is that while there is isolation between faults to
> > the same range in the DAX fault handlers via our DAX entry locking, there
> > is no isolation between faults in the code in mm/memory.c. This means for
> > instance that this code in __handle_mm_fault() can run:
> >
> > if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
> > ret = create_huge_pmd(&vmf);
> >
> > But by the time we actually get to run the fault handler called by
> > create_huge_pmd(), the PMD is no longer pmd_none() because a racing PTE
> > fault has installed a normal PMD here as a parent. This is the cause of
> > the 2nd race. The first race is similar - there is the following check in
> > handle_pte_fault():
> >
> > } else {
> > /* See comment in pte_alloc_one_map() */
> > if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
> > return 0;
> >
> > So if a pmd_devmap() PMD (a DAX PMD) has been installed at vmf->pmd, we
> > will bail and retry the fault. This is correct, but there is nothing
> > preventing the PMD from being installed after this check but before we
> > actually get to the DAX PTE fault handlers.
> >
> > In my testing these races result in the following types of errors:
> >
> > BUG: Bad rss-counter state mm:ffff8800a817d280 idx:1 val:1
> > BUG: non-zero nr_ptes on freeing mm: 15
> >
> > Fix this issue by having the DAX fault handlers verify that it is safe to
> > continue their fault after they have taken an entry lock to block other
> > racing faults.
> >
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Reported-by: Pawel Lebioda <pawel.lebioda@intel.com>
> > Cc: stable@vger.kernel.org
> >
> > ---
> >
> > I've written a new xfstest for this race, which I will send in response to
> > this patch series. This series has also survived an xfstest run without
> > any new issues.
> >
> > ---
> > fs/dax.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/fs/dax.c b/fs/dax.c
> > index c22eaf1..3cc02d1 100644
> > --- a/fs/dax.c
> > +++ b/fs/dax.c
> > @@ -1155,6 +1155,15 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
> > }
> >
> > /*
> > + * It is possible, particularly with mixed reads & writes to private
> > + * mappings, that we have raced with a PMD fault that overlaps with
> > + * the PTE we need to set up. Now that we have a locked mapping entry
> > + * we can safely unmap the huge PMD so that we can install our PTE in
> > + * our page tables.
> > + */
> > + split_huge_pmd(vmf->vma, vmf->pmd, vmf->address);
> > +
>
> Can we just check the PMD and if is isn't as we want it, bail out and retry
> the fault? IMHO it will be more obvious that way (and also more in line
> like these races are handled for the classical THP). Otherwise the patch
> looks good to me.
Yep, that works as well. I'll do this for v2.
Thanks for the review.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 2/2] dax: Fix race between colliding PMD & PTE entries
From: Ross Zwisler @ 2017-05-22 19:44 UTC (permalink / raw)
To: Jan Kara
Cc: Ross Zwisler, Andrew Morton, linux-kernel, Darrick J. Wong,
Alexander Viro, Christoph Hellwig, Dan Williams, Dave Hansen,
Matthew Wilcox, linux-fsdevel, linux-mm, linux-nvdimm,
Kirill A . Shutemov, Pawel Lebioda, Dave Jiang, Xiong Zhou,
Eryu Guan, stable
In-Reply-To: <20170522143748.GC25118@quack2.suse.cz>
On Mon, May 22, 2017 at 04:37:48PM +0200, Jan Kara wrote:
> On Thu 18-05-17 15:29:39, Ross Zwisler wrote:
> > On Thu, May 18, 2017 at 09:50:37AM +0200, Jan Kara wrote:
> > > On Wed 17-05-17 11:16:39, Ross Zwisler wrote:
<>
> > > The first scenario seems to be possible. dax_iomap_pmd_fault() will create
> > > PMD entry in the radix tree. Then dax_iomap_pte_fault() will come, do
> > > grab_mapping_entry(), there it sees entry is PMD but we are doing PTE fault
> > > so I'd think that pmd_downgrade = true... But actually the condition there
> > > doesn't trigger in this case. And that's a catch that although we asked
> > > grab_mapping_entry() for PTE, we've got PMD back and that screws us later.
> >
> > Yep, it was a concious decision when implementing the PMD support to allow one
> > thread to use PMDs and another to use PTEs in the same range, as long as the
> > thread faulting in PMDs is the first to insert into the radix tree. A PMD
> > radix tree entry will be inserted and used for locking and dirty tracking, and
> > each thread or process can fault in either PTEs or PMDs into its own address
> > space as needed.
>
> Well, for *threads* it doesn't really make good sense to mix PMDs and PTEs
> as they share page tables. However for *processes* it makes some sense to
> allow one process to use PTEs and another process to use PMDs. And I
> remember we were discussing this in the past.
Ugh, I was super sloppy with my use of "thread" and "process" in my previous
email. Sorry, and thanks for the clarifications. I think we're on the same
page, even if I had trouble articulating it. :)
> So normal fault path uses alloc_set_pte() for installing new PTE. And that
> uses pte_alloc_one_map() which checks whether PMD is still suitable for
> inserting a PTE. If not, we return VM_FAULT_NOPAGE. Probably it would be
> cleanest to factor our common parts of PTE and PMD insertion so that we can
> use these functions both from DAX and generic fault paths.
Makes sense, thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [kernel-hardening] [PATCH 1/1] Sealable memory support
From: Igor Stoppa @ 2017-05-22 19:45 UTC (permalink / raw)
To: Greg KH
Cc: mhocko, dave.hansen, labbott, linux-mm, kernel-hardening,
linux-kernel
In-Reply-To: <20170520085147.GA4619@kroah.com>
On 20/05/17 11:51, Greg KH wrote:
> On Fri, May 19, 2017 at 01:38:11PM +0300, Igor Stoppa wrote:
>> Dynamically allocated variables can be made read only,
[...]
> This is really nice, do you have a follow-on patch showing how any of
> the kernel can be changed to use this new subsystem?
Yes, actually I started from the need of turning into R/O some data
structures in both LSM Hooks and SE Linux.
> Without that, it
> might be hard to get this approved (we don't like adding new apis
> without users.)
Yes, I just wanted to give an early preview of the current
implementation, since it is significantly different from what I
initially proposed. So I was looking for early feedback.
Right now, I'm fixing it up and adding some more structured debugging.
Then I'll re-submit it together with the LSM Hooks example.
---
thanks, igor
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] LSM: Make security_hook_heads a local variable.
From: Igor Stoppa @ 2017-05-22 19:50 UTC (permalink / raw)
To: Casey Schaufler, Christoph Hellwig, Tetsuo Handa
Cc: linux-security-module, linux-mm, kernel-hardening, linux-kernel,
Greg KH, James Morris, Kees Cook, Paul Moore, Stephen Smalley
In-Reply-To: <d98f4cd5-3f21-3f7b-2842-12b9a009e453@schaufler-ca.com>
On 22/05/17 18:09, Casey Schaufler wrote:
> On 5/22/2017 7:03 AM, Christoph Hellwig wrote:
[...]
>> But even with those we can still chain
>> them together with a list with external linkage.
>
> I gave up that approach in 2012. Too many unnecessary calls to
> null functions, and massive function vectors with a tiny number
> of non-null entries. From a data structure standpoint, it was
> just wrong. The list scheme is exactly right for the task at
> hand.
I understand this as a green light, for me to continue with the plan of
using LSM Hooks as example for making dynamically allocated data become
read-only, using also Tetsuo's patch (thanks, btw).
Is that correct?
---
thanks, igor
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox