Re: [PATCH] libnvdimm: Clarify nd_pfn_init() flow

Wei Yang
2019-01-21T20:57:19Z

On Mon, Jan 21, 2019 at 10:04:40AM -0800, Dan Williams wrote:
>On Sun, Jan 20, 2019 at 11:51 PM Wei Yang <richardw.yang@linux.intel.com> wrote:
>> On Fri, Jan 18, 2019 at 04:47:23PM -0800, Dan Williams wrote:
>[..]
>> Also, I have one confusion about your saying: two probes.
>>
>> If the two probes are:
>>
>> * for dax%d.%d: 1. nd_dax_probe 2. dax_pmem_probe
>> * for pfn%d.%d: 1. nd_pfn_probe 2. nd_pmem_probe
>>
>> Then, if the first probe fails, the device itself would be destroyed. How the
>> second probe do its job?
>>
>> > rc = nd_pfn_validate(nd_pfn, sig);
>> > if (rc != -ENODEV)
>> > return rc;
>
>Here is an example path for a device-dax instance:
>
> /sys/devices/platform/e820_pmem/ndbus0/region0/dax0.1/dax0.0
>
>In this case the order of events is:
>
>1/ region0 discovers it contains a pmem namespace and registers namespace0.0
>2/ The pmem namespace driver calls nd_dax_probe() to check for the
>presence of a device-dax configuration
>3/ If present, nd_pfn_validate() returns 0 and nd_dax_probe()
>registers the dax0.1 device (this is a libnvdimm 'personality device).
>4/ When nd_pmem_probe() sees nd_dax_probe() return 0 it in turn fails
>the probe of namespace0.0 with -ENXIO. All devm allocations during the
>probe of namespace0.0 are released.

I may have another opinion here.

The probe return error means the device will not attach to this driver.
But the device itself it not released.

We allocate devm on one device and those memory will be released when
the device is destroyed. If I am correct.

This means at this point, pfn_sb's memory still exists in the system.
Even finally we will release it, when namespace0.0 is destroyed.

>5/ Meanwhile the created dax0.1 device is recognized by the dax_pmem device.
>6/ dax_pmem_probe() uses nd_pfn_init() to validate the configuration
>and ultimately creates the dax0.0 character device
>
>The pmem namepsace driver is effectively a router to other personality
>drivers that may be layered on top of the base namespace capacity.