Re: dev_pagemap related cleanups v2

Dan Williams
2019-06-18T19:47:22Z

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

On Mon, Jun 17, 2019 at 5:27 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi Dan, Jérôme and Jason,
>
> below is a series that cleans up the dev_pagemap interface so that
> it is more easily usable, which removes the need to wrap it in hmm
> and thus allowing to kill a lot of code
>
> Note: this series is on top of the rdma/hmm branch + the dev_pagemap
> releas fix series from Dan that went into 5.2-rc5.
>
> Git tree:
>
> git://git.infradead.org/users/hch/misc.git hmm-devmem-cleanup.2
>
> Gitweb:
>
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-devmem-cleanup.2
>
> Changes since v1:
> - rebase
> - also switch p2pdma to the internal refcount
> - add type checking for pgmap->type
> - rename the migrate method to migrate_to_ram
> - cleanup the altmap_valid flag
> - various tidbits from the reviews

Attached is my incremental fixups on top of this series, with those
integrated you can add:

Tested-by: Dan Williams <dan.j.williams@intel.com>

...to the patches that touch kernel/memremap.c, drivers/dax, and drivers/nvdimm.

You can also add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

...for the series.

[-- Attachment #2: incremental.diff --]
[-- Type: text/x-patch, Size: 5128 bytes --]

diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index a9d7c90ecf1e..1af823b2fe6b 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -428,6 +428,7 @@ int dev_dax_probe(struct device *dev)
return -EBUSY;
}

+ dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
addr = devm_memremap_pages(dev, &dev_dax->pgmap);
if (IS_ERR(addr))
return PTR_ERR(addr);
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index 54500798f23a..57d3a6c3ac70 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -118,4 +118,15 @@ config NVDIMM_KEYS
depends on ENCRYPTED_KEYS
depends on (LIBNVDIMM=ENCRYPTED_KEYS) || LIBNVDIMM=m

+config NVDIMM_TEST_BUILD
+ bool "Build the unit test core"
+ depends on COMPILE_TEST
+ default COMPILE_TEST
+ help
+ Build the core of the unit test infrastructure. The result of
+ this build is non-functional for unit test execution, but it
+ otherwise helps catch build errors induced by changes to the
+ core devm_memremap_pages() implementation and other
+ infrastructure.
+
endif
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index 6f2a088afad6..40080c120363 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -28,3 +28,7 @@ libnvdimm-$(CONFIG_BTT) += btt_devs.o
libnvdimm-$(CONFIG_NVDIMM_PFN) += pfn_devs.o
libnvdimm-$(CONFIG_NVDIMM_DAX) += dax_devs.o
libnvdimm-$(CONFIG_NVDIMM_KEYS) += security.o
+
+TOOLS := ../../tools
+TEST_SRC := $(TOOLS)/testing/nvdimm/test
+obj-$(CONFIG_NVDIMM_TEST_BUILD) := $(TEST_SRC)/iomap.o
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 7e0f072ddce7..470de68dabd6 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -55,12 +55,19 @@ struct vmem_altmap {
* MEMORY_DEVICE_PCI_P2PDMA:
* Device memory residing in a PCI BAR intended for use with Peer-to-Peer
* transactions.
+ *
+ * MEMORY_DEVICE_DEVDAX:
+ * Host memory that has similar access semantics as System RAM i.e. DMA
+ * coherent and supports page pinning. In contrast to
+ * MEMORY_DEVICE_FS_DAX, this memory is access via a device-dax
+ * character device.
*/
enum memory_type {
MEMORY_DEVICE_PRIVATE = 1,
MEMORY_DEVICE_PUBLIC,
MEMORY_DEVICE_FS_DAX,
MEMORY_DEVICE_PCI_P2PDMA,
+ MEMORY_DEVICE_DEVDAX,
};

struct dev_pagemap_ops {
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 60693a1e8e92..52b4968e62cd 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -173,6 +173,7 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
};
pgprot_t pgprot = PAGE_KERNEL;
int error, nid, is_ram;
+ bool get_ops = true;

switch (pgmap->type) {
case MEMORY_DEVICE_PRIVATE:
@@ -199,6 +200,8 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
}
break;
case MEMORY_DEVICE_PCI_P2PDMA:
+ case MEMORY_DEVICE_DEVDAX:
+ get_ops = false;
break;
default:
WARN(1, "Invalid pgmap type %d\n", pgmap->type);
@@ -222,7 +225,7 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
}
}

- if (pgmap->type != MEMORY_DEVICE_PCI_P2PDMA) {
+ if (get_ops) {
error = dev_pagemap_get_ops(dev, pgmap);
if (error)
return ERR_PTR(error);
diff --git a/tools/testing/nvdimm/test/iomap.c b/tools/testing/nvdimm/test/iomap.c
index 8cd9b9873a7f..9019dd8afbc1 100644
--- a/tools/testing/nvdimm/test/iomap.c
+++ b/tools/testing/nvdimm/test/iomap.c
@@ -106,7 +106,7 @@ EXPORT_SYMBOL(__wrap_devm_memremap);

static void nfit_test_kill(void *_pgmap)
{
- WARN_ON(!pgmap || !pgmap->ref)
+ struct dev_pagemap *pgmap = _pgmap;

if (pgmap->ops && pgmap->ops->kill)
pgmap->ops->kill(pgmap);
@@ -121,20 +121,45 @@ static void nfit_test_kill(void *_pgmap)
}
}

+static void dev_pagemap_percpu_release(struct percpu_ref *ref)
+{
+ struct dev_pagemap *pgmap =
+ container_of(ref, struct dev_pagemap, internal_ref);
+
+ complete(&pgmap->done);
+}
+
void *__wrap_devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
{
+ int error;
resource_size_t offset = pgmap->res.start;
struct nfit_test_resource *nfit_res = get_nfit_res(offset);

- if (nfit_res) {
- int rc;
+ if (!nfit_res)
+ return devm_memremap_pages(dev, pgmap);

- rc = devm_add_action_or_reset(dev, nfit_test_kill, pgmap);
- if (rc)
- return ERR_PTR(rc);
- return nfit_res->buf + offset - nfit_res->res.start;
+ pgmap->dev = dev;
+ if (!pgmap->ref) {
+ if (pgmap->ops && (pgmap->ops->kill || pgmap->ops->cleanup))
+ return ERR_PTR(-EINVAL);
+
+ init_completion(&pgmap->done);
+ error = percpu_ref_init(&pgmap->internal_ref,
+ dev_pagemap_percpu_release, 0, GFP_KERNEL);
+ if (error)
+ return ERR_PTR(error);
+ pgmap->ref = &pgmap->internal_ref;
+ } else {
+ if (!pgmap->ops || !pgmap->ops->kill || !pgmap->ops->cleanup) {
+ WARN(1, "Missing reference count teardown definition\n");
+ return ERR_PTR(-EINVAL);
+ }
}
- return devm_memremap_pages(dev, pgmap);
+
+ error = devm_add_action_or_reset(dev, nfit_test_kill, pgmap);
+ if (error)
+ return ERR_PTR(error);
+ return nfit_res->buf + offset - nfit_res->res.start;
}
EXPORT_SYMBOL_GPL(__wrap_devm_memremap_pages);

[-- Attachment #3: Type: text/plain, Size: 151 bytes --]