On Fri, Jan 18, 2019 at 04:47:23PM -0800, Dan Williams wrote:
>In recent days, 2 engineers, including the original author of
>nd_pfn_init(), overlooked the internal call to nd_pfn_validate() and the
>implications to memory allocation.
>
>Clarify this situation to help anyone that reads through this code in
>the future.
>
>Reported-by: Wei Yang <richardw.y...@linux.intel.com>
>Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
>---
> drivers/nvdimm/btt_devs.c | 5 +++++
> drivers/nvdimm/dax_devs.c | 5 +++++
> drivers/nvdimm/pfn_devs.c | 21 +++++++++++++++++++++
> 3 files changed, 31 insertions(+)
>
>diff --git a/drivers/nvdimm/btt_devs.c b/drivers/nvdimm/btt_devs.c
>index 795ad4ff35ca..e0a6f2491e57 100644
>--- a/drivers/nvdimm/btt_devs.c
>+++ b/drivers/nvdimm/btt_devs.c
>@@ -354,6 +354,11 @@ int nd_btt_probe(struct device *dev, struct
>nd_namespace_common *ndns)
> put_device(btt_dev);
> }
>
>+ /*
>+ * Successful probe indicates to the caller that an nd_btt
>+ * personality device has been registered and the caller can
>+ * fail the probe of the baseline namespace device.
>+ */
> return rc;
> }
> EXPORT_SYMBOL(nd_btt_probe);
>diff --git a/drivers/nvdimm/dax_devs.c b/drivers/nvdimm/dax_devs.c
>index 0453f49dc708..65010d955fa7 100644
>--- a/drivers/nvdimm/dax_devs.c
>+++ b/drivers/nvdimm/dax_devs.c
>@@ -136,6 +136,11 @@ int nd_dax_probe(struct device *dev, struct
>nd_namespace_common *ndns)
> } else
> __nd_device_register(dax_dev);
>
>+ /*
>+ * Successful probe indicates to the caller that a device-dax
>+ * personality device has been registered and the caller can
>+ * fail the probe of the baseline namespace device.
>+ */
> return rc;
> }
> EXPORT_SYMBOL(nd_dax_probe);
>diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
>index 6f22272e8d80..a8783b5a76ba 100644
>--- a/drivers/nvdimm/pfn_devs.c
>+++ b/drivers/nvdimm/pfn_devs.c
>@@ -576,6 +576,11 @@ int nd_pfn_probe(struct device *dev, struct
>nd_namespace_common *ndns)
> } else
> __nd_device_register(pfn_dev);
>
>+ /*
>+ * Successful probe indicates to the caller that an nd_pfn
>+ * personality device has been registered and the caller can
>+ * fail the probe of the baseline namespace device.
>+ */
> return rc;
> }
> EXPORT_SYMBOL(nd_pfn_probe);
>@@ -706,6 +711,22 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
> sig = DAX_SIG;
> else
> sig = PFN_SIG;
>+
>+ /*
>+ * Check for an existing 'pfn' superblock before writing a new
>+ * one. The intended flow is that on the first probe of an
>+ * nd_{pfn,dax} device the superblock is calculated and written
>+ * to the namespace. In this case nd_pfn_validate() returns
>+ * -ENODEV because no valid superblock exists currently.
As you replied in following mail:
3/ If present, nd_pfn_validate() returns 0 and nd_dax_probe()
registers the dax0.1 device (this is a libnvdimm 'personality device).
So at this point, nd_pfn_validate() return 0 or -ENODEV?
>+ *
>+ * On subsequent probes nd_pfn_validate() will find a valid
>+ * superblock and return 0.
>+ *
>+ * If an assumption of the superblock has been violated, like a
>+ * change to the physical alignment of the namespace,
>+ * nd_pfn_validate() will return an error other than -ENODEV to
>+ * fail probing.
>+ */
> rc = nd_pfn_validate(nd_pfn, sig);
> if (rc != -ENODEV)
> return rc;
--
Wei Yang
Help you, Help me
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
