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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
