Re: [PATCH net-next] be2net: fix INTx ISR for interrupt behaviour on BE2
From: Ben Hutchings <hidden>
Date: 2012-11-28 20:20:04
On Wed, 2012-11-28 at 11:20 +0530, Sathya Perla wrote:
On BE2 chip, an interrupt may be raised even when EQ is in un-armed state. As a result be_intx()::events_get() and be_poll:events_get() can race and notify an EQ wrongly. Fix this by counting events only in be_poll(). Commit 0b545a629 fixes the same issue in the MSI-x path. But, on Lancer, INTx can be de-asserted only by notifying num evts. This is not an issue as the above BE2 behavior doesn't exist/has never been seen on Lancer.
[...]
quoted hunk ↗ jump to hunk
@@ -2014,15 +1996,23 @@ static int be_rx_cqs_create(struct be_adapter *adapter) static irqreturn_t be_intx(int irq, void *dev) { - struct be_adapter *adapter = dev; - int num_evts; + struct be_eq_obj *eqo = dev; + struct be_adapter *adapter = eqo->adapter; + int num_evts = 0; - /* With INTx only one EQ is used */ - num_evts = event_handle(&adapter->eq_obj[0]); - if (num_evts) - return IRQ_HANDLED; - else - return IRQ_NONE; + /* On Lancer, clear-intr bit of the EQ DB does not work. + * INTx is de-asserted only on notifying num evts. + */ + if (lancer_chip(adapter)) + num_evts = events_get(eqo); + + /* The EQ-notify may not de-assert INTx rightaway, causing + * the ISR to be invoked again. So, return HANDLED even when + * num_evts is zero. + */ + be_eq_notify(adapter, eqo->q.id, false, true, num_evts); + napi_schedule(&eqo->napi); + return IRQ_HANDLED; }
[...] You shouldn't unconditionally return IRQ_HANDLED. This prevents interrupt storm detection from working, not just for your device but for anything else sharing its IRQ. I understand there is a real problem to be fixed (PCIe write completions overtaking INTx deassertion, and maybe a specific hardware bug). The way we dealt with such problems in sfc is to count the number of times in a row that we don't see any events, and only return IRQ_HANDLED the first time. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.