Re: CVS commit: src/sys/external/bsd/libnv/dist
On Fri, Feb 15, 2019 at 11:21:51PM +, Mindaugas Rasiukevicius wrote: > Well, I did run libnv tests as well as NPF tests and they both passed, > just not on NetBSD. :) Turns out NetBSD libnv and Github libnv code > bases deviated a little bit, causing a bug in NetBSD (but not upstream). > Anyway, I committed the fix. As for the NetBSD tests: it takes forever > to recompile and rerun them, so it is not really practical for me. You can recompile parts of it and also run sub-tests (like in this case: the npf tests, which would have taken like 10 min combined), and you do not need to run the (sub-) tests on qemu. You could post patches and ask for others to include the patches in their next test runs. The "not practical" bit sounds to me like "I am in a hurry, I can not deal properly with it", which feels slightly wrong. But you dealt with the fallout and this is -current, so no big deal and thank you for fixing it! Martin
Re: CVS commit: src/sys/external/bsd/libnv/dist
Martin Husemann wrote: > Can you try this patch? > > rmind: can you *please* run the tests before commiting such changes? > That would have immediately caught this. Well, I did run libnv tests as well as NPF tests and they both passed, just not on NetBSD. :) Turns out NetBSD libnv and Github libnv code bases deviated a little bit, causing a bug in NetBSD (but not upstream). Anyway, I committed the fix. As for the NetBSD tests: it takes forever to recompile and rerun them, so it is not really practical for me. -- Mindaugas
Re: CVS commit: src/sys/netinet
Rin Okuyama wrote: >Ping? Can I insert "break" here? Sorry, yes you are correct that it should be a break. Will commit it now. Robert Swindells
Re: Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)
On 2019/02/15 23:37, Nick Hudson wrote:
On 15/02/2019 13:44, Rin Okuyama wrote:
On 2019/02/15 21:57, Jonathan A. Kollasch wrote:
On Wed, Feb 13, 2019 at 06:35:14PM +0900, Rin Okuyama wrote:
Hi,
On 2019/02/13 3:54, Nick Hudson wrote:
On 12/02/2019 16:02, Rin Okuyama wrote:
Hi,
The system freezes indefinitely with xhci(4) or ehci(4), when NIC with
multiple outstanding transfers [axen(4), mue(4), and ure(4)] is stopped
by "ifconfig down" or detached.
As discussed in the previous message, this is due to infinite loop in
usbd_ar_pipe(); xfers with USBD_NOT_STARTED remain in a queue forever
because upm_abort [= xhci_device_bulk_abort() etc.] cannot remove them.
Why not the attached patch instead?
Nick
Thank you so much for your prompt reply!
It works if s/ux_state/ux_status/ here:
+ if (xfer->ux_state == USBD_NOT_STARTED) {
Can I commit the revised patch?
The revised patch results in
https://nxr.netbsd.org/xref/src/sys/dev/usb/ehci.c?r=1.265#1566
firing upon `drvctl detach -d bwfm0` on Pinebook.
Jonathan Kollasch
IMO, this is because NOT_STARTED queues are removed in a way that is
unexpected to ehci. For my old amd64 box, the attached patch fixes
assertion failures when devices are detached from ehci. I would like
to commit it together with the previous patch.
ux_state has probably out lived its usefulness.
Other/All HCs do the same ux_state check. So, either all need to be updated or
we do the same XFER_ONQU to XFER_BUSY transition in usbd_ar_pipe
Nick
The latter does not work because ehci and uhci also check its own flags,
ex_isdone and ux_isdone, respectively. Therefore, I chose the former:
http://www.netbsd.org/~rin/usbhc_freex_20190215.patch
Is this OK for you?
Thanks,
rin
Re: CVS commit: src/sys/netinet
On 2019/02/15 23:04, Robert Swindells wrote: Rin Okuyama wrote: Ping? Can I insert "break" here? Sorry, yes you are correct that it should be a break. Will commit it now. Robert Swindells Thanks! rin
Re: Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)
On 15/02/2019 13:44, Rin Okuyama wrote:
On 2019/02/15 21:57, Jonathan A. Kollasch wrote:
On Wed, Feb 13, 2019 at 06:35:14PM +0900, Rin Okuyama wrote:
Hi,
On 2019/02/13 3:54, Nick Hudson wrote:
On 12/02/2019 16:02, Rin Okuyama wrote:
Hi,
The system freezes indefinitely with xhci(4) or ehci(4), when NIC with
multiple outstanding transfers [axen(4), mue(4), and ure(4)] is
stopped
by "ifconfig down" or detached.
As discussed in the previous message, this is due to infinite loop in
usbd_ar_pipe(); xfers with USBD_NOT_STARTED remain in a queue forever
because upm_abort [= xhci_device_bulk_abort() etc.] cannot remove
them.
Why not the attached patch instead?
Nick
Thank you so much for your prompt reply!
It works if s/ux_state/ux_status/ here:
+ if (xfer->ux_state == USBD_NOT_STARTED) {
Can I commit the revised patch?
The revised patch results in
https://nxr.netbsd.org/xref/src/sys/dev/usb/ehci.c?r=1.265#1566
firing upon `drvctl detach -d bwfm0` on Pinebook.
Jonathan Kollasch
IMO, this is because NOT_STARTED queues are removed in a way that is
unexpected to ehci. For my old amd64 box, the attached patch fixes
assertion failures when devices are detached from ehci. I would like
to commit it together with the previous patch.
ux_state has probably out lived its usefulness.
Other/All HCs do the same ux_state check. So, either all need to be
updated or we do the same XFER_ONQU to XFER_BUSY transition in usbd_ar_pipe
Nick
Re: Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)
On Fri, Feb 15, 2019 at 10:44:23PM +0900, Rin Okuyama wrote:
> On 2019/02/15 21:57, Jonathan A. Kollasch wrote:
> > On Wed, Feb 13, 2019 at 06:35:14PM +0900, Rin Okuyama wrote:
> > > Hi,
> > >
> > > On 2019/02/13 3:54, Nick Hudson wrote:
> > > > On 12/02/2019 16:02, Rin Okuyama wrote:
> > > > > Hi,
> > > > >
> > > > > The system freezes indefinitely with xhci(4) or ehci(4), when NIC with
> > > > > multiple outstanding transfers [axen(4), mue(4), and ure(4)] is
> > > > > stopped
> > > > > by "ifconfig down" or detached.
> > > > >
> > > > > As discussed in the previous message, this is due to infinite loop in
> > > > > usbd_ar_pipe(); xfers with USBD_NOT_STARTED remain in a queue forever
> > > > > because upm_abort [= xhci_device_bulk_abort() etc.] cannot remove
> > > > > them.
> > > >
> > > >
> > > > Why not the attached patch instead?
> > > >
> > > > Nick
> > >
> > > Thank you so much for your prompt reply!
> > >
> > > It works if s/ux_state/ux_status/ here:
> > >
> > > + if (xfer->ux_state == USBD_NOT_STARTED) {
> > >
> > > Can I commit the revised patch?
> > >
> >
> > The revised patch results in
> > https://nxr.netbsd.org/xref/src/sys/dev/usb/ehci.c?r=1.265#1566
> > firing upon `drvctl detach -d bwfm0` on Pinebook.
> >
> > Jonathan Kollasch
>
> IMO, this is because NOT_STARTED queues are removed in a way that is
> unexpected to ehci. For my old amd64 box, the attached patch fixes
> assertion failures when devices are detached from ehci. I would like
> to commit it together with the previous patch.
>
Works for me; please commit. (I'm not 100% sure it's perfect, but
it's better than it was, and we can fix it again later if necessary.)
Jonathan Kollasch
Re: CVS commit: src/sys/netinet
Ping? Can I insert "break" here?
Thanks,
rin
On 2019/02/12 23:59, Rin Okuyama wrote:
Hi,
On 2019/02/12 23:40, Robert Swindells wrote:
Module Name: src
Committed By: rjs
Date: Tue Feb 12 14:40:38 UTC 2019
Modified Files:
src/sys/netinet: sctp_input.c sctp_usrreq.c
Log Message:
Add some fallthrough annotations.
...
Index: src/sys/netinet/sctp_usrreq.c
diff -u src/sys/netinet/sctp_usrreq.c:1.14 src/sys/netinet/sctp_usrreq.c:1.15
--- src/sys/netinet/sctp_usrreq.c:1.14 Mon Jan 28 12:53:01 2019
+++ src/sys/netinet/sctp_usrreq.c Tue Feb 12 14:40:38 2019
@@ -1,5 +1,5 @@
/* $KAME: sctp_usrreq.c,v 1.50 2005/06/16 20:45:29 jinmei Exp $ */
-/* $NetBSD: sctp_usrreq.c,v 1.14 2019/01/28 12:53:01 martin Exp $ */
+/* $NetBSD: sctp_usrreq.c,v 1.15 2019/02/12 14:40:38 rjs Exp $ */
/*
* Copyright (c) 2001, 2002, 2003, 2004 Cisco Systems, Inc.
@@ -33,7 +33,7 @@
* SUCH DAMAGE.
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: sctp_usrreq.c,v 1.14 2019/01/28 12:53:01 martin Exp
$");
+__KERNEL_RCSID(0, "$NetBSD: sctp_usrreq.c,v 1.15 2019/02/12 14:40:38 rjs Exp
$");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -2289,7 +2289,7 @@ sctp_optsget(struct socket *so, struct s
*s_info = stcb->asoc.def_send;
SCTP_TCB_UNLOCK(stcb);
sopt->sopt_size = sizeof(*s_info);
- }
+ } /* FALLTHROUGH */
case SCTP_INITMSG:
{
struct sctp_initmsg *sinit;
It seems for me that we need "break" here; sopt->sopt_data and
sopt_size are unconditionally overwritten if fallthrough.
Thoughts?
Thanks,
rin
Re: Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)
On 2019/02/15 21:57, Jonathan A. Kollasch wrote:
On Wed, Feb 13, 2019 at 06:35:14PM +0900, Rin Okuyama wrote:
Hi,
On 2019/02/13 3:54, Nick Hudson wrote:
On 12/02/2019 16:02, Rin Okuyama wrote:
Hi,
The system freezes indefinitely with xhci(4) or ehci(4), when NIC with
multiple outstanding transfers [axen(4), mue(4), and ure(4)] is stopped
by "ifconfig down" or detached.
As discussed in the previous message, this is due to infinite loop in
usbd_ar_pipe(); xfers with USBD_NOT_STARTED remain in a queue forever
because upm_abort [= xhci_device_bulk_abort() etc.] cannot remove them.
Why not the attached patch instead?
Nick
Thank you so much for your prompt reply!
It works if s/ux_state/ux_status/ here:
+ if (xfer->ux_state == USBD_NOT_STARTED) {
Can I commit the revised patch?
The revised patch results in
https://nxr.netbsd.org/xref/src/sys/dev/usb/ehci.c?r=1.265#1566
firing upon `drvctl detach -d bwfm0` on Pinebook.
Jonathan Kollasch
IMO, this is because NOT_STARTED queues are removed in a way that is
unexpected to ehci. For my old amd64 box, the attached patch fixes
assertion failures when devices are detached from ehci. I would like
to commit it together with the previous patch.
Thanks,
rin
Index: sys/dev/usb/ehci.c
===
RCS file: /home/netbsd/src/sys/dev/usb/ehci.c,v
retrieving revision 1.265
diff -p -u -r1.265 ehci.c
--- sys/dev/usb/ehci.c 18 Sep 2018 02:00:06 - 1.265
+++ sys/dev/usb/ehci.c 13 Feb 2019 19:13:34 -
@@ -1562,9 +1562,10 @@ ehci_freex(struct usbd_bus *bus, struct
struct ehci_softc *sc = EHCI_BUS2SC(bus);
struct ehci_xfer *ex __diagused = EHCI_XFER2EXFER(xfer);
- KASSERTMSG(xfer->ux_state == XFER_BUSY, "xfer %p state %d\n", xfer,
- xfer->ux_state);
- KASSERT(ex->ex_isdone);
+ KASSERTMSG(xfer->ux_status == USBD_NOT_STARTED ||
+ xfer->ux_state == XFER_BUSY,
+ "xfer %p state %d\n", xfer, xfer->ux_state);
+ KASSERT(xfer->ux_status == USBD_NOT_STARTED || ex->ex_isdone);
#ifdef DIAGNOSTIC
xfer->ux_state = XFER_FREE;
Re: Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)
On Wed, Feb 13, 2019 at 06:35:14PM +0900, Rin Okuyama wrote:
> Hi,
>
> On 2019/02/13 3:54, Nick Hudson wrote:
> > On 12/02/2019 16:02, Rin Okuyama wrote:
> > > Hi,
> > >
> > > The system freezes indefinitely with xhci(4) or ehci(4), when NIC with
> > > multiple outstanding transfers [axen(4), mue(4), and ure(4)] is stopped
> > > by "ifconfig down" or detached.
> > >
> > > As discussed in the previous message, this is due to infinite loop in
> > > usbd_ar_pipe(); xfers with USBD_NOT_STARTED remain in a queue forever
> > > because upm_abort [= xhci_device_bulk_abort() etc.] cannot remove them.
> >
> >
> > Why not the attached patch instead?
> >
> > Nick
>
> Thank you so much for your prompt reply!
>
> It works if s/ux_state/ux_status/ here:
>
> + if (xfer->ux_state == USBD_NOT_STARTED) {
>
> Can I commit the revised patch?
>
The revised patch results in
https://nxr.netbsd.org/xref/src/sys/dev/usb/ehci.c?r=1.265#1566
firing upon `drvctl detach -d bwfm0` on Pinebook.
Jonathan Kollasch
