On 2024-12-20 12:37 p.m., Jakub Kicinski wrote:
On Fri, 20 Dec 2024 12:15:33 -0700 Ahmed Zaki wrote:
I don't understand what you're trying to say, could you rephrase?
Sure. After this patch, we have (simplified):
void netif_napi_set_irq(struct napi_struct *napi, int irq, unsigned long
flags)
{
struct irq_glue *glue = NULL;
int rc;
napi->irq = irq;
#ifdef CONFIG_RFS_ACCEL
if (napi->dev->rx_cpu_rmap && flags & NAPIF_IRQ_ARFS_RMAP) {
rc = irq_cpu_rmap_add(napi->dev->rx_cpu_rmap, irq, napi,
netif_irq_cpu_rmap_notify);
.
.
.
}
#endif
if (flags & NAPIF_IRQ_AFFINITY) {
glue = kzalloc(sizeof(*glue), GFP_KERNEL);
if (!glue)
return;
glue->notify.notify = netif_irq_cpu_rmap_notify;
glue->notify.release = netif_napi_affinity_release;
.
.
}
}
Both branches assign the new cb function "netif_irq_cpu_rmap_notify()"
as the new IRQ notifier, but the first branch calls irq_cpu_rmap_add()
where the notifier is embedded in "struct irq_glue". So the cb function
needs to assume the notifier is inside irq_glue, so the second "if"
branch needs to do the same.
First off, I'm still a bit confused why you think the flags should be
per NAPI call and not set at init time, once.
Perhaps rename netif_enable_cpu_rmap() suggested earlier to something
more generic (netif_enable_irq_tracking()?) and pass the flags there?
Or is there a driver which wants to vary the flags per NAPI instance?
set_irq() just seemed like natural choice since it is already called for
each IRQ. I was also trying to avoid adding a new function. But sure I
can do that and move the flags to netdev.
Then you can probably register a single unified handler, and inside
that handler check if the device wanted to have rmap or just affinity?
This is what is in this patch already, all drivers following new
approach will have netif_irq_cpu_rmap_notify() as their IRQ notifier.
IIUC, your goal is to have the notifier inside napi, not irq_glue. For
this, we'll have to have our own version of irq_cpu_rmap_add() (for the
above reason).
sounds OK?
