Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758239AbYAKCd3 (ORCPT ); Thu, 10 Jan 2008 21:33:29 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755563AbYAKCdT (ORCPT ); Thu, 10 Jan 2008 21:33:19 -0500 Received: from hs-out-0708.google.com ([64.233.178.240]:25901 "EHLO hs-out-2122.google.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754736AbYAKCdS (ORCPT ); Thu, 10 Jan 2008 21:33:18 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=dEiLajpYNf9KWAJDn3gGtmQPrwelqvqpD9bHLF8Z4ArPdvVCZK8wKKFcig4rE9Bdg65Tz/k5k9Xyt04YG6oOzKFO+lnbyQThiyALyP4Vh5yStu0/myxnqGscX48o2ffecsZBepMXplb049oVpSyNW9cRrghtLtI563uEYlU1L3w= Message-ID: Date: Fri, 11 Jan 2008 10:33:16 +0800 From: "Dave Young" To: "Cornelia Huck" Subject: Re: [PATCH 0/7] convert semaphore to mutex in struct class Cc: "Greg KH" , "Stefan Richter" , James.Bottomley@hansenpartnership.com, linux-scsi@vger.kernel.org, a.zummo@towertech.it, peterz@infradead.org, cbou@mail.ru, linux-kernel@vger.kernel.org, "David Brownell" , krh@redhat.com, stern@rowland.harvard.edu, rtc-linux@googlegroups.com, spi-devel-general@lists.sourceforge.net, linux1394-devel@lists.sourceforge.net, dwmw2@infradead.org, davem@davemloft.net, jarkao2@gmail.com In-Reply-To: <20080110142341.71af4dc5@gondolin.boeblingen.de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <478227D5.8050402@s5r6.in-berlin.de> <20080107172009.GA25943@suse.de> <20080108224837.GA19623@suse.de> <20080109061316.GA3249@darkstar.te-china.tietoenator.com> <20080110094843.GA3014@darkstar.te-china.tietoenator.com> <20080110142341.71af4dc5@gondolin.boeblingen.de.ibm.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3557 Lines: 121 On Jan 10, 2008 9:23 PM, Cornelia Huck wrote: > On Thu, 10 Jan 2008 17:48:43 +0800, > Dave Young wrote: > > Please add a kerneldoc comment for each of the new interfaces. Will do. > > > +int class_for_each_device(struct class *class, void *data, > > + int (*fn)(struct device *, void *)) > > +{ > > + struct device *dev; > > + int error = 0; > > + > > + if (!class) > > + return -EINVAL; > > + mutex_lock(&class->mutex); > > + list_for_each_entry(dev, &class->devices, node) { > > + error = fn(dev, data); > > Hm, the equivalent _for_each_device() functions all elevate the > device's refcount while calling fn(). I wonder whether we want this > here as well? Thanks for comment. Hm, I'm not sure about this. Greg, what's your opinion? > > > + if (error) > > + break; > > + } > > + mutex_unlock(&class->mutex); > > + > > + return error; > > +} > > +EXPORT_SYMBOL_GPL(class_for_each_device); > > + > > +struct device *class_find_device(struct class *class, void *data, > > + int (*match)(struct device *, void *)) > > +{ > > + struct device *dev; > > + > > + if (!class) > > + return NULL; > > + > > + mutex_lock(&class->mutex); > > + list_for_each_entry(dev, &class->devices, node) > > + if (match(dev, data) && get_device(dev)) > > First get the reference and then drop it if the match function returns > 0 to make this analogous to the other _find_device() functions? It's just like other _find_device() functions. Are these more get/put really needed? > > > + break; > > + mutex_unlock(&class->mutex); > > + > > + return dev; > > +} > > +EXPORT_SYMBOL_GPL(class_find_device); > > + > > +int class_for_each_child(struct class *class, void *data, > > + int (*fn)(struct class_device *, void *)) > > Haven't looked at the callers, but isn't it better to convert them to > use struct device instead so we don't need to introduce new > class_device api? The drivers/scsi/hosts.c need it. > > > +{ > > + struct class_device *dev; > > + int error = 0; > > + > > + if (!class) > > + return -EINVAL; > > + mutex_lock(&class->mutex); > > + list_for_each_entry(dev, &class->children, node) { > > + error = fn(dev, data); > > Same comment as above concerning reference counts. > > > + if (error) > > + break; > > + } > > + mutex_unlock(&class->mutex); > > + > > + return error; > > +} > > +EXPORT_SYMBOL_GPL(class_for_each_child); > > + > > +struct class_device *class_find_child(struct class *class, void *data, > > + int (*match)(struct class_device *, void *)) > > +{ > > + struct class_device *dev; > > + > > + if (!class) > > + return NULL; > > + > > + mutex_lock(&class->mutex); > > + list_for_each_entry(dev, &class->children, node) > > + if (match(dev, data) && class_device_get(dev)) > > And here. > > > > + break; > > + mutex_unlock(&class->mutex); > > + > > + return dev; > > +} > > +EXPORT_SYMBOL_GPL(class_find_child); > > + > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/