On Tue, Sep 25, 2018 at 7:20 AM Eric Dumazet <eduma...@google.com> wrote: > > On Tue, Sep 25, 2018 at 7:02 AM Michael Chan <michael.c...@broadcom.com> > wrote: > > > > On Mon, Sep 24, 2018 at 2:18 PM Song Liu <songliubrav...@fb.com> wrote: > > > > > > > > > > > > > On Sep 24, 2018, at 2:05 PM, Eric Dumazet <eduma...@google.com> wrote: > > > > > > > >> > > > >> Interesting, maybe a bnxt specific issue. > > > >> > > > >> It seems their model is to process TX/RX notification in the same > > > >> queue, > > > >> they throw away RX events if budget == 0 > > > >> > > > >> It means commit e7b9569102995ebc26821789628eef45bd9840d8 is wrong and > > > >> must be reverted. > > > >> > > > >> Otherwise, we have a possibility of blocking a queue under netpoll > > > >> pressure. > > > > > > > > Hmm, actually a revert might not be enough, since code at lines > > > > 2030-2031 > > > > would fire and we might not call napi_complete_done() anyway. > > > > > > > > Unfortunately this driver logic is quite complex. > > > > > > > > Could you test on other NIC eventually ? > > > > > > > > > > It actually runs OK on ixgbe. > > > > > > @Michael, could you please help us with this? > > > > > I've taken a quick look using today's net tree plus Eric's > > poll_one_napi() patch. The problem I'm seeing is that netpoll calls > > bnxt_poll() with budget 0. And since work_done >= budget of 0, we > > return without calling napi_complete_done() and without arming the > > interrupt. netpoll doesn't always call us back until we call > > napi_complete_done(), right? So I think if there are in-flight TX > > completions, we'll miss those. > > That's the whole point of netpoll : > > We drain the TX queues, without interrupts being involved at all, > by calling ->napi() with a zero budget. > > napi_complete(), even if called from ->napi() while budget was zero, > should do nothing but return early. > > budget==0 means that ->napi() should process all TX completions.
All TX completions that we can see. We cannot see the in-flight ones. If budget is exceeded, I think the assumption is that poll will always be called again. > > So it looks like bnxt has a bug, that is showing up after the latest > poll_one_napi() patch. > This latest patch is needed otherwise the cpu attempting the > netpoll-TX-drain might drain nothing at all, > since it does not anymore call ndo_poll_controller() that was grabbing > SCHED bits on all queues (napi_schedule() like calls) I think the latest patch is preventing the normal interrupt -> NAPI path from coming in and cleaning the remaining TX completions and arming the interrupt.
