linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Federico Vaga <federico.vaga@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Alessandro Rubini <rubini@gnudd.com>
Subject: Re: drivers/base/core.c: about device_find_child() function
Date: Thu, 11 Apr 2013 06:48:44 -0700	[thread overview]
Message-ID: <20130411134844.GA3489@kroah.com> (raw)
In-Reply-To: <3798489.yISokZugC2@harkonnen>

On Thu, Apr 11, 2013 at 01:52:36PM +0200, Federico Vaga wrote:
> Hello,
> 
> I'm using the function device_find_child() [drivers/base/core.c] to retrieve 
> a specific child of a device. I see that this function invokes 
> get_device(child) when a child matches. I think that this function must 
> return the reference to the child device without getting it.

No, it should not, otherwise the device could "disappear" at any moment,
and the caller who had the handle, would now have a stale pointer.

> The function's comment does not explicitly talk about an increment of the 
> refcount of the device. So, "man 9 device_find_child" and various derivative 
> webpages do not talk about this. The developer is not correctly informed 
> about this function, unless (s)he looks at the source code.

You are right, I would gladly take a patch adding that comment to the
function, can you send me one?

> I see that users of this function, usually, immediately do put_device() after 
> the call to device_find_child(), so it is not expected that a 
> device_find_child() does a get_device() on the found child.
> 
> 
>    Immediately does put_device():
>      drivers/firewire/core-device.c
>      drivers/rpmsg/virtio_rpmsg_bus.c
>      drivers/s390/kvm/kvm_virtio.c

That's correct.

>    They effectively need a get_device():
>      drivers/net/bluetooth/hci_sysfs.c
>      drivers/net/dsa/dsa.c

Please fix these.

>    Maybe bugged because they do not do put_device():
>      drivers/media/platform/s5p-mfc/s5p_mfc.c
>      drivers/tty/serial/serial_core.c
>    Probably I'm wrong on this and I do not find the associated put_device()

I think the serial core is correct, but I'll audit it, the media one
should be fixed as well.

thanks,

greg k-h

  reply	other threads:[~2013-04-11 13:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-11 11:52 drivers/base/core.c: about device_find_child() function Federico Vaga
2013-04-11 13:48 ` Greg Kroah-Hartman [this message]
2013-04-12 12:09   ` Federico Vaga
2013-04-12 22:06     ` Greg Kroah-Hartman
2013-04-15  8:55       ` Federico Vaga
2013-04-13  7:59     ` Lars-Peter Clausen
2013-04-15  8:51       ` Federico Vaga

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=20130411134844.GA3489@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=cornelia.huck@de.ibm.com \
    --cc=federico.vaga@gmail.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rubini@gnudd.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).