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

From: Bjorn Helgaas
Date: Tue Oct 18 2022 - 12:09:11 EST


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