Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 24 Jul 2022 10:38:06 +0100
Message-ID: <877d42df5t.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Pali =?UTF-8?B?Um9ow6Fy?= <pali@kernel.org>,
        Johan Hovold <johan+linaro@kernel.org>,
        Kishon Vijay Abraham I <kishon@ti.com>,
        Xiaowei Song <songxiaowei@hisilicon.com>,
        Binghui Wang <wangbinghui@hisilicon.com>,
        Thierry Reding <thierry.reding@gmail.com>,
        Ryder Lee <ryder.lee@mediatek.com>,
        Jianjun Wang <jianjun.wang@mediatek.com>,
        linux-pci@vger.kernel.org,
        Krzysztof =?UTF-8?B?V2lsY3p5xYRza2k=?= <kw@linux.com>,
        Ley Foon Tan <ley.foon.tan@intel.com>,
        linux-kernel@vger.kernel.org
Subject: Re: Why set .suppress_bind_attrs even though .remove() implemented?
In-Reply-To: <20220722171706.GA1911557@bhelgaas>
References: <87k085xekg.wl-maz@kernel.org>
        <20220722171706.GA1911557@bhelgaas>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Fri, 22 Jul 2022 18:17:06 +0100,
Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> On Fri, Jul 22, 2022 at 06:06:07PM +0100, Marc Zyngier wrote:
> > On Fri, 22 Jul 2022 15:39:05 +0100,
> > Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > 
> > > [+cc Marc, can you clarify when we need irq_dispose_mapping()?]
> > 
> > In general, interrupt controllers should not have to discard mappings
> > themselves, just like they rarely create mappings themselves. That's
> > usually a different layer that has created it (DT, for example).
> > 
> > The problem is that these mappings persist even if the interrupt has
> > been released by the driver (it called free_irq()), and the IRQ number
> > can be further reused. The client driver could dispose of the mapping
> > after having released the IRQ, but nobody does that in practice.
> > 
> > From the point of view of the controller, there is no simple way to
> > tell when an interrupt is "unused". And even if a driver was
> > overzealous and called irq_dispose_mapping() on all the possible
> > mappings (and made sure no mapping could be created in parallel), this
> > could result in a bunch of dangling pointers should a client driver
> > still have the interrupt requested.
> > 
> > Fixing this is pretty hard, as IRQ descriptors are leaky (you can
> > either have a pointer to one, or just an IRQ number -- they are
> > strictly equivalent). So in general, being able to remove an interrupt
> > controller driver is at best fragile, and I'm trying not to get more
> > of this in the tree.
> 
> Thank you!
> 
> How do we identify an interrupt controller driver?  Apparently some of
> these PCIe controller drivers also include an interrupt controller
> driver, but I don't know what to look for to find them.

If you see a struct irq_chip somewhere, this is an interrupt
controller.  And yes, most of the PCIe RC drivers will have some sort
of interrupt controller driver for INTx support, as well as MSI when
the RC doesn't/cannot rely on the platform providing one.

It means that these PCIe RC drivers probably shouldn't be removable if
built as modules. Which I don't think is a big problem. You want
modularity to reduce the size of the kernel image and only load the
drivers the platform actually requires, saving memory in the process.
And for something as fundamental as an interrupt controller (and PCIe
in general), you probably want to keep it around for the lifetime of
the machine.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.