On Wed, Dec 02, 2015 at 10:31:07AM +0100, Takashi Iwai wrote:
> 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().
It's better then I will remove the devm_xxx and will manage the resources in
the driver itself.
Regards,
Subhransu
>
>
> Takashi
--
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
