re: CVS commit: src/usr.bin/crunch/crunchgen
> >> 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. FWIW, there are several files that have problem with the varasm.c change applied. https://www.netbsd.org/~mrg/gcc7-no-varasm-change-vs-libstdc++.txt all these fail: compile libstdc++-v3/bitmap_allocator.o compile libstdc++-v3/pool_allocator.o compile libstdc++-v3/mt_allocator.o compile libstdc++-v3/locale-inst.o compile libstdc++-v3/string-inst.o compile libstdc++-v3/wlocale-inst.o compile libstdc++-v3/wstring-inst.o christos, can you be more detailed in what exactly you think we can delete or so? it's not just stuff related to the GLIBCXX_FORCE_NEW variable AFAICT. .mrg.
re: CVS commit: src
"Andreas Gustafsson" writes: > Module Name: src > Committed By: gson > Date: Wed Feb 13 07:55:33 UTC 2019 > > Modified Files: > src/distrib/pmax/ramdisk: Makefile > src/sys/arch/pmax/conf: RAMDISK > > Log Message: > Bump pmax install ramdisk size by another 100k, as 3500k is no longer > enough with GCC 7. FYI: i might have broken this again with the eh_frame non-removal commit to crunchgen -- it will leave some bloat in place, but we should be removing that sooner than later, so the bloat will be removed again. if we have to bump it again, i'll try to remember to check unbumping this as well. .mrg.
Re: DIAGNOSTIC for modules (Re: CVS commit: src/sys/dev/usb)
On 2019/02/13 21:13, Christos Zoulas wrote: In article , Rin Okuyama wrote: Hi, On 2019/02/13 6:07, Paul Goyette wrote: 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 ?? Thank you for your comment. Well, I examined that (both for GCC7 & clang). Among ~ 360 modules, - 2 (lua and zfs) need WARNS=0 - 1 (solaris) needs WARNS=1 - 136 need WARNS=3 (mostly due to sign-compare) - 4 need WARNS=4 - Others can be compiled with WARNS=5 I propose this patch: http://www.netbsd.org/~rin/modules_bump_warns_20190213.patch - Bump default value of WARNS for modules from 3 to 5 - Explicitly set WARNS for modules that fail with WARNS=5 - Then, expect someone in charge will fix them ;-) Thoughts? Go for it, we can fix the ones that don't come from 3rd party sources opportunistically. christos Thank you for your comment. I will commit it within a few days, if there's no objection. rin
Re: DIAGNOSTIC for modules (Re: CVS commit: src/sys/dev/usb)
In article , Rin Okuyama wrote: >Hi, > >On 2019/02/13 6:07, Paul Goyette wrote: >> 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 ?? > >Thank you for your comment. > >Well, I examined that (both for GCC7 & clang). Among ~ 360 modules, >- 2 (lua and zfs) need WARNS=0 >- 1 (solaris) needs WARNS=1 >- 136 need WARNS=3 (mostly due to sign-compare) >- 4 need WARNS=4 >- Others can be compiled with WARNS=5 > >I propose this patch: >http://www.netbsd.org/~rin/modules_bump_warns_20190213.patch > >- Bump default value of WARNS for modules from 3 to 5 >- Explicitly set WARNS for modules that fail with WARNS=5 >- Then, expect someone in charge will fix them ;-) > >Thoughts? Go for it, we can fix the ones that don't come from 3rd party sources opportunistically. christos
Re: CVS commit: src/usr.bin/crunch/crunchgen
In article <20190213065626.ga22...@mail.duskware.de>, Martin Husemann wrote: >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? I understand that, this is why I am saying that removing the code that triggers the bug, we can restore the varasm change that makes the segment readonly... We can then take our time (or have the gcc folks) fix the underlying issue. christos
Re: DIAGNOSTIC for modules (Re: CVS commit: src/sys/dev/usb)
On 2019/02/13 19:06, Paul Goyette wrote: 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 ?? Thank you for your comment. Well, I examined that (both for GCC7 & clang). Among ~ 360 modules, - 2 (lua and zfs) need WARNS=0 - 1 (solaris) needs WARNS=1 - 136 need WARNS=3 (mostly due to sign-compare) - 4 need WARNS=4 - Others can be compiled with WARNS=5 I propose this patch: http://www.netbsd.org/~rin/modules_bump_warns_20190213.patch - Bump default value of WARNS for modules from 3 to 5 - Explicitly set WARNS for modules that fail with WARNS=5 - Then, expect someone in charge will fix them ;-) I really appreciate the effort you expended on this! I really did not expect it. I would be happy to have your proposed patch committed, but I would prefer that we wait a bit so that other developers can express their opinions. Thanks :-). Yes, I will wait for input from others. rin
Re: CVS commit: src/sys/arch/x86
Le 13/02/2019 à 10:08, Cherry G.Mathew a écrit : (resent to source-changes-d@) "Maxime Villard" writes: - There is no recursive slot possible, so we can't use pmap_map_ptes(). Rather, we walk down the EPT trees via the direct map, and that's actually a lot simpler (and probably faster too...). Does this mean that nvmm hosts have to have __HAVE_DIRECT_MAP ? Yes, and all of them do in practice by default (GENERIC), so it's not a problem. It becomes a problem on certain special configurations such as KASAN that disable the direct map. In this case EPT is not compiled. So if you use KASAN but don't use NVMM+Intel there is no problem. In fact, maybe I should add direct map support in KASAN. Initially I didn't do it because I wanted to force all kernel allocations into pmap_kenter_pa, to have 100% KASAN coverage of the KVA. But if I was using two separate flags, such as __HAVE_DIRECT_MAP = whether the kernel has a direct map __USE_DIRECT_MAP = whether the allocators can use the direct map In KASAN we could have __HAVE_DIRECT_MAP=1 and __USE_DIRECT_MAP=0, meaning that EPT can use the direct map internally but nobody in UVM/etc can use the direct map for allocations. This would maintain KASAN coverage and would enable KASAN+EPT support. - The kernel is never mapped in an EPT pmap. An EPT pmap cannot be loaded on the host. This has two sub-consequences: at creation time we must zero out all of the top-level PTEs, and at destruction time we force the page out of the pool cache and into the pool, to ensure that a next allocation will invoke pmap_pdp_ctor() to create a native pmap and not recycle some stale EPT entries. Can you not use a separate poolcache ? This could isolate host/guest related memory pressure as well ? The poolcache I was talking about is the one that stores the top-level page of the page tables (PML4). It is only a 4KB page, and there is only one such page per guest. Therefore there is no need to separate more, one page per guest is pretty insignificant. However, it is true that we could separate the caches of the non-top-level page tables (L3, L2 and L1). But I think the interest is not huge: under pressure UVM will unmap guest pages, and when it happens, we also free the empty L3/L2/L1 pages, so the pressure is "naturally" reduced in the shared poolcaches.
Re: DIAGNOSTIC for modules (Re: CVS commit: src/sys/dev/usb)
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 ?? Thank you for your comment. Well, I examined that (both for GCC7 & clang). Among ~ 360 modules, - 2 (lua and zfs) need WARNS=0 - 1 (solaris) needs WARNS=1 - 136 need WARNS=3 (mostly due to sign-compare) - 4 need WARNS=4 - Others can be compiled with WARNS=5 I propose this patch: http://www.netbsd.org/~rin/modules_bump_warns_20190213.patch - Bump default value of WARNS for modules from 3 to 5 - Explicitly set WARNS for modules that fail with WARNS=5 - Then, expect someone in charge will fix them ;-) I really appreciate the effort you expended on this! I really did not expect it. I would be happy to have your proposed patch committed, but I would prefer that we wait a bit so that other developers can express their opinions. +--+--++ | 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: DIAGNOSTIC for modules (Re: CVS commit: src/sys/dev/usb)
Hi, On 2019/02/13 6:07, Paul Goyette wrote: 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 ?? Thank you for your comment. Well, I examined that (both for GCC7 & clang). Among ~ 360 modules, - 2 (lua and zfs) need WARNS=0 - 1 (solaris) needs WARNS=1 - 136 need WARNS=3 (mostly due to sign-compare) - 4 need WARNS=4 - Others can be compiled with WARNS=5 I propose this patch: http://www.netbsd.org/~rin/modules_bump_warns_20190213.patch - Bump default value of WARNS for modules from 3 to 5 - Explicitly set WARNS for modules that fail with WARNS=5 - Then, expect someone in charge will fix them ;-) Thoughts? Thanks, rin
Re: Multiple outstanding transfer vs xhci / ehci (Re: CVS commit: src/sys/dev/usb)
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?
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 13 Feb 2019 09:32:00 -
@@ -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_status == USBD_NOT_STARTED) {
+ SIMPLEQ_REMOVE_HEAD(>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;
Re: CVS commit: src/sys/arch/x86
(resent to source-changes-d@) "Maxime Villard" writes: [...] > > Contrary to AMD-SVM, Intel-VMX uses a different set of PTE bits from > native, and this has three important consequences: > > - We can't use the native PTE bits, so each time we want to modify the >page tables, we need to know whether we're dealing with a native pmap >or an EPT pmap. This is accomplished with callbacks, that handle >everything PTE-related. > I like this approach - perhaps it's a way to separate out other similar pmaps (OT). > - There is no recursive slot possible, so we can't use pmap_map_ptes(). >Rather, we walk down the EPT trees via the direct map, and that's >actually a lot simpler (and probably faster too...). > Does this mean that nvmm hosts have to have __HAVE_DIRECT_MAP ? If so, it might be worth having a separate kernel conf rather than GENERIC (I don't know how this works now). I recently built an amd64 kernel without __HAVE_DIRECT_MAP and was quite impressed that it actually booted. That's a nice to have feature. > - The kernel is never mapped in an EPT pmap. An EPT pmap cannot be loaded >on the host. This has two sub-consequences: at creation time we must >zero out all of the top-level PTEs, and at destruction time we force >the page out of the pool cache and into the pool, to ensure that a next >allocation will invoke pmap_pdp_ctor() to create a native pmap and not >recycle some stale EPT entries. Can you not use a separate poolcache ? This could isolate host/guest related memory pressure as well ? -- ~cherry
