Re: [PATCH] device-dax: avoid hang on error before devm_memremap_pages()

2018-07-31 Thread Dave Jiang



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

2018-07-31 Thread Dave Jiang



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

2018-07-31 Thread Jiang, Dave



> -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

2018-07-31 Thread Alison Schofield
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?

2018-07-31 Thread Dave Jiang



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

2018-07-31 Thread Ross Zwisler
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?

2018-07-31 Thread Ross Zwisler
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()

2018-07-31 Thread Dave Jiang

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()

2018-07-31 Thread Stefan Hajnoczi
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