On Mon, Jan 21, 2019 at 03:51:02PM +0800, Wei Yang wrote:
>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.
>>+ *
>>+ * 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.
>>+ */
>
>Let me reply in this thread. Sorry for my poor understand, I don't get it
>clearly now.
>
>To be honest, the structure is a little bit complicated, if my understanding
>is not correct, please forgive my poor understand.
>
>Below is a code flow. To simply analysis, I setup kernel parameter memmap to
>emulate, and configure one namespace to mode devdax. So that we would have the
>same root for code flow.
>
>Let's start with nd_region_driver:
>
> nd_region_probe
> nd_region_register_namespaces
> create_namespaces
> nd_region->btt_seed = nd_btt_create(nd_region);
> nd_region->pfn_seed = nd_pfn_create(nd_region);
> nd_region->dax_seed = nd_dax_create(nd_region);
May I ask a question about the purpose to create these three device here?
I see nd_pfn_create() doesn't allocate pfn_sb here, and the probe on these
devices failed. Confused why we need these three devices.
--
Wei Yang
Help you, Help me
