| Message ID | 20181123213724.14018-1-eduardoabinader@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | mt76: handle protocol error to proper deinit rx_tasklet | expand |
> During removal of usb dongle, noticed many unhandled rx urb > below. This this patch, make it possible and early completion > of the rx tasklet. > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > ... Hi Eduardo, I think EPROTO is a more general error (it is not strictly related to device removal) and it could happen even during normal operation. In this case I guess we should reinsert the urb to usb-core. Regards, Lorenzo > > Signed-off-by: Eduardo Abinader <eduardoabinader@gmail.com> > --- > drivers/net/wireless/mediatek/mt76/usb.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c > index 5f0faf07c346..dc33df9cd155 100644 > --- a/drivers/net/wireless/mediatek/mt76/usb.c > +++ b/drivers/net/wireless/mediatek/mt76/usb.c > @@ -446,6 +446,7 @@ static void mt76u_complete_rx(struct urb *urb) > case -ECONNRESET: > case -ESHUTDOWN: > case -ENOENT: > + case -EPROTO: > return; > default: > dev_err(dev->dev, "rx urb failed: %d\n", urb->status); > -- > 2.19.1 >
On Sat, Nov 24, 2018 at 10:25:52AM +0100, Lorenzo Bianconi wrote: > > During removal of usb dongle, noticed many unhandled rx urb > > below. This this patch, make it possible and early completion > > of the rx tasklet. > > > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 If problem here is just printing errors, I think we are ok. If problem is some hung or infinity loop, it should be fixed. > Hi Eduardo, > > I think EPROTO is a more general error (it is not strictly related to > device removal) > and it could happen even during normal operation. In this case I guess we should > reinsert the urb to usb-core. Some (out of tree) usb host drivers returns EPROTO all the time in some error conditions. On rt2x00 resubmitting urb resulted in infinity loop: https://marc.info/?t=153375128700002&r=1&w=2 Fix I proposed for that was counting EPROTO errors and mark device as removed if 10 of them happened in row: https://marc.info/?l=linux-wireless&m=153441755529960&w=2 Thanks Stanislaw
On Mon, 26 Nov 2018 at 10:59, Stanislaw Gruszka <sgruszka@redhat.com> wrote: > > On Sat, Nov 24, 2018 at 10:25:52AM +0100, Lorenzo Bianconi wrote: > > > During removal of usb dongle, noticed many unhandled rx urb > > > below. This this patch, make it possible and early completion > > > of the rx tasklet. > > > > > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > > > mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 > > If problem here is just printing errors, I think we are ok. If problem > is some hung or infinity loop, it should be fixed. > > > Hi Eduardo, > > > > I think EPROTO is a more general error (it is not strictly related to > > device removal) > > and it could happen even during normal operation. In this case I guess we should > > reinsert the urb to usb-core. > > Some (out of tree) usb host drivers returns EPROTO all the time > in some error conditions. On rt2x00 resubmitting urb resulted in > infinity loop: > https://marc.info/?t=153375128700002&r=1&w=2 > > Fix I proposed for that was counting EPROTO errors and > mark device as removed if 10 of them happened in row: > https://marc.info/?l=linux-wireless&m=153441755529960&w=2 Thanks for the feedback, guys. Would it be ok for you if I respin the patch with the suggested approach by Stanislaw? Regards, Eduardo > > Thanks > Stanislaw
> > > > During removal of usb dongle, noticed many unhandled rx urb > > > > below. This this patch, make it possible and early completion > > > > of the rx tasklet. [...] > > > Hi Eduardo, > > > > > > I think EPROTO is a more general error (it is not strictly related to > > > device removal) > > > and it could happen even during normal operation. In this case I guess we should > > > reinsert the urb to usb-core. > > > > Some (out of tree) usb host drivers returns EPROTO all the time > > in some error conditions. On rt2x00 resubmitting urb resulted in > > infinity loop: > > https://marc.info/?t=153375128700002&r=1&w=2 > > > > Fix I proposed for that was counting EPROTO errors and > > mark device as removed if 10 of them happened in row: > > https://marc.info/?l=linux-wireless&m=153441755529960&w=2 > > Thanks for the feedback, guys. > > Would it be ok for you if I respin the patch with the suggested > approach by Stanislaw? Hi Eduardo, AFAIU the -EPROTO error is reported only during device removal or just sporadically (you have never experienced any infinity loop). Is it correct? If so I guess we do not need to develop any intrusive workaround since it is just an annoying log message Regards, Lorenzo > > Regards, > Eduardo > > > > Thanks > > Stanislaw
On Mon, 26 Nov 2018 at 22:31, Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > > > > > During removal of usb dongle, noticed many unhandled rx urb > > > > > below. This this patch, make it possible and early completion > > > > > of the rx tasklet. > > [...] > > > > > Hi Eduardo, > > > > > > > > I think EPROTO is a more general error (it is not strictly related to > > > > device removal) > > > > and it could happen even during normal operation. In this case I guess we should > > > > reinsert the urb to usb-core. > > > > > > Some (out of tree) usb host drivers returns EPROTO all the time > > > in some error conditions. On rt2x00 resubmitting urb resulted in > > > infinity loop: > > > https://marc.info/?t=153375128700002&r=1&w=2 > > > > > > Fix I proposed for that was counting EPROTO errors and > > > mark device as removed if 10 of them happened in row: > > > https://marc.info/?l=linux-wireless&m=153441755529960&w=2 > > > > Thanks for the feedback, guys. > > > > Would it be ok for you if I respin the patch with the suggested > > approach by Stanislaw? > > Hi Eduardo, > > AFAIU the -EPROTO error is reported only during device removal or just > sporadically > (you have never experienced any infinity loop). Is it correct? > If so I guess we do not need to develop any intrusive workaround since > it is just an annoying log message > It is reported only during device removal as I've been seen with my comfast dongle. The device removal does not lead to an infinity loop, but there's a bouncing around 100 urb failed msgs... Thanks, Eduardo > Regards, > Lorenzo > > > > > Regards, > > Eduardo > > > > > > Thanks > > > Stanislaw
> On Mon, 26 Nov 2018 at 22:31, Lorenzo Bianconi > <lorenzo.bianconi@redhat.com> wrote: [...] > > Hi Eduardo, > > > > AFAIU the -EPROTO error is reported only during device removal or just > > sporadically > > (you have never experienced any infinity loop). Is it correct? > > If so I guess we do not need to develop any intrusive workaround since > > it is just an annoying log message > > > > It is reported only during device removal as I've been seen with my > comfast dongle. > The device removal does not lead to an infinity loop, but there's a > bouncing around 100 urb failed msgs... > Thx for the clarification Eduardo. So is it better to mute all -EPROTO errors or tolerate these annoying messages during dongle removal? I would prefer the second choice since would be useful to know if the controller reports that kind of errors. Regards, Lorenzo > Thanks, > Eduardo > > > Regards, > > Lorenzo > > > > > > > > Regards, > > > Eduardo > > > > > > > > Thanks > > > > Stanislaw
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c index 5f0faf07c346..dc33df9cd155 100644 --- a/drivers/net/wireless/mediatek/mt76/usb.c +++ b/drivers/net/wireless/mediatek/mt76/usb.c @@ -446,6 +446,7 @@ static void mt76u_complete_rx(struct urb *urb) case -ECONNRESET: case -ESHUTDOWN: case -ENOENT: + case -EPROTO: return; default: dev_err(dev->dev, "rx urb failed: %d\n", urb->status);
During removal of usb dongle, noticed many unhandled rx urb below. This this patch, make it possible and early completion of the rx tasklet. mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 mt76x2u 1-3.4.3.1.2:1.0: rx urb failed: -71 ... Signed-off-by: Eduardo Abinader <eduardoabinader@gmail.com> --- drivers/net/wireless/mediatek/mt76/usb.c | 1 + 1 file changed, 1 insertion(+)