From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ranjani Sridharan Subject: Re: [PATCH] ASoC: hda: increment codec device refcount when it is added to the card Date: Fri, 31 May 2019 06:52:27 -0700 Message-ID: <0f75caffee97f7cec5a1bd31e9f3eb3ea1a12948.camel@linux.intel.com> References: <20190530201828.2648-1-ranjani.sridharan@linux.intel.com> <684fe069-d2fb-f716-bd3e-67f0c7a52de0@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id B4FAAF80C1B for ; Fri, 31 May 2019 15:52:30 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" To: Takashi Iwai Cc: alsa-devel@alsa-project.org, broonie@kernel.org, Pierre-Louis Bossart List-Id: alsa-devel@alsa-project.org 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