On Mon, Jan 01, 2024 at 07:37:25PM +0200, Ilpo Järvinen wrote: > On Sat, 30 Dec 2023, Lukas Wunner wrote: > > On Fri, Sep 29, 2023 at 02:57:20PM +0300, Ilpo Järvinen wrote: > > > + pcie_capability_write_word(dev, PCI_EXP_LNKSTA, PCI_EXP_LNKSTA_LBMS); > > > + pcie_capability_set_word(dev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_LBMIE); > > > > I'm wondering why we're not enabling LABIE as well? > > (And clear LABS.) > > > > Can't it happen that we miss bandwidth changes unless we enable that > > as well? > > Thanks. Reading the spec, it sounds like both are necessary to not miss > changes. I guess this is an artefact of Alex' original patch. I don't know why he enabled one but not the other. > > > + ret = request_irq(srv->irq, pcie_bw_notification_irq, > > > + IRQF_SHARED, "PCIe BW ctrl", srv); > > > > Is there a reason to run the IRQ handler in hardirq context > > or would it work to run it in an IRQ thread? Usually on systems > > than enable PREEMPT_RT, a threaded IRQ handler is preferred, > > so unless hardirq context is necessary, I'd recommend using > > an IRQ thread. > > Can I somehow postpone the decision between IRQ_NONE / IRQ_HANDLED > straight into the thread_fn? One LNKSTA read is necessary to decide > that. > > I suppose the other write + reread of LNKSTA could be moved into > thread_fn even if the first read would not be movable. You can just use request_threaded_irq(), pass NULL for the "handler" argument and pcie_bw_notification_irq for the "thread_fn" argument. Because of the NULL argument for "handler", the hardirq handler will then become irq_default_primary_handler(). Which does nothing else but return IRQ_WAKE_THREAD. And the decision between IRQ_NONE and IRQ_HANDLED is then indeed postponed to the IRQ thread. Alternatively you can split the IRQ handler, move the check whether PCI_EXP_LNKSTA_LBMS is set to the hardirq handler and keep the rest in the IRQ thread. Means you won't have unnecessary wakeups of the IRQ thread if the interrupt is caused by something else (I understand it's always shared with PME and hotplug). But you'll spend more time in hardirq context. In practice bandwidth notifications may be more frequent than PME and hotplug interrupts, so unnecessary wakeups of the IRQ thread will be rare. Hence not splitting the IRQ handler may be better. Dunno. Ask Thomas Gleixner or Sebastian Siewior. :) Thanks, Lukas
