All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Gen Zhang <blackgod016574@gmail.com>
Cc: rafael@kernel.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: Re: [Question: devm_kfree] When should devm_kfree() be used?
Date: Tue, 28 May 2019 14:41:38 +0200	[thread overview]
Message-ID: <20190528124138.GA3578@kroah.com> (raw)
In-Reply-To: <20190528071400.GB18498@zhanggen-UX430UQ>

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

  reply	other threads:[~2019-05-28 12:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-28  0:32 [Question: devm_kfree] When should devm_kfree() be used? Gen Zhang
2019-05-28  6:49 ` Greg KH
2019-05-28  7:14   ` Gen Zhang
2019-05-28 12:41     ` Greg KH [this message]
2019-05-28 12:47       ` Gen Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190528124138.GA3578@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=blackgod016574@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.