Re: [PATCH v3 2/2] PCI: qcom: Add support for modular builds

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Oct 18, 2022 at 05:10:53PM +0200, Johan Hovold wrote:
> On Mon, Oct 17, 2022 at 12:34:22PM -0500, Bjorn Helgaas wrote:
> > On Mon, Oct 17, 2022 at 01:47:05PM +0200, Johan Hovold wrote:
> > > Allow the Qualcomm PCIe controller driver to be built as a module, which
> > > is useful for multi-platform kernels as well as during development.
> > 
> > There are two different goals here, and there's no real reason to
> > bundle them together:
> > 
> >   1) Make qcom a loadable module.  This is a hard requirement so
> >      multi-platform kernels don't need to build in all drivers
> >      statically.
> > 
> >   2) Make qcom unloadable.  This is a high want, possibly even a
> >      requirement for developers, but is not really a big issue for
> >      users.
> > 
> > There are different changes required: 1) requires the Kconfig change;
> > 2) requires .remove() to be implemented.  Since there's no requirement
> > that these be done together, let's split them into separate patches.
> > 
> > Then we can make sure that at least 1) gets done, and if for any
> > reason 2) isn't safe or breaks something, we can at least bisect and
> > if necessary revert it without losing 1).
> 
> Implementing 1) in itself requires more than simply splitting this
> patch. And I don't think we should be making life harder for developers,
> as well as users assisting during debugging, by going in that direction.

If you're saying this patch can't be split, can you elaborate on the
details of *why* it can't be split?

> We have tons of modules in the kernel and very few that cannot be
> unloaded. Anyone who doesn't trust root to not unload modules can
> always disable unloading completely using CONFIG_MODULE_UNLOAD.

This is all true, but IIUC, the issue is about unloading IRQ
controller drivers, and this doesn't address that.  I don't have a
clear understanding of the issue, and it would be nice if a patch that
specifically added unloadability could elaborate on that.  Then we can
decide that "yes, this is a risk, and we're willing to accept it." An
argument that "tons of modules do this" totally avoids the issues of
this particular case.

Bjorn