[Question: devm_kfree] When should devm_kfree() be used?

[Question: devm_kfree] When should devm_kfree() be used?

[Gen Zhang]

devm_kmalloc() is used to allocate memory for a driver dev. Comments
above the definition and doc 
(https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
imply that allocated the memory is automatically freed on driver attach,
no matter allocation fail or not. However, I examined the code, and
there are many sites that devm_kfree() is used to free devm_kmalloc().
e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
So I am totally confused about this issue. Can anybody give me some
guidance? When should we use devm_kfree()?

Thanks

Re: [Question: devm_kfree] When should devm_kfree() be used?

[Greg KH]

On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
> devm_kmalloc() is used to allocate memory for a driver dev. Comments
> above the definition and doc 
> (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
> imply that allocated the memory is automatically freed on driver attach,
> no matter allocation fail or not. However, I examined the code, and
> there are many sites that devm_kfree() is used to free devm_kmalloc().
> e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
> So I am totally confused about this issue. Can anybody give me some
> guidance? When should we use devm_kfree()?

If you "know" you need to free the memory now, call devm_kfree().  If
you want to wait for it to be cleaned up latter, like normal, then do
not call it.

Do you have a driver that you think uses it incorrectly?

thanks,

greg k-h

Re: [Question: devm_kfree] When should devm_kfree() be used?

[Gen Zhang]

On Tue, May 28, 2019 at 08:49:49AM +0200, Greg KH wrote:
> On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
> > devm_kmalloc() is used to allocate memory for a driver dev. Comments
> > above the definition and doc 
> > (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
> > imply that allocated the memory is automatically freed on driver attach,
> > no matter allocation fail or not. However, I examined the code, and
> > there are many sites that devm_kfree() is used to free devm_kmalloc().
> > e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
> > So I am totally confused about this issue. Can anybody give me some
> > guidance? When should we use devm_kfree()?
> 
> If you "know" you need to free the memory now, call devm_kfree().  If
> you want to wait for it to be cleaned up latter, like normal, then do
> not call it.
> 
> Do you have a driver that you think uses it incorrectly?
> 
> thanks,
> 
> greg k-h
Yes, that is my question! I do have several patches about devm_kfree() 
and debate with maintainers. e.g. you can kindly refer to this lkml:
https://lkml.org/lkml/2019/5/23/1547. 

In tegra_wm9712_driver_probe(), 'machine->codec' is allocated by
platform_device_alloc(). When it is NULL, function returns ENOMEM.
However, 'machine' is allocated by devm_kzalloc() before this site.
Thus we should free 'machine' before function ends to prevent memory
leaking.

And as mentioned above, e.g. hisi_sas_debugfs_init() in 
drivers/scsi/hisi_sas/hisi_sas_main.c, devm_kfree() is used to free 
devm_kmalloc().

So, thanks for looking into this, Greg.

Thanks
Gen

Re: [Question: devm_kfree] When should devm_kfree() be used?

[Greg KH]

On Tue, May 28, 2019 at 03:14:00PM +0800, Gen Zhang wrote:
> On Tue, May 28, 2019 at 08:49:49AM +0200, Greg KH wrote:
> > On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
> > > devm_kmalloc() is used to allocate memory for a driver dev. Comments
> > > above the definition and doc 
> > > (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
> > > imply that allocated the memory is automatically freed on driver attach,
> > > no matter allocation fail or not. However, I examined the code, and
> > > there are many sites that devm_kfree() is used to free devm_kmalloc().
> > > e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
> > > So I am totally confused about this issue. Can anybody give me some
> > > guidance? When should we use devm_kfree()?
> > 
> > If you "know" you need to free the memory now, call devm_kfree().  If
> > you want to wait for it to be cleaned up latter, like normal, then do
> > not call it.
> > 
> > Do you have a driver that you think uses it incorrectly?
> > 
> > thanks,
> > 
> > greg k-h
> Yes, that is my question! I do have several patches about devm_kfree() 
> and debate with maintainers. e.g. you can kindly refer to this lkml:
> https://lkml.org/lkml/2019/5/23/1547. 
> 
> In tegra_wm9712_driver_probe(), 'machine->codec' is allocated by
> platform_device_alloc(). When it is NULL, function returns ENOMEM.
> However, 'machine' is allocated by devm_kzalloc() before this site.
> Thus we should free 'machine' before function ends to prevent memory
> leaking.

No, you are not leaking any memory if you do not call that function.
Try it and see :)

The function is there if you just want to "free the memory now!", it's
not necessary if you return an error as when the device is removed the
memory will be freed then (or if probe fails, depending on the code
path.)

thanks,

greg k-h

Re: [Question: devm_kfree] When should devm_kfree() be used?

[Gen Zhang]

On Tue, May 28, 2019 at 02:41:38PM +0200, Greg KH wrote:
> No, you are not leaking any memory if you do not call that function.
> Try it and see :)
> 
> The function is there if you just want to "free the memory now!", it's
> not necessary if you return an error as when the device is removed the
> memory will be freed then (or if probe fails, depending on the code
> path.)
> 
> thanks,
> 
> greg k-h
Thanks for your patient replies, Greg. I thinks I am getting to know 
this issue.

Thanks
Gen