[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221018160859.GA3805344@bhelgaas>
Date: Tue, 18 Oct 2022 11:08:59 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Johan Hovold <johan@...nel.org>
Cc: Johan Hovold <johan+linaro@...nel.org>,
Stanimir Varbanov <svarbanov@...sol.com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...ainline.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof WilczyĆski <kw@...ux.com>,
Manivannan Sadhasivam <mani@...nel.org>,
linux-arm-msm@...r.kernel.org, linux-pci@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Rob Herring <robh@...nel.org>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Subject: Re: [PATCH v3 2/2] PCI: qcom: Add support for modular builds
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
Powered by blists - more mailing lists