Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-02-12 Thread Martin Husemann
On Tue, Feb 12, 2019 at 09:20:23PM -, Christos Zoulas wrote:
> Well, regardless of what the right permissions of .eh_frame are,
> we could just nuke the code and default to the "new" behavior...
> We can then put back the varasm.c change...
> This of course needs to be evaluated carefully.

Why would anyone modify the eh frame list at runtime? Or is any other
writable data placed there on purpose? I didn't see any, and if - wouldn't
it better go into a separate section?

Martin


Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-02-12 Thread Christos Zoulas
In article <16690.1550005...@splode.eterna.com.au>,
matthew green   wrote:
>Joerg Sonnenberger writes:
>> On Tue, Feb 12, 2019 at 10:16:58AM +, matthew green wrote:
>> > Module Name:   src
>> > Committed By:  mrg
>> > Date:  Tue Feb 12 10:16:58 UTC 2019
>> > 
>> > Modified Files:
>> >src/usr.bin/crunch/crunchgen: crunchgen.c
>> > 
>> > Log Message:
>> > hack alert time:
>> > 
>> > on sparc and sparc64, don't remove .eh_frame section.  it leads
>> > to failure as something is referenced, and objcopy ends up
>> > emitting a broken binary that can't be run -- it attempts to
>> > load at va=0, beyond having missing referenced data.
>> > 
>> > also, on sparc64 also don't remove .note.netbsd.mcmodel.
>> > 
>> > the former should be revised when we can avoid it.
>> 
>> The real bug is the reverted varasm.c change. GCC creates the .eh_frame
>> section with the wrong permissions.
>
>yes - putting your varasm.c back fixes the crtbegin.o
>build, but it breaks the libstdc++ one:
>
>In file included from
>/usr/src4/external/gpl3/gcc/dist/libstdc++-v3/src/c++98/pool_allocator.cc:31:0:
>/usr/src4/external/gpl3/gcc/dist/libstdc++-v3/include/ext/pool_allocator.h:210:5:
> error: only zero initializers are allowed in section
> __pool_alloc<_Tp>::_S_force_new;
> ^
>i'm not sure how to solve this -- we apparently want both
>behaviours with GCC 7.  uwe@ suggested an explicit asm()
>or .S file for the crtbegin issue.

Isn't this used only to deal with "GLIBCXX_FORCE_NEW"? And "NEW" is a
misnomer since this thing started more than 10 years ago, and it still
bites: https://www.zerotier.com/blog/2017-05-05-theleak.shtml

Well, regardless of what the right permissions of .eh_frame are,
we could just nuke the code and default to the "new" behavior...
We can then put back the varasm.c change...
This of course needs to be evaluated carefully.

christos



Re: DIAGNOSTIC for modules (Re: CVS commit: src/sys/dev/usb)

2019-02-12 Thread Paul Goyette

On Tue, 12 Feb 2019, Rin Okuyama wrote:


Hi,

As Martin pointed out, it is useful for debugging to turn on
DIAGNOSTIC for modules (for non-release branches).

Now, all modules for amd64 are successfully built with DIAGNOSTIC.

I'd like to commit the patch below, if there's no objection.


This would be very helpful.

I would also wonder if we could increase the WARNS?= level from 3 to 5 
(to match the current WARNS?= level used for kernel builds).  Has anyone 
tried to see how many modules would fail with WARNS?=5  ??




Thanks,
rin

Index: sys/modules/Makefile.inc
===
RCS file: /home/netbsd/src/sys/modules/Makefile.inc,v
retrieving revision 1.6
diff -p -u -r1.6 Makefile.inc
--- sys/modules/Makefile.inc11 Sep 2011 18:38:02 -  1.6
+++ sys/modules/Makefile.inc10 Feb 2019 11:39:11 -
@@ -5,6 +5,10 @@ CPPFLAGS+= -I${NETBSDSRCDIR}/common/incl
USE_FORT=   no
WARNS?= 3
+# inexpensive kernel consistency checks
+# XXX to be commented out on release branch
+CPPFLAGS+= -DDIAGNOSTIC
+
.if !empty(IOCONF)
_BSD_IOCONF_MK_USER_=1
.include 

On 2019/02/09 23:13, Martin Husemann wrote:

On Sat, Feb 09, 2019 at 11:53:57AM +0100, Michael van Elst wrote:

And that's why I see it for modules:

Index: sys/modules/Makefile.inc
===
RCS file: /cvsroot/src/sys/modules/Makefile.inc,v
retrieving revision 1.6
diff -p -u -r1.6 Makefile.inc
--- sys/modules/Makefile.inc11 Sep 2011 18:38:02 -  1.6
+++ sys/modules/Makefile.inc9 Feb 2019 10:53:03 -
@@ -1,7 +1,7 @@
 #  $NetBSD: Makefile.inc,v 1.6 2011/09/11 18:38:02 mbalmer Exp $
  S!=cd ${.PARSEDIR}/..;pwd
-CPPFLAGS+= -I${NETBSDSRCDIR}/common/include
+CPPFLAGS+= -I${NETBSDSRCDIR}/common/include -DDIAGNOSTIC
 USE_FORT=  no
 WARNS?=3


How about you commit that for HEAD and we remove it on branches for the
first non-beta build (like we remove options DIAGOSTIC from lots of 
kernels)?


Martin



!DSPAM:5c62daaa30766798715834!



+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+--+--++


re: CVS commit: src/usr.bin/crunch/crunchgen

2019-02-12 Thread matthew green
Joerg Sonnenberger writes:
> On Tue, Feb 12, 2019 at 10:16:58AM +, matthew green wrote:
> > Module Name:src
> > Committed By:   mrg
> > Date:   Tue Feb 12 10:16:58 UTC 2019
> > 
> > Modified Files:
> > src/usr.bin/crunch/crunchgen: crunchgen.c
> > 
> > Log Message:
> > hack alert time:
> > 
> > on sparc and sparc64, don't remove .eh_frame section.  it leads
> > to failure as something is referenced, and objcopy ends up
> > emitting a broken binary that can't be run -- it attempts to
> > load at va=0, beyond having missing referenced data.
> > 
> > also, on sparc64 also don't remove .note.netbsd.mcmodel.
> > 
> > the former should be revised when we can avoid it.
> 
> The real bug is the reverted varasm.c change. GCC creates the .eh_frame
> section with the wrong permissions.

yes - putting your varasm.c back fixes the crtbegin.o
build, but it breaks the libstdc++ one:

In file included from 
/usr/src4/external/gpl3/gcc/dist/libstdc++-v3/src/c++98/pool_allocator.cc:31:0:
/usr/src4/external/gpl3/gcc/dist/libstdc++-v3/include/ext/pool_allocator.h:210:5:
 error: only zero initializers are allowed in section 
'.bss._ZN9__gnu_cxx12__pool_allocIwE12_S_force_newE'
 __pool_alloc<_Tp>::_S_force_new;
 ^
i'm not sure how to solve this -- we apparently want both
behaviours with GCC 7.  uwe@ suggested an explicit asm()
or .S file for the crtbegin issue.


.mrg.


Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-02-12 Thread Joerg Sonnenberger
On Tue, Feb 12, 2019 at 10:16:58AM +, matthew green wrote:
> Module Name:  src
> Committed By: mrg
> Date: Tue Feb 12 10:16:58 UTC 2019
> 
> Modified Files:
>   src/usr.bin/crunch/crunchgen: crunchgen.c
> 
> Log Message:
> hack alert time:
> 
> on sparc and sparc64, don't remove .eh_frame section.  it leads
> to failure as something is referenced, and objcopy ends up
> emitting a broken binary that can't be run -- it attempts to
> load at va=0, beyond having missing referenced data.
> 
> also, on sparc64 also don't remove .note.netbsd.mcmodel.
> 
> the former should be revised when we can avoid it.

The real bug is the reverted varasm.c change. GCC creates the .eh_frame
section with the wrong permissions.

Joerg


Re: Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)

2019-02-12 Thread Nick Hudson

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
? sys/dev/usb/cscope.out
Index: sys/dev/usb/usbdi.c
===
RCS file: /cvsroot/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.181
diff -u -p -r1.181 usbdi.c
--- sys/dev/usb/usbdi.c 10 Jan 2019 22:13:07 -  1.181
+++ sys/dev/usb/usbdi.c 12 Feb 2019 18:51:28 -
@@ -884,9 +884,13 @@ usbd_ar_pipe(struct usbd_pipe *pipe)
USBHIST_LOG(usbdebug, "pipe = %#jx xfer = %#jx "
"(methods = %#jx)", (uintptr_t)pipe, (uintptr_t)xfer,
(uintptr_t)pipe->up_methods, 0);
-   /* Make the HC abort it (and invoke the callback). */
-   pipe->up_methods->upm_abort(xfer);
-   /* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
+   if (xfer->ux_state == USBD_NOT_STARTED) {
+   SIMPLEQ_REMOVE_HEAD(&pipe->up_queue, ux_next);
+   } else {
+   /* Make the HC abort it (and invoke the callback). */
+   pipe->up_methods->upm_abort(xfer);
+   /* XXX only for non-0 usbd_clear_endpoint_stall(pipe); 
*/
+   }
}
pipe->up_aborting = 0;
return USBD_NORMAL_COMPLETION;


Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)

2019-02-12 Thread Rin Okuyama

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.

I guess that this can happen for host controllers in which up_serialise
is true for bulk transfers [it does not occur for dwctwo(4), in which
up_serialise == false for bulk transfers]. For such adapters, IMO,
usbd_start_next(pipe) should be called whenever some xfer is removed
from pipe->up_queue.

Actually, the freeze seems to be fixed by the attached patch.
Can I commit the patch? Or, am I missing something?

Thanks,
rin

Index: usbdi.c
===
RCS file: /home/netbsd/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.181
diff -p -u -r1.181 usbdi.c
--- sys/dev/usb/usbdi.c 10 Jan 2019 22:13:07 -  1.181
+++ sys/dev/usb/usbdi.c 12 Feb 2019 14:07:35 -
@@ -1013,7 +1013,7 @@ usb_transfer_complete(struct usbd_xfer *
if (erred && pipe->up_iface != NULL) /* not control pipe */
pipe->up_running = 0;
}
-   if (pipe->up_running && pipe->up_serialise)
+   if (pipe->up_serialise)
usbd_start_next(pipe);
 }
 



On 2019/02/09 22:28, sc dying wrote:

On Sat, Feb 9, 2019 at 8:18 AM Rin Okuyama  wrote:


Hi,

On 2019/02/08 23:16, sc dying wrote:

On 2019/01/31 05:51, Rin Okuyama wrote:

By the way, I find that the system hangs silently by
"ifconfig mueN down" or detaching LAN7500 from USB port when
multiple outstanding requests are enabled. This does not occur
when MUE_TX_LIST_CNT = MUE_RX_LIST_CNT = 1. Do you have any ideas
to fix it?


My axen dongle locks up if AXEN_RX_LIST_CNT > 1
when ifconfig down on amd64 8.99.34.

db{0}> bt
breakpoint() at netbsd:breakpoint+0x5
comintr() at netbsd:comintr+0x861
Xhandle_ioapic_edge4() at netbsd:Xhandle_ioapic_edge4+0x66
--- interrupt ---
xhci_device_bulk_abort() at netbsd:xhci_device_bulk_abort+0x1c
usbd_ar_pipe() at netbsd:usbd_ar_pipe+0x1e9
usbd_abort_pipe() at netbsd:usbd_abort_pipe+0x27
axen_stop() at netbsd:axen_stop+0xc4
axen_ioctl() at netbsd:axen_ioctl+0x1d9
doifioctl() at netbsd:doifioctl+0xa99
sys_ioctl() at netbsd:sys_ioctl+0x11c
syscall() at netbsd:syscall+0xb4
--- syscall (number 54) ---
732731d1a88a:

Looks like kernel goes infinite loop in usbd_ar_pipe by some reason.
It tries to abort NOT_STARTED xfers.


Can you tell me how to reproduce this failure?
"ifconfig down" works for me on RPI3B+.


I tested on intel 3000 series CPU + Ivebridge
with kernel from my local tree.
But I found it does not happen with the lastest kernel from
https://nycdn.netbsd.org/pub/NetBSD-daily/HEAD/latest/amd64/binary/kernel/netbsd-GENERIC.gz.



Re: CVS commit: src/sys/netinet

2019-02-12 Thread Rin Okuyama

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


DIAGNOSTIC for modules (Re: CVS commit: src/sys/dev/usb)

2019-02-12 Thread Rin Okuyama

Hi,

As Martin pointed out, it is useful for debugging to turn on
DIAGNOSTIC for modules (for non-release branches).

Now, all modules for amd64 are successfully built with DIAGNOSTIC.

I'd like to commit the patch below, if there's no objection.

Thanks,
rin

Index: sys/modules/Makefile.inc
===
RCS file: /home/netbsd/src/sys/modules/Makefile.inc,v
retrieving revision 1.6
diff -p -u -r1.6 Makefile.inc
--- sys/modules/Makefile.inc11 Sep 2011 18:38:02 -  1.6
+++ sys/modules/Makefile.inc10 Feb 2019 11:39:11 -
@@ -5,6 +5,10 @@ CPPFLAGS+= -I${NETBSDSRCDIR}/common/incl
 USE_FORT=  no
 WARNS?=3
 
+# inexpensive kernel consistency checks

+# XXX to be commented out on release branch
+CPPFLAGS+= -DDIAGNOSTIC
+
 .if !empty(IOCONF)
 _BSD_IOCONF_MK_USER_=1
 .include 

On 2019/02/09 23:13, Martin Husemann wrote:

On Sat, Feb 09, 2019 at 11:53:57AM +0100, Michael van Elst wrote:

And that's why I see it for modules:

Index: sys/modules/Makefile.inc
===
RCS file: /cvsroot/src/sys/modules/Makefile.inc,v
retrieving revision 1.6
diff -p -u -r1.6 Makefile.inc
--- sys/modules/Makefile.inc11 Sep 2011 18:38:02 -  1.6
+++ sys/modules/Makefile.inc9 Feb 2019 10:53:03 -
@@ -1,7 +1,7 @@
  #  $NetBSD: Makefile.inc,v 1.6 2011/09/11 18:38:02 mbalmer Exp $
  
  S!=cd ${.PARSEDIR}/..;pwd

-CPPFLAGS+= -I${NETBSDSRCDIR}/common/include
+CPPFLAGS+= -I${NETBSDSRCDIR}/common/include -DDIAGNOSTIC
  USE_FORT=  no
  WARNS?=3


How about you commit that for HEAD and we remove it on branches for the
first non-beta build (like we remove options DIAGOSTIC from lots of kernels)?

Martin