Re: [PATCH] device-dax: avoid hang on error before devm_memremap_pages()
On 7/31/2018 7:32 AM, Stefan Hajnoczi wrote: dax_pmem_percpu_exit() waits for dax_pmem_percpu_release() to invoke the dax_pmem->cmp completion. Unfortunately this approach to cleaning up the percpu_ref only works after devm_memremap_pages() was successful. If devm_add_action_or_reset() or devm_memremap_pages() fails, dax_pmem_percpu_release() is not invoked. Therefore dax_pmem_percpu_exit() hangs waiting for the completion: rc = devm_add_action_or_reset(dev, dax_pmem_percpu_exit, _pmem->ref); if (rc) return rc; dax_pmem->pgmap.ref = _pmem->ref; addr = devm_memremap_pages(dev, _pmem->pgmap); Avoid the hang by calling percpu_ref_exit() in the error paths instead of going through dax_pmem_percpu_exit(). Signed-off-by: Stefan Hajnoczi Applied --- Found by code inspection. Compile-tested only. --- drivers/dax/pmem.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c index fd49b24fd6af..99e2aace8078 100644 --- a/drivers/dax/pmem.c +++ b/drivers/dax/pmem.c @@ -105,15 +105,19 @@ static int dax_pmem_probe(struct device *dev) if (rc) return rc; - rc = devm_add_action_or_reset(dev, dax_pmem_percpu_exit, - _pmem->ref); - if (rc) + rc = devm_add_action(dev, dax_pmem_percpu_exit, _pmem->ref); + if (rc) { + percpu_ref_exit(_pmem->ref); return rc; + } dax_pmem->pgmap.ref = _pmem->ref; addr = devm_memremap_pages(dev, _pmem->pgmap); - if (IS_ERR(addr)) + if (IS_ERR(addr)) { + devm_remove_action(dev, dax_pmem_percpu_exit, _pmem->ref); + percpu_ref_exit(_pmem->ref); return PTR_ERR(addr); + } rc = devm_add_action_or_reset(dev, dax_pmem_percpu_kill, _pmem->ref); ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH] tools/testing/nvdimm: improve emulation of smart injection
On 7/30/2018 3:11 PM, Vishal Verma wrote: The emulation for smart injection commands for nfit neglected to check the smart field validity flags before injecting to that field. This is required as a way to distinguish un-injection vs. leave-alone. The emulation was also missing support for un-injection entirely. To add this support, first, fix the above flags check. Second, use the 'enable' field in the injection command to determine injection vs un-injection. Third, move the smart initialization struct to be a global static structure for the nfit_test module. Reference this to get the smart 'defaults' when un-injecting a smart field. Signed-off-by: Vishal Verma applied --- tools/testing/nvdimm/test/nfit.c | 78 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c index a012ab765083..cffc2c5a778d 100644 --- a/tools/testing/nvdimm/test/nfit.c +++ b/tools/testing/nvdimm/test/nfit.c @@ -142,6 +142,28 @@ static u32 handle[] = { static unsigned long dimm_fail_cmd_flags[NUM_DCR]; static int dimm_fail_cmd_code[NUM_DCR]; +static const struct nd_intel_smart smart_def = { + .flags = ND_INTEL_SMART_HEALTH_VALID + | ND_INTEL_SMART_SPARES_VALID + | ND_INTEL_SMART_ALARM_VALID + | ND_INTEL_SMART_USED_VALID + | ND_INTEL_SMART_SHUTDOWN_VALID + | ND_INTEL_SMART_MTEMP_VALID + | ND_INTEL_SMART_CTEMP_VALID, + .health = ND_INTEL_SMART_NON_CRITICAL_HEALTH, + .media_temperature = 23 * 16, + .ctrl_temperature = 25 * 16, + .pmic_temperature = 40 * 16, + .spares = 75, + .alarm_flags = ND_INTEL_SMART_SPARE_TRIP + | ND_INTEL_SMART_TEMP_TRIP, + .ait_status = 1, + .life_used = 5, + .shutdown_state = 0, + .vendor_size = 0, + .shutdown_count = 100, +}; + struct nfit_test_fw { enum intel_fw_update_state state; u32 context; @@ -752,15 +774,30 @@ static int nfit_test_cmd_smart_inject( if (buf_len != sizeof(*inj)) return -EINVAL; - if (inj->mtemp_enable) - smart->media_temperature = inj->media_temperature; - if (inj->spare_enable) - smart->spares = inj->spares; - if (inj->fatal_enable) - smart->health = ND_INTEL_SMART_FATAL_HEALTH; - if (inj->unsafe_shutdown_enable) { - smart->shutdown_state = 1; - smart->shutdown_count++; + if (inj->flags & ND_INTEL_SMART_INJECT_MTEMP) { + if (inj->mtemp_enable) + smart->media_temperature = inj->media_temperature; + else + smart->media_temperature = smart_def.media_temperature; + } + if (inj->flags & ND_INTEL_SMART_INJECT_SPARE) { + if (inj->spare_enable) + smart->spares = inj->spares; + else + smart->spares = smart_def.spares; + } + if (inj->flags & ND_INTEL_SMART_INJECT_FATAL) { + if (inj->fatal_enable) + smart->health = ND_INTEL_SMART_FATAL_HEALTH; + else + smart->health = ND_INTEL_SMART_NON_CRITICAL_HEALTH; + } + if (inj->flags & ND_INTEL_SMART_INJECT_SHUTDOWN) { + if (inj->unsafe_shutdown_enable) { + smart->shutdown_state = 1; + smart->shutdown_count++; + } else + smart->shutdown_state = 0; } inj->status = 0; smart_notify(bus_dev, dimm_dev, smart, thresh); @@ -1317,30 +1354,9 @@ static void smart_init(struct nfit_test *t) .ctrl_temperature = 30 * 16, .spares = 5, }; - const struct nd_intel_smart smart_data = { - .flags = ND_INTEL_SMART_HEALTH_VALID - | ND_INTEL_SMART_SPARES_VALID - | ND_INTEL_SMART_ALARM_VALID - | ND_INTEL_SMART_USED_VALID - | ND_INTEL_SMART_SHUTDOWN_VALID - | ND_INTEL_SMART_MTEMP_VALID - | ND_INTEL_SMART_CTEMP_VALID, - .health = ND_INTEL_SMART_NON_CRITICAL_HEALTH, - .media_temperature = 23 * 16, - .ctrl_temperature = 25 * 16, - .pmic_temperature = 40 * 16, - .spares = 75, - .alarm_flags = ND_INTEL_SMART_SPARE_TRIP - | ND_INTEL_SMART_TEMP_TRIP, - .ait_status = 1, - .life_used = 5, - .shutdown_state = 0, - .vendor_size = 0, - .shutdown_count = 100, - }; for (i = 0; i < t->num_dcr; i++) { - memcpy(>smart[i], _data, sizeof(smart_data)); + memcpy(>smart[i], _def, sizeof(smart_def));
RE: [PATCH v6 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
> -Original Message- > From: Schofield, Alison > Sent: Tuesday, July 31, 2018 3:05 PM > To: Jiang, Dave > Cc: linux-nvdimm@lists.01.org; Williams, Dan J ; > keyri...@vger.kernel.org; dhowe...@redhat.com; > keesc...@chromium.org; elli...@hpe.com; ebigge...@gmail.com > Subject: Re: [PATCH v6 04/11] nfit/libnvdimm: add unlock of nvdimm support > for Intel DIMMs > > On Wed, Jul 25, 2018 at 01:58:53PM -0700, Dave Jiang wrote: > > Add support to allow query the security status of the Intel nvdimms and > > also unlock the dimm via the kernel key management APIs. The passphrase is > > expected to be pulled from userspace through keyutils. Moving the Intel > > related bits to its own source file as well. > > > > Signed-off-by: Dave Jiang > > Reviewed-by: Dan Williams > > --- > > drivers/acpi/nfit/Makefile |1 > > drivers/acpi/nfit/core.c |3 + > > drivers/acpi/nfit/intel.c | 152 > > > > drivers/acpi/nfit/intel.h | 15 > > drivers/nvdimm/dimm.c |7 ++ > > drivers/nvdimm/dimm_devs.c | 132 ++ > > drivers/nvdimm/nd-core.h |4 + > > drivers/nvdimm/nd.h|2 + > > include/linux/libnvdimm.h | 24 +++ > > security/keys/key.c|1 > > Dave - Why don't I see a config change "depends on KEYS" to make > LIBNVDIMM build? Good catch Alison. I'll add. > > > > 10 files changed, 338 insertions(+), 3 deletions(-) > > create mode 100644 drivers/acpi/nfit/intel.c > > > > diff --git a/drivers/acpi/nfit/Makefile b/drivers/acpi/nfit/Makefile > > index a407e769f103..443c7ef4e6a6 100644 > > --- a/drivers/acpi/nfit/Makefile > > +++ b/drivers/acpi/nfit/Makefile > > @@ -1,3 +1,4 @@ > > obj-$(CONFIG_ACPI_NFIT) := nfit.o > > nfit-y := core.o > > +nfit-$(CONFIG_X86) += intel.o > > nfit-$(CONFIG_X86_MCE) += mce.o > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > > index 21235d555b5f..9cb6a108ecba 100644 > > --- a/drivers/acpi/nfit/core.c > > +++ b/drivers/acpi/nfit/core.c > > @@ -1904,7 +1904,8 @@ static int acpi_nfit_register_dimms(struct > > acpi_nfit_desc *acpi_desc) > > nvdimm = nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem, > > acpi_nfit_dimm_attribute_groups, > > flags, cmd_mask, flush ? flush->hint_count : 0, > > - nfit_mem->flush_wpq, _mem->id[0]); > > + nfit_mem->flush_wpq, _mem->id[0], > > + acpi_nfit_get_security_ops(nfit_mem->family)); > > if (!nvdimm) > > return -ENOMEM; > > > > diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c > > new file mode 100644 > > index ..4bfc1c1da339 > > --- /dev/null > > +++ b/drivers/acpi/nfit/intel.c > > @@ -0,0 +1,152 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright(c) 2018 Intel Corporation. All rights reserved. */ > > +/* > > + * Intel specific NFIT ops > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include "intel.h" > > +#include "nfit.h" > > + > > +static int intel_dimm_security_unlock(struct nvdimm_bus *nvdimm_bus, > > + struct nvdimm *nvdimm, const struct nvdimm_key_data *nkey) > > +{ > > + struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus); > > + int cmd_rc, rc = 0; > > + struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm); > > + struct { > > + struct nd_cmd_pkg pkg; > > + struct nd_intel_unlock_unit cmd; > > + } nd_cmd = { > > + .pkg = { > > + .nd_command = NVDIMM_INTEL_UNLOCK_UNIT, > > + .nd_family = NVDIMM_FAMILY_INTEL, > > + .nd_size_in = ND_INTEL_PASSPHRASE_SIZE, > > + .nd_size_out = ND_INTEL_STATUS_SIZE, > > + .nd_fw_size = ND_INTEL_STATUS_SIZE, > > + }, > > + .cmd = { > > + .status = 0, > > + }, > > + }; > > + > > + if (!test_bit(NVDIMM_INTEL_UNLOCK_UNIT, _mem->dsm_mask)) > > + return -ENOTTY; > > + > > + memcpy(nd_cmd.cmd.passphrase, nkey->data, > > + sizeof(nd_cmd.cmd.passphrase)); > > + rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, _cmd, > > + sizeof(nd_cmd), _rc); > > + if (rc < 0) > > + goto out; > > + if (cmd_rc < 0) { > > + rc = cmd_rc; > > + goto out; > > + } > > + > > + switch (nd_cmd.cmd.status) { > > + case 0: > > + break; > > + case ND_INTEL_STATUS_INVALID_PASS: > > + rc = -EINVAL; > > + goto out; > > + case ND_INTEL_STATUS_INVALID_STATE: > > + default: > > + rc = -ENXIO; > > + goto out; > > + } > > + > > + /* > > +* TODO: define a cross arch wbinvd when/if
Re: [PATCH v6 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
On Wed, Jul 25, 2018 at 01:58:53PM -0700, Dave Jiang wrote: > Add support to allow query the security status of the Intel nvdimms and > also unlock the dimm via the kernel key management APIs. The passphrase is > expected to be pulled from userspace through keyutils. Moving the Intel > related bits to its own source file as well. > > Signed-off-by: Dave Jiang > Reviewed-by: Dan Williams > --- > drivers/acpi/nfit/Makefile |1 > drivers/acpi/nfit/core.c |3 + > drivers/acpi/nfit/intel.c | 152 > > drivers/acpi/nfit/intel.h | 15 > drivers/nvdimm/dimm.c |7 ++ > drivers/nvdimm/dimm_devs.c | 132 ++ > drivers/nvdimm/nd-core.h |4 + > drivers/nvdimm/nd.h|2 + > include/linux/libnvdimm.h | 24 +++ > security/keys/key.c|1 Dave - Why don't I see a config change "depends on KEYS" to make LIBNVDIMM build? > 10 files changed, 338 insertions(+), 3 deletions(-) > create mode 100644 drivers/acpi/nfit/intel.c > > diff --git a/drivers/acpi/nfit/Makefile b/drivers/acpi/nfit/Makefile > index a407e769f103..443c7ef4e6a6 100644 > --- a/drivers/acpi/nfit/Makefile > +++ b/drivers/acpi/nfit/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_ACPI_NFIT) := nfit.o > nfit-y := core.o > +nfit-$(CONFIG_X86) += intel.o > nfit-$(CONFIG_X86_MCE) += mce.o > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index 21235d555b5f..9cb6a108ecba 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -1904,7 +1904,8 @@ static int acpi_nfit_register_dimms(struct > acpi_nfit_desc *acpi_desc) > nvdimm = nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem, > acpi_nfit_dimm_attribute_groups, > flags, cmd_mask, flush ? flush->hint_count : 0, > - nfit_mem->flush_wpq, _mem->id[0]); > + nfit_mem->flush_wpq, _mem->id[0], > + acpi_nfit_get_security_ops(nfit_mem->family)); > if (!nvdimm) > return -ENOMEM; > > diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c > new file mode 100644 > index ..4bfc1c1da339 > --- /dev/null > +++ b/drivers/acpi/nfit/intel.c > @@ -0,0 +1,152 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright(c) 2018 Intel Corporation. All rights reserved. */ > +/* > + * Intel specific NFIT ops > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "intel.h" > +#include "nfit.h" > + > +static int intel_dimm_security_unlock(struct nvdimm_bus *nvdimm_bus, > + struct nvdimm *nvdimm, const struct nvdimm_key_data *nkey) > +{ > + struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus); > + int cmd_rc, rc = 0; > + struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm); > + struct { > + struct nd_cmd_pkg pkg; > + struct nd_intel_unlock_unit cmd; > + } nd_cmd = { > + .pkg = { > + .nd_command = NVDIMM_INTEL_UNLOCK_UNIT, > + .nd_family = NVDIMM_FAMILY_INTEL, > + .nd_size_in = ND_INTEL_PASSPHRASE_SIZE, > + .nd_size_out = ND_INTEL_STATUS_SIZE, > + .nd_fw_size = ND_INTEL_STATUS_SIZE, > + }, > + .cmd = { > + .status = 0, > + }, > + }; > + > + if (!test_bit(NVDIMM_INTEL_UNLOCK_UNIT, _mem->dsm_mask)) > + return -ENOTTY; > + > + memcpy(nd_cmd.cmd.passphrase, nkey->data, > + sizeof(nd_cmd.cmd.passphrase)); > + rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, _cmd, > + sizeof(nd_cmd), _rc); > + if (rc < 0) > + goto out; > + if (cmd_rc < 0) { > + rc = cmd_rc; > + goto out; > + } > + > + switch (nd_cmd.cmd.status) { > + case 0: > + break; > + case ND_INTEL_STATUS_INVALID_PASS: > + rc = -EINVAL; > + goto out; > + case ND_INTEL_STATUS_INVALID_STATE: > + default: > + rc = -ENXIO; > + goto out; > + } > + > + /* > + * TODO: define a cross arch wbinvd when/if NVDIMM_FAMILY_INTEL > + * support arrives on another arch. > + */ > + /* DIMM unlocked, invalidate all CPU caches before we read it */ > + wbinvd_on_all_cpus(); > + > + out: > + return rc; > +} > + > +static int intel_dimm_security_state(struct nvdimm_bus *nvdimm_bus, > + struct nvdimm *nvdimm, enum nvdimm_security_state *state) > +{ > + struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus); > + int cmd_rc, rc = 0; > + struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm); > + struct { > +
Re: Help trying to use /dev/pmem for dax debugging?
On 7/31/2018 12:36 PM, Ross Zwisler wrote: On Mon, Jul 30, 2018 at 07:53:12PM -0400, Theodore Y. Ts'o wrote: In newer kernels, it looks like you can't use /dev/pmem0 for DAX unless it's marked as being DAX capable. This appears to require CONFIG_NVDIMM_PFN. But when I tried to build a kernel with that configured, I get the following BUG: [0.00] Linux version 4.18.0-rc4-xfstests-00031-g7c2d77aa7d80 (tytso@cwcc) (gcc version 7.3.0 (Debian 7.3.0-27)) #460 SMP Mon Jul 30 19:38:44 EDT 2018 [0.00] Command line: systemd.show_status=auto systemd.log_level=crit root=/dev/vda console=ttyS0,115200 cmd=maint fstesttz=America/New_York fstesttyp=ext4 fstestapi=1.4 memmap=4G!9G memmap=9G!14G Hey Ted, You're using the memmap kernel command line parameter to reserve normal memory to be treated as normal memory, but you've also got kernel address randomization turned on in your kernel config: CONFIG_RANDOMIZE_BASE=y CONFIG_RANDOMIZE_MEMORY=y You need to turn these off for the memmap kernel command line parameter, else the memory we're using could overlap with addresses used for other things. I believe this issue was fixed a while back. Although we probably can see if that is the issue or something else. Once that is off you probably want to double check that the addresses you're reserving are marked as 'usable' in the e820 table. Gory details here, sorry for the huge link: https://nvdimm.wiki.kernel.org/how_to_choose_the_correct_memmap_kernel_parameter_for_pmem_on_your_system - Ross ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v4 0/2] ext4: fix DAX dma vs truncate/hole-punch
On Tue, Jul 10, 2018 at 01:10:29PM -0600, Ross Zwisler wrote: > Changes since v3: > * Added an ext4_break_layouts() call to ext4_insert_range() to ensure >that the {ext4,xfs}_break_layouts() calls have the same meaning. >(Dave, Darrick and Jan) > > --- > > This series from Dan: > > https://lists.01.org/pipermail/linux-nvdimm/2018-March/014913.html > > added synchronization between DAX dma and truncate/hole-punch in XFS. > This short series adds analogous support to ext4. > > I've added calls to ext4_break_layouts() everywhere that ext4 removes > blocks from an inode's map. > > The timings in XFS are such that it's difficult to hit this race. Dan > was able to show the race by manually introducing delays in the direct > I/O path. > > For ext4, though, its trivial to hit this race, and a hit will result in > a trigger of this WARN_ON_ONCE() in dax_disassociate_entry(): > > WARN_ON_ONCE(trunc && page_ref_count(page) > 1); > > I've made an xfstest which tests all the paths where we now call > ext4_break_layouts(). Each of the four paths easily hits this race many > times in my test setup with the xfstest. You can find that test here: > > https://lists.01.org/pipermail/linux-nvdimm/2018-June/016435.html > > With these patches applied, I've still seen occasional hits of the above > WARN_ON_ONCE(), which tells me that we still have some work to do. I'll > continue looking at these more rare hits. One last ping on this - do we want to merge this for v4.19? I've tracked down the more rare warnings, and have reported the race I'm seeing here: https://lists.01.org/pipermail/linux-nvdimm/2018-July/017205.html AFAICT the race exists equally for XFS and ext4, and I'm not sure how to solve it easily. Essentially it seems like we need to synchronize not just page faults but calls to get_page() with truncate/hole punch operations, else we can have the reference count for a given DAX page going up and down while we are in the middle of a truncate. I'm not sure if this is even feasible. The equivalent code for this series already exists in XFS, so taking the series now gets ext4 and XFS on the same footing moving forward, so I guess I'm in favor of merging it now, though I can see the argument that it's not a complete solution. Thoughts? ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: Help trying to use /dev/pmem for dax debugging?
On Mon, Jul 30, 2018 at 07:53:12PM -0400, Theodore Y. Ts'o wrote: > In newer kernels, it looks like you can't use /dev/pmem0 for DAX > unless it's marked as being DAX capable. This appears to require > CONFIG_NVDIMM_PFN. But when I tried to build a kernel with that > configured, I get the following BUG: > > [0.00] Linux version 4.18.0-rc4-xfstests-00031-g7c2d77aa7d80 > (tytso@cwcc) (gcc version 7.3.0 (Debian 7.3.0-27)) #460 SMP Mon Jul 30 > 19:38:44 EDT 2018 > [0.00] Command line: systemd.show_status=auto systemd.log_level=crit > root=/dev/vda console=ttyS0,115200 cmd=maint fstesttz=America/New_York > fstesttyp=ext4 fstestapi=1.4 memmap=4G!9G memmap=9G!14G Hey Ted, You're using the memmap kernel command line parameter to reserve normal memory to be treated as normal memory, but you've also got kernel address randomization turned on in your kernel config: CONFIG_RANDOMIZE_BASE=y CONFIG_RANDOMIZE_MEMORY=y You need to turn these off for the memmap kernel command line parameter, else the memory we're using could overlap with addresses used for other things. Once that is off you probably want to double check that the addresses you're reserving are marked as 'usable' in the e820 table. Gory details here, sorry for the huge link: https://nvdimm.wiki.kernel.org/how_to_choose_the_correct_memmap_kernel_parameter_for_pmem_on_your_system - Ross ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v3 0/6] kaddr and pfn can be NULL to ->direct_access()
On 7/30/2018 12:15 AM, Huaisheng Ye wrote: Applied From: Huaisheng Ye Changes since v2 [2]: * Collect Martin and Mike's acks for dcssblk and dm-writecache; * Rebase the series of patch to v4.18-rc7. Changes since v1 [1]: * Involve the previous patches for pfn can be NULL. * Reword the patch descriptions according to Christian's comment. * According to Ross's suggestion, replace local pointer dummy_addr with NULL within md/dm-writecache for direct_access. [1]: https://lkml.org/lkml/2018/7/24/199 [2]: https://lkml.org/lkml/2018/7/25/581 Some functions within fs/dax, dax/super and md/dm-writecache don't need to get local pointer kaddr or variable pfn from direct_access. Assigning NULL to kaddr or pfn with ->direct_access() is more straightforward and simple than offering a useless local pointer or variable. So all ->direct_access() need to check the validity of pointer kaddr and pfn for NULL assignment. If either of them is equal to NULL, that is to say callers may have no need for kaddr or pfn, so this series of patch are prepared for allowing them to pass in NULL instead of having to pass in a local pointer or variable that they then just throw away. Huaisheng Ye (6): libnvdimm, pmem: kaddr and pfn can be NULL to ->direct_access() s390, dcssblk: kaddr and pfn can be NULL to ->direct_access() tools/testing/nvdimm: kaddr and pfn can be NULL to ->direct_access() dax/super: Do not request a pointer kaddr when not required md/dm-writecache: Don't request pointer dummy_addr when not required filesystem-dax: Do not request kaddr and pfn when not required drivers/dax/super.c | 3 +-- drivers/md/dm-writecache.c | 3 +-- drivers/nvdimm/pmem.c | 7 +-- drivers/s390/block/dcssblk.c| 8 +--- fs/dax.c| 13 - tools/testing/nvdimm/pmem-dax.c | 12 6 files changed, 24 insertions(+), 22 deletions(-) ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
[PATCH] device-dax: avoid hang on error before devm_memremap_pages()
dax_pmem_percpu_exit() waits for dax_pmem_percpu_release() to invoke the dax_pmem->cmp completion. Unfortunately this approach to cleaning up the percpu_ref only works after devm_memremap_pages() was successful. If devm_add_action_or_reset() or devm_memremap_pages() fails, dax_pmem_percpu_release() is not invoked. Therefore dax_pmem_percpu_exit() hangs waiting for the completion: rc = devm_add_action_or_reset(dev, dax_pmem_percpu_exit, _pmem->ref); if (rc) return rc; dax_pmem->pgmap.ref = _pmem->ref; addr = devm_memremap_pages(dev, _pmem->pgmap); Avoid the hang by calling percpu_ref_exit() in the error paths instead of going through dax_pmem_percpu_exit(). Signed-off-by: Stefan Hajnoczi --- Found by code inspection. Compile-tested only. --- drivers/dax/pmem.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c index fd49b24fd6af..99e2aace8078 100644 --- a/drivers/dax/pmem.c +++ b/drivers/dax/pmem.c @@ -105,15 +105,19 @@ static int dax_pmem_probe(struct device *dev) if (rc) return rc; - rc = devm_add_action_or_reset(dev, dax_pmem_percpu_exit, - _pmem->ref); - if (rc) + rc = devm_add_action(dev, dax_pmem_percpu_exit, _pmem->ref); + if (rc) { + percpu_ref_exit(_pmem->ref); return rc; + } dax_pmem->pgmap.ref = _pmem->ref; addr = devm_memremap_pages(dev, _pmem->pgmap); - if (IS_ERR(addr)) + if (IS_ERR(addr)) { + devm_remove_action(dev, dax_pmem_percpu_exit, _pmem->ref); + percpu_ref_exit(_pmem->ref); return PTR_ERR(addr); + } rc = devm_add_action_or_reset(dev, dax_pmem_percpu_kill, _pmem->ref); -- 2.17.1 ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm