[alsa-devel] [PATCH 03/15] ASoC: hdac_hdmi - Use list to add pins and converters

[Takashi Iwai]

On Tue, 01 Dec 2015 19:52:14 +0100,
Takashi Iwai wrote:
> 
> On Tue, 01 Dec 2015 23:50:50 +0100,
> Subhransu S. Prusty wrote:
> > 
> > On Tue, Dec 01, 2015 at 01:47:04PM +0100, Takashi Iwai wrote:
> > > On Tue, 01 Dec 2015 18:46:59 +0100,
> > > Subhransu S. Prusty wrote:
> > > > 
> > > > +static int hdac_hdmi_add_cvt(struct hdac_ext_device *edev, hda_nid_t nid)
> > > > +{
> > > > +	struct hdac_hdmi_priv *hdmi = edev->private_data;
> > > > +	struct hdac_hdmi_cvt *cvt;
> > > > +
> > > > +	cvt = devm_kzalloc(&edev->hdac.dev, sizeof(*cvt), GFP_KERNEL);
> > > 
> > > Be careful about devm_*() usage here.  The devres resources may be
> > > freed before the release callback is called for the assigned device
> > > object.  See drivers/base/core.c:device_release().
> > > 
> > > For the top-level object, usually it's OK, because the top-level
> > > object itself won't be released but only the driver is detached.
> > > Then devres_release_all() is called in device_release_driver() while
> > > the device itself is still present.
> > 
> > Not sure if I understood it correctly. But as the devres_release_all is
> > called after the driver remove and the above resource is not expected to
> > be used outside the scope of driver.
> 
> devres_release_all() is called *before* calling the device's release
> callback, which is usually triggered by put_device().  So it depends
> on how you call the device removal whether it's safe or not.
> 
> > So should't deleting the list entries
> > be good enough during driver remove?
> 
> It depends on how the allocated data is accessed.  I guess it won't be
> a problem in this case, but in general, you have to be careful about
> devm_*() usage.  It can't be used always blindly.

To make clear my intention: the code used in this patch appears OK, as
far as I see.  But, for example, if this code would be moved to HDA
core layer, then it might not work -- there you'll have to release the
resource in another way.  A driver-bound resource shall be released
with the driver's unbinding, but a resource bound with the device
isn't necessarily released there but first at device_release().


Takashi