public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Federico Vaga <federico.vaga@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alessandro Rubini <rubini@gnudd.com>
Subject: drivers/base/core.c: about device_find_child() function
Date: Thu, 11 Apr 2013 13:52:36 +0200	[thread overview]
Message-ID: <3798489.yISokZugC2@harkonnen> (raw)

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.

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.

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

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

   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 should propose the following solution:

* Deprecate the device_find_child() function

* Create the following functions

   struct device *device_search_child(struct device *parent, void *data,
            int (*match)(struct device *dev, void *data))
   {
        struct klist_iter i;
        struct device *child;

        if (!parent)
                return NULL;

        klist_iter_init(&parent->p->klist_children, &i);
        while ((child = next_device(&i)))
                if (match(child, data))
                        break;
        klist_iter_exit(&i);
        return child;
  }

  struct device *get_device_child(struct device *parent, void *data,
            int (*match)(struct device *dev, void *data))
  {
        struct device *child;

        child = device_search_child(parent, data, match);
        if (child)
              get_device(child);
        return child;
  }


In this way, when a driver needs to find and get a child, it uses 
get_device_child() and , when it finishes its duty, it uses put_device(). In 
this situation, the developer use a pair of function with a symmetric names: 
get_device_child() and put_device().

If the driver do not need to get_device() on a child device, it simply does a 
device_search_child() to retrieve a pointer.

-- 
Federico Vaga

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

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-11 11:52 Federico Vaga [this message]
2013-04-11 13:48 ` drivers/base/core.c: about device_find_child() function Greg Kroah-Hartman
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=3798489.yISokZugC2@harkonnen \
    --to=federico.vaga@gmail.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=gregkh@linuxfoundation.org \
    --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