On 1/30/21 1:22 PM, Jakub Kicinski wrote:
On Sat, 30 Jan 2021 10:25:16 -0500 Willem de Bruijn wrote:
@@ -894,12 +894,16 @@ int gsi_channel_start(struct gsi *gsi, u32 channel_id)
struct gsi_channel *channel = &gsi->channel[channel_id];
int ret;
- /* Enable the completion interrupt */
+ /* Enable NAPI and the completion interrupt */
+ napi_enable(&channel->napi);
gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
ret = __gsi_channel_start(channel, true);
- if (ret)
- gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
+ if (!ret)
+ return 0;
+
+ gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
+ napi_disable(&channel->napi);
return ret;
}
subjective, but easier to parse when the normal control flow is linear
and the error path takes a branch (or goto, if reused).
FWIW - fully agreed, I always thought this was part of the kernel
coding style.
That's fine. I will post a v2 of the series to fix this up.
I'll wait a bit (maybe until Monday morning), in case there's
any other input on the series to address.
Thanks both of you for your comments.
-Alex
