Re: [PATCH] ASoC: hda: increment codec device refcount when it is added to the card

[Ranjani Sridharan <ranjani.sridharan@linux.intel.com>]

On Fri, 2019-05-31 at 15:25 +0200, Takashi Iwai wrote:
> On Fri, 31 May 2019 15:18:03 +0200,
> Ranjani Sridharan wrote:
> > 
> > On Fri, 2019-05-31 at 08:11 +0200, Takashi Iwai wrote:
> > > On Thu, 30 May 2019 23:00:10 +0200,
> > > Pierre-Louis Bossart wrote:
> > > > 
> > > > 
> > > > 
> > > > On 5/30/19 3:18 PM, Ranjani Sridharan wrote:
> > > > > Calling snd_device_new() makes the codec devices managed by
> > > > > the
> > > > > card.
> > > > > So, when the card is removed, the refcount for the codec
> > > > > device is decremented and results in the codec device's
> > > > > kobject
> > > > > being cleaned up if the refcount is 0. But, this leads to a
> > > > > NULL
> > > > > pointer exception while attempting to remove the symlinks
> > > > > when
> > > > > the
> > > > > codec driver is released later on. Therefore, increment the
> > > > > codec
> > > > > device's refcount before adding it to the card to prevent
> > > > > this.
> > > > 
> > > > Ranjani, you should add a bit of context for the rest of the
> > > > list...
> > > > 
> > > > This patch suggest a solution to a set of sightings occurring
> > > > when
> > > > removing/adding modules in a loop, and the current analysis
> > > > points
> > > > to
> > > > a difference between the way the HDMI and HDaudio codecs are
> > > > handled.
> > > > 
> > > > https://github.com/thesofproject/linux/issues/981
> > > > https://github.com/thesofproject/linux/issues/966
> > > > https://github.com/thesofproject/linux/pull/988
> > > > 
> > > > Since it's not SOF specific it's better to get feedback
> > > > directly
> > > > from
> > > > the large ALSA community/maintainers. We probably want to focus
> > > > on
> > > > the
> > > > platform-specific/vendor-specific stuff on GitHub and use the
> > > > mailing
> > > > list for such framework-level changes.
> > > 
> > > Hm, I still wonder why this doens't happen with the HDA legacy.
> > > 
> > > What is the shortest way to trigger the bug manually without a
> > > script?
> > 
> > Hi Takashi,
> > 
> > With SOF, I can reproduce the issue if I just unload the
> > sof_pci_dev
> > module with rmmod. 
> > 
> > Basically, the remove routine for the SOF pci device, unregisters
> > the
> > machine driver and then removes the codec device. So the first step
> > of
> > unregistering the machine driver frees the card which decrements
> > the
> > refcount for the HDA codec's kobject. In the case of HDMI codec,
> > since
> > it is not managed by the card, the refcount is not decremented when
> > the
> > card is removed. 
> 
> So it's only about hdac_hdmi codec, or only about hdac_hda codec?

It is only about the hdac_hda codec. 
> 
> And why HDMI codec isn't managed by the card...?  IOW, isn't it
> dangerous -- it means the codec being always removable after bound to
> the card?
That is a good point. Probably this needs to be fixed as well. I can
include the change for that if you think it is the right thing to do.

But I was wondering if it makes sense to increment the refcount when
the device is added to the card with snd_device_new()? 
I'm not sure how it affects the other devices so didnt go down this
route.

Thanks,
Ranjani

> 
> 
> thanks,
> 
> Takashi