Re: [PATCH v4 4/9] dax: fix mapping lifetime handling, convert to __pfn_t + kmap_atomic_pfn_t()

[Dan Williams <dan.j.williams@intel.com>]

On Mon, Jun 8, 2015 at 9:29 AM, Elliott, Robert (Server Storage)
<Elliott@hp.com> wrote:
>> -----Original Message-----
>> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf
>> Of Dan Williams
>> Sent: Friday, June 05, 2015 3:19 PM
>> Subject: [PATCH v4 4/9] dax: fix mapping lifetime handling, convert to
>> __pfn_t + kmap_atomic_pfn_t()
> ...
>> diff --git a/arch/powerpc/sysdev/axonram.c
>> b/arch/powerpc/sysdev/axonram.c
>> index e8657d3bc588..20725006592e 100644
>> --- a/arch/powerpc/sysdev/axonram.c
>> +++ b/arch/powerpc/sysdev/axonram.c
> ...
>> @@ -165,9 +166,13 @@ static int axon_ram_probe(struct platform_device
>> *device)
>>  {
>>       static int axon_ram_bank_id = -1;
>>       struct axon_ram_bank *bank;
>> -     struct resource resource;
>> +     struct resource *resource;
>>       int rc = 0;
>>
>> +     resource = devm_kzalloc(&device->dev, sizeof(*resource),
>> GFP_KERNEL);
>> +     if (!resource)
>> +             return -ENOMEM;
>> +
>
> Since resource is now a pointer...
>
> ...
>> @@ -184,13 +189,13 @@ static int axon_ram_probe(struct platform_device
>> *device)
>>
>>       bank->device = device;
>>
>> -     if (of_address_to_resource(device->dev.of_node, 0, &resource) != 0)
>> {
>> +     if (of_address_to_resource(device->dev.of_node, 0, resource) != 0) {
>>               dev_err(&device->dev, "Cannot access device tree\n");
>>               rc = -EFAULT;
>>               goto failed;
>>       }
> ...
>
> ... I'd expect to see a devm_kfree call added after the failed label,
> like was  done here:

No, because unlike dccsblk the axon_ram driver actually adheres to the
device model, so it will be auto-freed on failed probe.  The dcssblk
implementation is not a proper "driver" so we don't get the
devres_release_all() until the device is unregistered.  That causes
the "resource" object to stick around longer than necessary unless we
force devm_kfree() it.