Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: hdac_hdmi: Set ops to NULL on remove

[Amadeusz SÅawiÅski]

On Thu, 20 Jun 2019 08:17:33 +0200
Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:

> >>>>> Could you please give a bit more context on what error you see
> >>>>> when this happens?  
> >>>>
> >>>> Hi,
> >>>>
> >>>> I get Oops. This is what happens with all other patches in this
> >>>> series and only this one reverted:
> >>>>
> >>>> root@APL:~# rmmod snd_soc_sst_bxt_rt298
> >>>> root@APL:~# rmmod snd_soc_hdac_hdmi
> >>>> root@APL:~# rmmod snd_soc_skl  
> >>>
> >>> Thanks, Amadeusz. I think the order in which the drivers are
> >>> removed
> >>> is what's causing the oops in your case. With SOF, the order we
> >>> remove is
> >>>
> >>> 1. rmmod sof_pci_dev
> >>> 2. rmmod snd_soc_sst_bxt_rt298
> >>> 3. rmmod snd_soc_hdac_hdmi
> >>>  
> >>
> >> Well, there is nothing enforcing the order in which modules can be
> >> unloaded (and I see no reason to force it), as you can see from
> >> following excerpt, you can either start unloading from
> >> snd_soc_sst_bxt_rt298 or snd_soc_skl, and yes if you start from
> >> snd_soc_skl, there is no problem.  
> 
> there is a fundamental dependency that you are ignoring: the module 
> snd_soc_sst_bxt_rt298 is a machine driver which will be probed when 
> snd_soc_skl creates a platform_device.
> Sure you can remove modules in a different order, but that's a bit of
> an artificial/academic exercise isn't it?
> 
> >>  
> > I am good with this patch. I just wanted to understand why we werent
> > seeing this error with SOF. Sure, there's nothing enforcing the
> > order in which modules are unloaded  but there must be a logical
> > order for testing purposes.
> > 
> > Pierre, can you please comment on it. I vaguely remember discussing
> > this with you last year.  
> 
> Our tests remove the modules by taking care of dependencies and it's 
> already unveiled dozens of issues.
> We could add a sequence similar to Amadeusz and unbind the modules
> which are loaded with the creation of a platform_device (machine
> driver, dmic), I am just not sure how of useful this would be.

You work under the assumption that users will remove modules in
"correct" order. Because it is not enforced by modules dependencies you
can expect users to do everything possible at some point in time. In
this case unloading modules in not expected order will lead to kernel
Oops, which is not what should happen.