Re: svn commit: r325504 - head/lib/libclang_rt

[Ngie Cooper (yaneurabeya)]

> On Nov 7, 2017, at 11:20, Bryan Drewery  wrote:
> 
> On 11/6/2017 10:26 PM, Ngie Cooper wrote:
>> Author: ngie
>> Date: Tue Nov  7 06:26:48 2017
>> New Revision: 325504
>> URL: https://svnweb.freebsd.org/changeset/base/325504
>> 
>> Log:
>>  Redo r325502
>> 
>>  :U:Mfoo expands to :Mfoo, apparently. Explicit check for CPUTYPE being
>>  defined, and test for it's value not containing *soft* before calling 
>> CRTARCH
>>  armhf.
>> 
>>  Tested, somewhat. Unfortunately recent changes appear to have affected
>>  cross-builds where it no longer works, per my tests after universe12a being
>>  upgraded from 07/2017 to 11/2017 sources (DESTDIR isn't being used in 
>> WORLDTMP;
>>  MK_SYSTEM_COMPILER might be causing issues right now).
>> 
> 
> In buildworld DESTDIR usually is set to WORLDTMP.  But how does the
> comment relate to CPUTYPE?  What "no longer works" about cross-builds?

If I had my log, I would dig it up, but it’s easy to trigger with “make 
tinderbox TARGETS=arm” on that branch :/.

Basically, it wasn’t looking for libraries in the host toolchain’s directory 
(`/usr/lib/…`), not `/usr/lib/…`.

Cheers,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r325443 - in head: . lib/libclang_rt share/mk

[Ngie Cooper (yaneurabeya)]

> On Nov 5, 2017, at 11:38, Ngie Cooper  wrote:
> 
> Author: ngie
> Date: Sun Nov  5 19:38:51 2017
> New Revision: 325443
> URL: https://svnweb.freebsd.org/changeset/base/325443
> 
> Log:
>  Fix paths for cross-built versions of lib/libclang_rt and hardfloat arm 
> variants
> 
>  - Define TARGET_CPUARCH and use in libclang_rt as the basis for CRTARCH
> 
>When cross-compiling, the wrong architecture was being embedded in the
>libclang_rt binary filenames. It should be based on TARGET_ARCH (target), 
> not
>MACHINE_ARCH (host).
> 
>If TARGET_ARCH isn't defined (host-builds), fallback to MACHINE_ARCH.
> 
>  - Define CRTARCH to armhf when TARGET/TARGET_ARCH are set to arm/armv[67]
> 
>TARGET_ABI/TARGET_CPU in Makefile.inc1 sets the ABI to gnueabihf, which
>affects the clang lookup path per `getArchNameForCompilerRTLib(..)` in
>contrib/llvm/tools/clang/lib/Driver/ToolChain.cpp, so chase clang and
>Linux's assumed naming convention for hard-float arm architectures.
> 
>CROSSENV (in Makefile.inc1) sets CPUTYPE/MACHINE(_ARCH)? to the
>TARGET*-relevant values when building the `libraries` target, so test
>those variables instead.
> 
>  - Add OLD_FILES/OLD_LIBS entries for TARGET/TARGET_ARCH == arm/armv[67]. This
>impacts only arm/armv6 and arm/armv7.
> 
>  PR:  222925

MFC after:  2 weeks


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r325442 - in head/lib/libclang_rt: . profile

[Ngie Cooper (yaneurabeya)]

> On Nov 5, 2017, at 11:25, Ngie Cooper  wrote:
> 
> Author: ngie
> Date: Sun Nov  5 19:25:06 2017
> New Revision: 325442
> URL: https://svnweb.freebsd.org/changeset/base/325442
> 
> Log:
>  Support compiling lib/libclang_rt/profile a bit more with recent versions of 
> gcc
> 
>  Several of the flags were being treated as CFLAGS, when they were actually
>  technically CXXFLAGS. Move them there.
> 
>  Only apply -fno-sanitize=safe-stack with clang.
> 
>  PR: 223179

MFC after:  2 weeks


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r325116 - head

[Ngie Cooper (yaneurabeya)]

> On Nov 4, 2017, at 08:31, Bryan Drewery <bdrew...@freebsd.org> wrote:
> 
> On 11/4/2017 12:08 AM, Ngie Cooper (yaneurabeya) wrote:
>>  I just ran into this with bash on universe12a.freebsd.org by doing ^d. 
>> I think the change should be reverted.
>> Thanks,
>> -Ngie
>> 
>> $ exit
>> *** Error code 130
>> 
>> Stop.
>> make[1]: stopped in /scratch/tmp/ngie/svn
>> *** Error code 1
>> 
>> Stop.
>> make: stopped in /scratch/tmp/ngie/svn
>> 
> 
> When you first go into buildenv check 'echo $?'.
> My problem was a construct like '[ -f xxx ] && . xxx' at the end of one
> of my profile scripts.  It needs to be 'if [ -f xxx ]; then . xxx; fi'
> to avoid the bad status.  It's really unrelated to buildenv.

The last line in my ~/.bashrc on my VM is export PATH that doesn’t fail with a 
syntax error, and the last line on my ~/.bashrc on universe12a is an alias set, 
which also doesn’t fail with a syntax error. Both cases set $? = 0.

The issue I reported is caused by doing ^c + ^d (hence the error code 130 in my 
original reply). I guess this is expected, but it’s going to confuse people and 
force them to have to investigate the issue, unnecessarily, when it wasn’t an 
“issue” before.

-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r325116 - head

[Ngie Cooper (yaneurabeya)]

I just ran into this with bash on universe12a.freebsd.org by doing ^d. 
I think the change should be reverted.
Thanks,
-Ngie

$ exit
*** Error code 130

Stop.
make[1]: stopped in /scratch/tmp/ngie/svn
*** Error code 1

Stop.
make: stopped in /scratch/tmp/ngie/svn



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r325062 - in head/lib: . libcasper libcasper/libcasper libcasper/services libcasper/services/cap_dns libcasper/services/cap_dns/tests libcasper/services/cap_grp libcasper/services/cap_

[Ngie Cooper (yaneurabeya)]

> On Oct 28, 2017, at 12:23, Mariusz Zaborski  wrote:
> 
> Author: oshogbo
> Date: Sat Oct 28 19:23:57 2017
> New Revision: 325062
> URL: https://svnweb.freebsd.org/changeset/base/325062
> 
> Log:
>  Introduce caspermocks.
> 
>  The idea behinds mocks is that we don't need to ifdef a lot of code in
>  tools itself but those defines are hidden in the casper library.
>  Right now the mocks are implemented as define/inlines functions.
>  There was a very long discussion how this should be implemented.
>  This approach has some advantages like we don't need to link to any 
> additional
>  libraries. Unfortunately there are also some disadvantages for example it is
>  easy to get library out of sync between two versions of functions or that we
>  need extra define to compile program with casper support.
>  This isn't an ideal solution but it's good enough for now and should simplify
>  capsicumizing programs. This also doesn't close us any other ways to do those
>  mocks and this should evolve in time.
> 
>  Discussed with:  pjd, emaste, ed, rwatson, bapt, cem, bdrewery
>  Differential Revision:https://reviews.freebsd.org/D8753

This change broke tinderbox/Jenkins for powerpc and sparc64: 
https://ci.freebsd.org/job/FreeBSD-head-powerpc-build/1865/console .
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322951 - head

[Ngie Cooper (yaneurabeya)]

> On Oct 25, 2017, at 13:34, Bryan Drewery  wrote:

…

> I don't understand what you're saying here. If I build with MK_TCSH=yes
> and the host does not have tcsh, a failure occurs AFTER or BEFORE this
> change?
> Neither case makes sense to me.
> What is the failure?
> 
> The change here is to build-tools which just calls 'make build-tools' in
> bin/csh which just builds 'gethost'.

Yes. The relnotes concerns I mentioned are bogus.

Yeah, a ~400 line C file isn’t that bad, I agree.

I really don’t care about arguing this or debating it. Yes, you’re right on all 
counts. It’s a micro optimization.


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324890 - head/sys/kern

[Ngie Cooper (yaneurabeya)]

> On Oct 22, 2017, at 18:00, Mateusz Guzik  wrote:
> 
> Author: mjg
> Date: Mon Oct 23 01:00:35 2017
> New Revision: 324890
> URL: https://svnweb.freebsd.org/changeset/base/324890
> 
> Log:
>  Bump WITNESS_PENDLIST to accomodate sleepq chain bump.
> 
>  Reported by: ngie

*crosses fingers and waits for next Jenkins run*


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324870 - in head/sys: amd64/include kern

[Ngie Cooper (yaneurabeya)]

> On Oct 22, 2017, at 13:43, Mateusz Guzik  wrote:
> 
> Author: mjg
> Date: Sun Oct 22 20:43:50 2017
> New Revision: 324870
> URL: https://svnweb.freebsd.org/changeset/base/324870
> 
> Log:
>  Make the sleepq chain hash size configurable per-arch and bump on amd64.
> 
>  While here cache-align chains.
> 
>  This shortens longest found chain during poudriere -j 80 from 32 to 16.
> 
>  Pushing this higher up will probably require allocation on boot.

Hi Mateusz,
This change causes the Jenkins VMs to panic at boot with "panic: 
witness_init: pending locks list is too small, increase WITNESS_PENDLIST” when 
WITNESS is enabled: 
https://ci.freebsd.org/job/FreeBSD-head-amd64-test/4781/console .
Please fix or revert.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324863 - in head/sys: kern sys

[Ngie Cooper (yaneurabeya)]

> On Oct 22, 2017, at 06:42, Mateusz Guzik  wrote:
> 
> Author: mjg
> Date: Sun Oct 22 13:42:56 2017
> New Revision: 324863
> URL: https://svnweb.freebsd.org/changeset/base/324863
> 
> Log:
>  Change kdb_active type to u_char.
> 
>  Fixes warnings from gcc and keeps the small size. Perhaps nesting should be 
> moved
>  to another variablle.
> 
>  Reported by: ngie

Thank you for fixing this.

Question though — since u_char is unsigned, are you concerned about underflow 
(-1 -> 255)?

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324789 - in head/sys: kern sys

[Ngie Cooper (yaneurabeya)]

> On Oct 19, 2017, at 21:02, Mateusz Guzik  wrote:
> 
> Author: mjg
> Date: Fri Oct 20 04:02:53 2017
> New Revision: 324789
> URL: https://svnweb.freebsd.org/changeset/base/324789
> 
> Log:
>  Mark kdb_active as __read_frequently and switch to bool to eat less space.
> 
> Modified:
>  head/sys/kern/subr_kdb.c
>  head/sys/sys/kdb.h
> 
> Modified: head/sys/kern/subr_kdb.c
> ==
> --- head/sys/kern/subr_kdb.c  Fri Oct 20 03:38:58 2017(r324788)
> +++ head/sys/kern/subr_kdb.c  Fri Oct 20 04:02:53 2017(r324789)
> @@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
> #include 
> #endif
> 
> -int kdb_active = 0;
> +bool __read_frequently kdb_active = 0;
> static void *kdb_jmpbufp = NULL;
> struct kdb_dbbe *kdb_dbbe = NULL;
> static struct pcb kdb_pcb;
> 
> Modified: head/sys/sys/kdb.h
> ==
> --- head/sys/sys/kdb.hFri Oct 20 03:38:58 2017(r324788)
> +++ head/sys/sys/kdb.hFri Oct 20 04:02:53 2017(r324789)
> @@ -59,7 +59,7 @@ struct kdb_dbbe {
>   };  \
>   DATA_SET(kdb_dbbe_set, name##_dbbe)
> 
> -extern int kdb_active;   /* Non-zero while in debugger. 
> */
> +extern bool kdb_active;  /* Non-zero while in debugger. 
> */
> extern int debugger_on_panic; /* enter the debugger on panic. */
> extern struct kdb_dbbe *kdb_dbbe; /* Default debugger backend or NULL. */
> extern struct trapframe *kdb_frame;   /* Frame to kdb_trap(). */

This broke `kdb_active` use in `kdb_trap`. It’s noticed by gcc on 
riscv64: https://ci.freebsd.org/job/FreeBSD-head-riscv64-build/4356/console .
Please fix or revert.
Thanks,
-Ngie



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324835 - head/etc

[Ngie Cooper (yaneurabeya)]

> On Oct 21, 2017, at 14:58, Brad Davis  wrote:
> 
> Author: brd (doc,ports committer)
> Date: Sat Oct 21 21:58:24 2017
> New Revision: 324835
> URL: https://svnweb.freebsd.org/changeset/base/324835
> 
> Log:
>  Remove a atrun check that is nullified by r318443.
> 
>  Approved by: will

Thanks :).
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324811 - in head/sys/dev/pms/freebsd/driver: common ini/src

[Ngie Cooper (yaneurabeya)]

> On Oct 21, 2017, at 00:23, Ryan Libby  wrote:
> 
> Author: rlibby
> Date: Sat Oct 21 07:23:45 2017
> New Revision: 324811
> URL: https://svnweb.freebsd.org/changeset/base/324811
> 
> Log:
>  pms/freebsd: fix compiler warnings
> 
>   - A number of unused variable warnings,
>   - a missing prototype warning (actually a dead function),
>   - and a potential use of an uninitialized variable.

You probably should look for the other _fantastic_ fixes that Danny Ryan cooked 
up for the driver. I don’t think you’ve found the branches that result in logic 
errors which trigger corruption yet…
Cheers,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324666 - head/sys/kern

[Ngie Cooper (yaneurabeya)]

> On Oct 16, 2017, at 09:14, Matt Joras  wrote:
> 
> Author: mjoras
> Date: Mon Oct 16 16:14:50 2017
> New Revision: 324666
> URL: https://svnweb.freebsd.org/changeset/base/324666
> 
> Log:
>  Properly reset the fields in clean_unrhdr.
> 
>  In r324542 I neglected to reset the first and last fields of struct
>  unrhdr. This causes a tmpfs to fail the unr(9) consistency checks with
>  DIAGNOSTIC on. Fix this by resetting the fields by calling init_unrhdr.
>  While here, change a loop to use TAILQ_FOREACH_SAFE since it is more
>  readable and equally fast.

Thanks!


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324541 - in head: share/man/man9 sys/kern sys/sys

[Ngie Cooper (yaneurabeya)]

> On Oct 11, 2017, at 14:53, Matt Joras  wrote:
> 
> Author: mjoras
> Date: Wed Oct 11 21:53:50 2017
> New Revision: 324541
> URL: https://svnweb.freebsd.org/changeset/base/324541
> 
> Log:
>  Add clearing function for unr(9).
> 
>  Previously before you could call unrhdr_delete you needed to
>  individually free every allocated unit. It is useful to be able to tear
>  down the unr without having to go through this process, as it is
>  significantly faster than freeing the individual units.
> 
>  Reviewed by: cem, lidl
>  Approved by: rstone (mentor)
>  Sponsored by:Dell EMC Isilon
>  Differential Revision:   https://reviews.freebsd.org/D12591
> 
> Modified:
>  head/share/man/man9/Makefile
>  head/share/man/man9/unr.9
>  head/sys/kern/subr_unit.c
>  head/sys/sys/systm.h
> 
> Modified: head/share/man/man9/Makefile
> ==
> --- head/share/man/man9/Makefile  Wed Oct 11 20:36:22 2017
> (r324540)
> +++ head/share/man/man9/Makefile  Wed Oct 11 21:53:50 2017
> (r324541)
> @@ -414,6 +414,7 @@ MAN=  accept_filter.9 \
> MLINKS=   unr.9 alloc_unr.9 \
>   unr.9 alloc_unrl.9 \
>   unr.9 alloc_unr_specific.9 \
> + unr.9 clear_unrhdr.9 \
>   unr.9 delete_unrhdr.9 \
>   unr.9 free_unr.9 \
>   unr.9 new_unrhdr.9
> 
> Modified: head/share/man/man9/unr.9
> ==
> --- head/share/man/man9/unr.9 Wed Oct 11 20:36:22 2017(r324540)
> +++ head/share/man/man9/unr.9 Wed Oct 11 21:53:50 2017(r324541)
> @@ -24,11 +24,12 @@
> .\"
> .\" $FreeBSD$
> .\"
> -.Dd July 5, 2010
> +.Dd October 4, 2017
> .Dt UNR 9
> .Os
> .Sh NAME
> .Nm new_unrhdr ,
> +.Nm clear_unrhdr ,
> .Nm delete_unrhdr ,
> .Nm alloc_unr ,
> .Nm alloc_unr_specific ,
> @@ -39,6 +40,8 @@
> .Ft "struct unrhdr *"
> .Fn new_unrhdr "int low" "int high" "struct mtx *mutex"
> .Ft void
> +.Fn clear_unrhdr "struct unrhdr *uh"
> +.Ft void
> .Fn delete_unrhdr "struct unrhdr *uh"
> .Ft int
> .Fn alloc_unr "struct unrhdr *uh"
> @@ -70,8 +73,16 @@ is not
> .Dv NULL ,
> it is used for locking when allocating and freeing units.
> Otherwise, internal mutex is used.
> +.It Fn clear_unrhdr uh
> +Clear all units from the specified unit number allocator entity.
> +This function resets the entity as if it were just initialized with
> +.Fn new_unrhdr .
> .It Fn delete_unrhdr uh
> -Destroy specified unit number allocator entity.
> +Delete specified unit number allocator entity.
> +This function frees the memory associated with the entity, it does not free
> +any units.
> +To free all units use
> +.Fn clear_unrhdr .
> .It Fn alloc_unr uh
> Return a new unit number.
> The lowest free number is always allocated.
> 
> Modified: head/sys/kern/subr_unit.c
> ==
> --- head/sys/kern/subr_unit.c Wed Oct 11 20:36:22 2017(r324540)
> +++ head/sys/kern/subr_unit.c Wed Oct 11 21:53:50 2017(r324541)
> @@ -366,6 +366,27 @@ delete_unrhdr(struct unrhdr *uh)
>   Free(uh);
> }
> 
> +void
> +clear_unrhdr(struct unrhdr *uh)
> +{
> + struct unr *up, *uq;
> +
> + KASSERT(TAILQ_EMPTY(>ppfree),
> + ("unrhdr has postponed item for free"));
> + up = TAILQ_FIRST(>head);
> + while (up != NULL) {

Could this be done with TAILQ_FOREACH_SAFE?
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324471 - head/sys/boot

[Ngie Cooper (yaneurabeya)]

> On Oct 9, 2017, at 21:56, Warner Losh  wrote:
> 
> DO NOT MAKE EDITS TO sys/boot. YOU ARE CREATING CONFLICTS FOR ME.
> 
> DO NOT MAKE ANY COMMITS TO sys/boot.
> 
> BACK OFF.
> 
> Seriously, though, extra changes create extra friction, and these changes 
> aren't worth any friction at all. I'm deleting LIBSAU and this guarantees a 
> conflict when I update.
> 
> So please, do not make any edits to sys/boot whatsoever, no matter how 
> trivial.
> 
> At least until I'm done.

Ok


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324452 - in head/sys/boot: . arm/uboot efi/boot1 efi/loader i386/gptboot i386/gptzfsboot i386/loader i386/zfsboot mips/beri/boot2 mips/beri/loader mips/uboot powerpc/kboot powerpc/ofw

[Ngie Cooper (yaneurabeya)]

> On Oct 9, 2017, at 16:23, John Baldwin  wrote:

…

> s/along/alone/.  bde@ probably prefers that the comments start with a capital
> letter as well.

Also: standalone is one word, not 2.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324406 - head/sys/modules/em

[Ngie Cooper (yaneurabeya)]

> On Oct 9, 2017, at 18:54, Ian Lepore  wrote:

...

> I think the right answer here is just
> 
>   LINKS= ${KMODDIR}/${KMOD}.ko ${KMODDIR}/if_igb.ko

That will definitely work and is probably the best pattern.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324360 - in head/sys/boot/efi: include libefi loader

[Ngie Cooper (yaneurabeya)]

> On Oct 7, 2017, at 09:29, Guy Yur  wrote:
> 
> On 6 October 2017 at 15:21, Warner Losh  wrote:
>> Author: imp
>> Date: Fri Oct  6 12:21:46 2017
>> New Revision: 324360
>> URL: https://svnweb.freebsd.org/changeset/base/324360
>> 
>> Log:
>>  Encapsulate  ZFS preferences into efi_zfs_is_preferred
>> 
>>  Move the retrieval of the image information into loader's main instead
>>  of doing it in efizfs.c
>> 
>>  Differential Revision: https://reviews.freebsd.org/D12564
>>  Submitted by: Eric McCorkle
>> 
>> Modified:
>>  head/sys/boot/efi/include/efizfs.h
>>  head/sys/boot/efi/libefi/efizfs.c
>>  head/sys/boot/efi/loader/main.c
>> 
>> ...
> 
> Hi,
> 
> When setting WITHOUT_ZFS in src.conf the build fails with:
> 
> /usr/src/sys/boot/efi/loader/main.c:78:1: error: no previous prototype
> for function 'efi_zfs_is_preferred'
>  [-Werror,-Wmissing-prototypes]
> efi_zfs_is_preferred(EFI_HANDLE *h)
> ^
> 1 error generated.
> *** Error code 1
> 
> 
> The prototype is under #ifdef EFI_ZFS_BOOT guard but the function is not.

Fixed: r324388.
Thanks!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r324109 - in head: sys/contrib/dev/acpica sys/contrib/dev/acpica/common sys/contrib/dev/acpica/compiler sys/contrib/dev/acpica/components/dispatcher sys/contrib/dev/acpica/components/h

[Ngie Cooper (yaneurabeya)]

> On Sep 30, 2017, at 09:16, Konstantin Belousov  wrote:

Given the number of panic reports, could the commit be reverted and a 
fix be submitted later so others can develop on ^/head?
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r323935 - head/sys/kern

[Ngie Cooper (yaneurabeya)]

> On Sep 22, 2017, at 13:01, Conrad Meyer  wrote:
> 
> Author: cem
> Date: Fri Sep 22 20:01:12 2017
> New Revision: 323935
> URL: https://svnweb.freebsd.org/changeset/base/323935
> 
> Log:
>  ddb(4): Add 'show badstacks' command to show witness badstacks
> 
>  Add a DDB command that mirrors sysctl debug.witness.badstacks.
> 
>  Reported by: rstone
>  Reviewed by: rstone
>  Sponsored by:Dell EMC Isilon
>  Differential Revision:   https://reviews.freebsd.org/D12468

This broke GENERIC:

--- subr_witness.o ---
/usr/src/sys/kern/subr_witness.c:2577:4: error: use of undeclared identifier 
'req'
req->oldidx = 0;
^
/usr/src/sys/kern/subr_witness.c:2603:5: error: use of undeclared identifier 
'req'
req->oldidx = 0;
^
/usr/src/sys/kern/subr_witness.c:2662:3: error: use of undeclared identifier 
'req'
req->oldidx = 0;
^


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r323899 - head/tests/sys/opencrypto

[Ngie Cooper (yaneurabeya)]

> On Sep 21, 2017, at 21:41, Conrad Meyer  wrote:
> 
> Author: cem
> Date: Fri Sep 22 04:41:48 2017
> New Revision: 323899
> URL: https://svnweb.freebsd.org/changeset/base/323899
> 
> Log:
>  cryptotest.py: Like r323869, skip SHA HMAC tests on non-SHA drivers
> 
>  Sponsored by:Dell EMC Isilon
> 
> Modified:
>  head/tests/sys/opencrypto/cryptotest.py
> 
> Modified: head/tests/sys/opencrypto/cryptotest.py
> ==
> --- head/tests/sys/opencrypto/cryptotest.py   Fri Sep 22 04:25:44 2017
> (r323898)
> +++ head/tests/sys/opencrypto/cryptotest.py   Fri Sep 22 04:41:48 2017
> (r323899)
> @@ -237,6 +237,7 @@ def GenTestCase(cname):
>   #for i in iglob('SHA1*'):
>   #   self.runSHA(i)
> 
> + @unittest.skipIf(cname not in shamodules, 'skipping SHA on %s' 
> % `name`)

Backticks for stringily’ing things isn’t py3k compatible, FYI.
-Ngie



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r323770 - in stable/11/sys: amd64/conf arm64/conf i386/conf powerpc/conf riscv/conf sparc64/conf

[Ngie Cooper (yaneurabeya)]

> On Sep 19, 2017, at 09:51, Josh Paetzel  wrote:
> 
> Author: jpaetzel
> Date: Tue Sep 19 16:51:51 2017
> New Revision: 323770
> URL: https://svnweb.freebsd.org/changeset/base/323770
> 
> Log:
>  MFC: 323068
> 
>Allow kldload tcpmd5

Wasn’t this reverted on ^/head ?
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r323635 - in head/sys: dev/bnxt dev/e1000 kern net sys

[Ngie Cooper (yaneurabeya)]

> On Sep 15, 2017, at 19:41, Stephen Hurd  wrote:
> 
> Author: shurd
> Date: Sat Sep 16 02:41:38 2017
> New Revision: 323635
> URL: https://svnweb.freebsd.org/changeset/base/323635
> 
> Log:
>  Revert r323516 (iflib rollup)
> 
>  This was really too big of a commit even if everything worked, but there
>  are multiple new issues introduced in the one huge commit, so it's not
>  worth keeping this until it's fixed.
> 
>  I'll work on splitting this up into logical chunks and introduce them one
>  at a time over the next week or two.

Thank you,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r323365 - in head: cddl/lib/libzpool gnu/usr.bin/binutils/libbfd usr.bin/svn/lib/libsvn_client

[Ngie Cooper (yaneurabeya)]

> On Sep 9, 2017, at 14:51, Konstantin Belousov  wrote:

…

> Then, why peigen.c was left in the SRCS ?

I’m not sure, but you might have thought peigen.c and pepigen.c were the same 
file. If so, they’re not — they have slightly different spelling.
HTH,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r323365 - in head: cddl/lib/libzpool gnu/usr.bin/binutils/libbfd usr.bin/svn/lib/libsvn_client

[Ngie Cooper (yaneurabeya)]

> On Sep 9, 2017, at 11:35, Konstantin Belousov  wrote:
> 
> On Sat, Sep 09, 2017 at 01:18:33PM +, Jonathan Anderson wrote:
>> Author: jonathan
>> Date: Sat Sep  9 13:18:32 2017
>> New Revision: 323365
>> URL: https://svnweb.freebsd.org/changeset/base/323365
>> 
>> Log:
>>  Remove redundant source and object files.
> 
>> Modified: head/gnu/usr.bin/binutils/libbfd/Makefile.amd64
>> ==
>> --- head/gnu/usr.bin/binutils/libbfd/Makefile.amd64  Sat Sep  9 12:50:12 
>> 2017(r323364)
>> +++ head/gnu/usr.bin/binutils/libbfd/Makefile.amd64  Sat Sep  9 13:18:32 
>> 2017(r323365)
>> @@ -9,7 +9,6 @@ DEFAULT_VECTOR=  bfd_elf64_x86_64_freebsd_vec
>> 
>> SRCS+=   elf64-x86-64.c \
>>  efi-app-x86_64.c \
>> -efi-app-ia32.c \
>>  elf64.c \
>>  elf64-gen.c \
>>  elf64-target.h \
> Why did you claimed that x86 efi binfmt is  redundant ?

It’s redundant because of how the Makefile gets .include’d (amd64 
requires i386’s Makefile).
Cheers,
-Ngie

$ grep -r efi-app gnu/usr.bin/binutils/libbfd/
gnu/usr.bin/binutils/libbfd/Makefile.amd64: efi-app-x86_64.c \
gnu/usr.bin/binutils/libbfd/Makefile.amd64: efi-app-ia32.c \
gnu/usr.bin/binutils/libbfd/Makefile.i386:  efi-app-ia32.c \
$ grep include gnu/usr.bin/binutils/libbfd/Makefile.amd64
.include "${.CURDIR}/Makefile.i386”


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r323329 - head/sys/sys

[Ngie Cooper (yaneurabeya)]

> On Sep 8, 2017, at 13:09, Mateusz Guzik  wrote:
> 
> Author: mjg
> Date: Fri Sep  8 20:09:14 2017
> New Revision: 323329
> URL: https://svnweb.freebsd.org/changeset/base/323329
> 
> Log:
>  Allow __builtin_memset instead of bzero for small buffers of known size
> 
>  In particular this eliminates function calls and related register 
> save/restore
>  when only few writes would suffice.
> 
>  Example speed up can be seen in a fstat microbenchmark on AMD Ryzen cpus, 
> where
>  the throughput went up by ~4.5%.
> 
>  Thanks to cem@ for benchmarking and reviewing the patch.

This change breaks all platforms that use gcc. If it’s not reverted in the next 
3 hours, I’ll revert it.

Please fix ASAP if possible: 
https://ci.freebsd.org/job/FreeBSD-head-riscv64-build/3668/console .

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322952 - head/share/mk

[Ngie Cooper (yaneurabeya)]

> On Aug 27, 2017, at 09:57, Conrad Meyer  wrote:
> 
> This appears to be breaking the build:
> 
> --
 stage 1.1: legacy release compatibility shims
> --
> ===> tools/build (obj,includes,all,install)
> make[5]: "/home/cem/head.svn/share/mk/bsd.lib.mk" line 79: Malformed
> conditional (${MK_COVERAGE} != "no" && (!empty(CFLAGS:M-g*) ||
> !empty(CXXFLAGS:M-g*)))
> make[5]: Fatal errors encountered -- cannot continue
> make[5]: stopped in /home/cem/head.svn/tools/build

The svn switch didn’t complete successfully — great. I’ll fix this shortly.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322770 - head/usr.sbin/chown/tests

[Ngie Cooper (yaneurabeya)]

> On Aug 21, 2017, at 13:23, Glen Barber  wrote:
> 
> Author: gjb
> Date: Mon Aug 21 20:23:05 2017
> New Revision: 322770
> URL: https://svnweb.freebsd.org/changeset/base/322770
> 
> Log:
>  Apply changes from bin/chmod/tests/chmod_test.sh (r321949, r321950,
>  and r322101), adding atf_expect_fail() before chflags(8) is invoked
>  if the filesystem is ZFS, which does not support UF_IMMUTABLE.
> 
>  MFC after:   3 days
>  Sponsored by:The FreeBSD Foundation

Ah, thanks :)!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322759 - stable/10/bin/chmod/tests

[Ngie Cooper (yaneurabeya)]

> On Aug 21, 2017, at 10:20, Glen Barber  wrote:
> 
> Author: gjb
> Date: Mon Aug 21 17:20:31 2017
> New Revision: 322759
> URL: https://svnweb.freebsd.org/changeset/base/322759
> 
> Log:
>  MFC r321949, r321950, r322101:

Thanks Glen!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322678 - in head/usr.sbin/pw: . tests

[Ngie Cooper (yaneurabeya)]

> On Aug 18, 2017, at 17:32, Ed Maste  wrote:
> 
> Author: emaste
> Date: Sat Aug 19 00:32:26 2017
> New Revision: 322678
> URL: https://svnweb.freebsd.org/changeset/base/322678
> 
> Log:
>  pw useradd: Validate the user name before creating the entry
> 
>  Previouly it was possible to create users with spaces in the name with:
>  pw useradd -u 1234 -g 1234 -n 'test user'
> 
>  The "-g 1234" is relevant, without it the name was already rejected
>  as expected:
> 
>  [fk@test ~]$ sudo pw useradd -u 1234 -n 'test user'
>  pw: invalid character ` ' at position 4 in userid/group name
> 
>  Bug unintentionally found with a salt config without explicit name entry:
> 
>  test user:
>user.present:
>  - uid: 1234
>  - gid: 1234
>  - fullname: Test user
>  - shell: /usr/local/bin/bash
>  - home: /home/test
>  - groups:
>- wheel
>- salt
> 
>  "Luckily" salt modules rarely bother with input validation either ...
> 
>  PR:  221416
>  Submitted by:Fabian Keil
>  Obtained from:   ElectroBSD
>  MFC after:   1 week
> 
> Modified:
>  head/usr.sbin/pw/pw_user.c
>  head/usr.sbin/pw/tests/pw_useradd_test.sh

Usernames with passwords are permitted in some cases, e.g., AD.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322620 - svnadmin/conf

[Ngie Cooper (yaneurabeya)]

> On Aug 17, 2017, at 07:40, Matthew Seaman  wrote:
> 
> Author: matthew (ports committer)
> Date: Thu Aug 17 14:40:48 2017
> New Revision: 322620
> URL: https://svnweb.freebsd.org/changeset/base/322620
> 
> Log:
>  Take in ache's commit bit.
> 
>  requiescat in pace
> 
>  Approved by: core (implicit)

Hi Matthew,
ache’s death should be commemorated in 
usr.bin/calendar/calendars/calendar.freebsd and in docs as well. I would do it 
myself but I don’t know where he passed away :’(.
Thank you,
-Ngie

PS RIP Andrey — you’ll be missed :(.


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322418 - head/lib/msun/src

[Ngie Cooper (yaneurabeya)]

> On Aug 12, 2017, at 14:45, Ngie Cooper (yaneurabeya) <yaneurab...@gmail.com> 
> wrote:
> 
>> 
>> On Aug 12, 2017, at 10:55, Ryan Libby <rli...@freebsd.org> wrote:
> 
> …
> 
>> A few possible quick fixes:
>> 
>> - Change the {arm64,riscv,sparc64}/include/float.h LDBL_MAX_EXP
>>  definitions from (+16384) to 16384.
>> 
>> - Change HALF_LDBL_MAX from
>> #define HALF_LDBL_MAX __CONCAT(__CONCAT(0x0.8p, LDBL_MAX_EXP), L)
>>  to
>> #if LDBL_MAX_EXP != 0x4000
>> #error "Unsupported long double format"
>> #endif
>> #define HALF_LDBL_MAX 0x0.8p16384L
>> 
>> - Change the HALF_LDBL_MAX constant to a variable constructed with
>>  LD80C(), I think with
>> LD80C(0x8000, 16383, 5.9486574767861588254e+4931L)
>>  but this also requires #error for LDBL_MAX_EXP != 0x4000, so not
>>  really any better than the above.
>> 
>> I think I will back the patch out for now and go back to review.
> 
> The issue is an inconsistency with how LDBL_MAX_EXP is defined in the MD 
> headers:

* is an -> is due to an

> $ grep -r LDBL_MAX_EXP sys/sparc64/
> sys/sparc64/include/float.h:#define LDBL_MAX_EXP(+16384)
> $ grep -r LDBL_MAX_EXP sys/x86/
> sys/x86/include/float.h:#define LDBL_MAX_EXP16384

Here are the full gamut of definitions for LDBL_MAX_EXP. It seems that 
sparc64 isn’t the only architecture using this pattern:

$ grep -r DBL_MAX_EXP sys/*/include
sys/arm/include/float.h:#define DBL_MAX_EXP 1024
sys/arm/include/float.h:#define LDBL_MAX_EXPDBL_MAX_EXP
sys/arm64/include/float.h:#define   DBL_MAX_EXP 1024
sys/arm64/include/float.h:#define   LDBL_MAX_EXP(+16384)
sys/mips/include/float.h:#defineDBL_MAX_EXP 1024
sys/mips/include/float.h:#defineLDBL_MAX_EXPDBL_MAX_EXP
sys/powerpc/include/float.h:#define DBL_MAX_EXP 1024
sys/powerpc/include/float.h:#define LDBL_MAX_EXPDBL_MAX_EXP
sys/riscv/include/float.h:#define   DBL_MAX_EXP 1024
sys/riscv/include/float.h:#define   LDBL_MAX_EXP(+16384)
sys/sparc64/include/float.h:#define DBL_MAX_EXP 1024
sys/sparc64/include/float.h:#define LDBL_MAX_EXP(+16384)
sys/x86/include/float.h:#define DBL_MAX_EXP 1024
sys/x86/include/float.h:#define LDBL_MAX_EXP16384

It might also be an inconsistency with how clang vs gcc [4.2.1] handles 
__CONCAT, and what -std= flags are passed to ${CC} in the Makefile, since the 
implementation is predicated by whether or not it’s C++ or __STDC__ is defined. 
There might be an update that we can grab from NetBSD (since the macro 
originated there).
I don’t understand [right now] why the (+foo) form is used *shrugs*.
Cheers,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322418 - head/lib/msun/src

[Ngie Cooper (yaneurabeya)]

> On Aug 12, 2017, at 10:55, Ryan Libby  wrote:

…

> A few possible quick fixes:
> 
> - Change the {arm64,riscv,sparc64}/include/float.h LDBL_MAX_EXP
>   definitions from (+16384) to 16384.
> 
> - Change HALF_LDBL_MAX from
> #define HALF_LDBL_MAX __CONCAT(__CONCAT(0x0.8p, LDBL_MAX_EXP), L)
>   to
> #if LDBL_MAX_EXP != 0x4000
> #error "Unsupported long double format"
> #endif
> #define HALF_LDBL_MAX 0x0.8p16384L
> 
> - Change the HALF_LDBL_MAX constant to a variable constructed with
>   LD80C(), I think with
> LD80C(0x8000, 16383, 5.9486574767861588254e+4931L)
>   but this also requires #error for LDBL_MAX_EXP != 0x4000, so not
>   really any better than the above.
> 
> I think I will back the patch out for now and go back to review.

The issue is an inconsistency with how LDBL_MAX_EXP is defined in the MD 
headers:

$ grep -r LDBL_MAX_EXP sys/sparc64/
sys/sparc64/include/float.h:#define LDBL_MAX_EXP(+16384)
$ grep -r LDBL_MAX_EXP sys/x86/
sys/x86/include/float.h:#define LDBL_MAX_EXP16384

Cheers,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321369 - in head: . contrib/compiler-rt/include/sanitizer contrib/compiler-rt/include/xray contrib/compiler-rt/lib/asan contrib/compiler-rt/lib/builtins contrib/compiler-rt/lib/builti

[Ngie Cooper (yaneurabeya)]

> On Jul 22, 2017, at 04:08, Dimitry Andric  wrote:
> 
> Author: dim
> Date: Sat Jul 22 11:08:25 2017
> New Revision: 321369
> URL: https://svnweb.freebsd.org/changeset/base/321369
> 
> Log:
>  Upgrade our copies of clang, llvm, lld, lldb, compiler-rt and libc++ to
>  5.0.0 (trunk r308421).  Upstream has branched for the 5.0.0 release,
>  which should be in about a month.  Please report bugs and regressions,
>  so we can get them into the release.
> 
>  Please note that from 3.5.0 onwards, clang, llvm and lldb require C++11
>  support to build; see UPDATING for more information.
> 
>  MFC after:   2 months

Hi Dmitry,
This commit changed a lot of mergeinfo under contrib/ and sys/contrib 
unnecessarily — could you please revert those changes before merging back to 
^/head next time?
Thank you!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r322029 - in head: usr.bin usr.sbin

[Ngie Cooper (yaneurabeya)]

> On Aug 3, 2017, at 14:30, Jeremie Le Hen  wrote:
> 
> Author: jlh
> Date: Thu Aug  3 21:30:12 2017
> New Revision: 322029
> URL: https://svnweb.freebsd.org/changeset/base/322029
> 
> Log:
>  rwho/ruptime/rwhod shouldn't be gated by RCMDS.

The why in this commit and the next you made (r322031) would have been 
incredibly helpful. If bugzilla disappears I only have the commit history 
tracking the what.
Cheers,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321969 - in head/sys/boot: arm/at91/libat91 arm/ixp425/boot2 i386/boot2

[Ngie Cooper (yaneurabeya)]

> On Aug 3, 2017, at 00:53, Bruce Evans  wrote:
> 
> On Thu, 3 Aug 2017, Ngie Cooper wrote:
> 
>> Log:
>> Fix the return types for printf and putchar to match their libc and
>> POSIX equivalents
>> 
>> Both printf and putchar return int, not void.
>> 
>> This will allow code that leverages the libcalls and checks/rely on the
>> return type to interchangeably between loader code and non-loader
>> code.
>> 
>> MFC after:   1 month
>> 
>> Modified:
>> head/sys/boot/arm/at91/libat91/lib.h
>> head/sys/boot/arm/at91/libat91/printf.c
>> head/sys/boot/arm/at91/libat91/putchar.c
>> head/sys/boot/arm/ixp425/boot2/ixp425_board.c
>> head/sys/boot/arm/ixp425/boot2/lib.h
>> head/sys/boot/i386/boot2/boot2.c
> 
> This is wrong for at least i386/boot2.  It isn't part of the loader, and
> saves space by not returning unused values.
> 
>> Modified: head/sys/boot/i386/boot2/boot2.c
>> ==
>> --- head/sys/boot/i386/boot2/boot2.c Thu Aug  3 03:45:48 2017
>> (r321968)
>> +++ head/sys/boot/i386/boot2/boot2.c Thu Aug  3 05:27:05 2017
>> (r321969)
>> @@ -114,8 +114,8 @@ void exit(int);
>> static void load(void);
>> static int parse(void);
>> static int dskread(void *, unsigned, unsigned);
>> -static void printf(const char *,...);
>> -static void putchar(int);
>> +static int printf(const char *,...);
>> +static int putchar(int);
> 
> These are freestanding static functions, so they have nothing to do
> with library functions except their name is a hint that they are
> similar.
> 
> Since they are static, -funit-at-a-time might allow the unused return values
> to be optimized away.  Then returning unused values would be just an
> obfuscation.
> 
> This file still has a static memcpy() which is quite different from the
> libc version.  It doesn't return an unused value, and its arg types are
> all different (no newfangled size_t or newerfangled restrict).
> 
> Freestanding versions (static and otherwise) cause problems with builtins.
> -ffreestanding turns off all builtins.  The static memcpy used to be
> ifdefed so as to use __builtin_memcpy instead of the static one if the
> compiler is gcc.  That apparently broke with gcc-4.2, since the builtin
> will call libc memcpy() in some cases, although memcpy() is unavailable
> in the freestanding case.

I get the point about them being freestanding functions, but if the functions 
aren’t meant to be compatible they should be named differently. Part of the 
issue some code that bridged loader and non-loader users (bootdevtest and 
zfsboottest for example) relied on feature parity (in part because the ZFS code 
required it and because of how things are compiled/linked together). If 
compilers get pedantic enough, then they’ll flag these issues as errors and 
we’ll have to address these compatibilities at that point.

My intent was ok (I think), but the implementation I did is wrong, so I’ll 
revert the change completely and leave it for someone else to deal with the 
incompatibilities (I’ll just integrate bootdevtest and zfsboottest into 
buildworld under sys/boot so they won’t remain broken for weeks on end, like 
they were recently).

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321969 - in head/sys/boot: arm/at91/libat91 arm/ixp425/boot2 i386/boot2

[Ngie Cooper (yaneurabeya)]

> On Aug 3, 2017, at 04:48, Ed Schouten  wrote:
> 
> 2017-08-03 7:27 GMT+02:00 Ngie Cooper :
>> Modified: head/sys/boot/arm/at91/libat91/printf.c
>> ==
>> --- head/sys/boot/arm/at91/libat91/printf.c Thu Aug  3 03:45:48 2017 
>>(r321968)
>> +++ head/sys/boot/arm/at91/libat91/printf.c Thu Aug  3 05:27:05 2017 
>>(r321969)
>> @@ -20,12 +20,13 @@
>> #include 
>> #include "lib.h"
>> 
>> -void
>> +int
>> printf(const char *fmt,...)
>> {
>>va_list ap;
>>const char *hex = "0123456789abcdef";
>>char buf[10];
>> +   const char *fmt_orig = fmt;
>>char *s;
>>unsigned u;
>>int c;
>> @@ -66,5 +67,5 @@ printf(const char *fmt,...)
>>}
>>va_end(ap);
>> 
>> -   return;
>> +   return (int)(fmt - fmt_orig);
>> }
> 
> This makes printf() return the number of characters from the format
> processed, right? This is different from libc's printf(), which
> returns the number of characters printed.

Yes. markj identified flaws with my approach that need to be addressed 
in another commit (unfortunately I didn’t pay close enough attention to the 
details when I implemented the change).
Thanks,
-Ngie



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321093 - in stable/10: bin/ln bin/ln/tests etc/mtree

[Ngie Cooper (yaneurabeya)]

> On Jul 31, 2017, at 14:34, Alan Somers  wrote:
> 
> On Mon, Jul 17, 2017 at 3:12 PM, Ngie Cooper  wrote:
>> Author: ngie
>> Date: Mon Jul 17 21:12:02 2017
>> New Revision: 321093
>> URL: https://svnweb.freebsd.org/changeset/base/321093
>> 
>> Log:
>>  MFC r319857:
>> 
>>  ln(1): wordsmith -F option description
>> 
>> Added:
>>  stable/10/bin/ln/tests/
>> - copied from r319714, head/bin/ln/tests/
>> Modified:
>>  stable/10/bin/ln/Makefile
>>  stable/10/etc/mtree/BSD.tests.dist
>> Directory Properties:
>>  stable/10/   (props changed)
> 
> It looks like the commit message for this one was wrong, though the
> merge tracking info is correct.  The commit message should've read:
> 
> MFC r319714:
> 
> Add tests for ln(1)
> 
> * Verify that when creating a hard link to a symbolic link, '-L' option
>  creates a hard link to the target of the symbolic link
> * Verify that when creating a hard link to a symbolic link, '-P' option
>  creates a hard link to the symbolic link itself
> * Verify that if the target file already exists, '-f' option unlinks it so
>  that link may occur
> * Verify that if the target file or directory is a symbolic link, '-shf'
>  option prevents following the link
> * Verify that if the target file or directory is a symbolic link, '-snf'
>  option prevents following the link
> * Verify that '-s' option creates a symbolic link
> * Verify that '-w' option produces a warning if the source of a symbolic
>  link does not currently exist
> 
> Submitted by: shivansh
> Reviewed by: asomers, ngie
> MFC after: 1 month
> Sponsored by: Google, Inc (GSoC 2017)
> Differential Revision: https://reviews.freebsd.org/D11084

Whoops — yeah, I might have screwed that up by accident :(.
Thanks for the clarification!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321849 - in head/tools/tools: bootparttest zfsboottest

[Ngie Cooper (yaneurabeya)]

> On Jul 31, 2017, at 23:39, Andriy Gapon  wrote:
> 
> On 01/08/2017 08:29, Ngie Cooper wrote:
>> Author: ngie
>> Date: Tue Aug  1 05:29:36 2017
>> New Revision: 321849
>> URL: https://svnweb.freebsd.org/changeset/base/321849
>> 
>> Log:
>>  Fix the return type for pager_output(..) in bootparttest and zfsboottest
>> 
>>  pager_output(..) has always returned int. For some reason (probably the
>>  clang 5.0 update), this mismatch now results in an error with CC=clang.
>> 
>>  MFC after:  1 week
> 
> I am not sure if this a correct fix...
> Return value of pager_output() signifies whether to continue printing, return
> value of printf is a number of characters printed.

You’re right — it was wrong. Fixed in r321852.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321849 - in head/tools/tools: bootparttest zfsboottest

[Ngie Cooper (yaneurabeya)]

> On Aug 1, 2017, at 00:13, Andriy Gapon  wrote:

…

> Thank you!
> The next problem you are going to encounter would be missing ldi_get_size().
> I have some WIP for that if you are interested.

That’d be great — please feel free to pass it along :)!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321824 - head/tests/sys/file

[Ngie Cooper (yaneurabeya)]

> On Jul 31, 2017, at 15:00, Bryan Drewery  wrote:
> 
> Author: bdrewery
> Date: Mon Jul 31 22:00:27 2017
> New Revision: 321824
> URL: https://svnweb.freebsd.org/changeset/base/321824
> 
> Log:
>  Allow changing the test PORT at compile-time.
> 
>  Sponsored by:Dell EMC Isilon

MFC after?


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321448 - stable/11/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Jul 24, 2017, at 17:30, Mark Johnston  wrote:
> 
> Author: markj
> Date: Tue Jul 25 00:30:25 2017
> New Revision: 321448
> URL: https://svnweb.freebsd.org/changeset/base/321448
> 
> Log:
>  Include stdbool.h for r321447.
> 
>  This is a direct commit to stable/11.

Shouldn’t the pollution be committed to ^/head and backported instead of doing 
a direct commit?
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321444 - head/etc

[Ngie Cooper (yaneurabeya)]

> On Jul 24, 2017, at 17:17, Bryan Drewery  wrote:
> 
> On 7/24/2017 4:57 PM, Ngie Cooper wrote:
>> Author: ngie
>> Date: Mon Jul 24 23:57:43 2017
>> New Revision: 321444
>> URL: https://svnweb.freebsd.org/changeset/base/321444
>> 
>> Log:
>>  Remove ${MTREE} and leverage etc/mtree/Makefile instead with
>>  "make distribution".
>> 
>>  This also fixes the fact that BSD.debug.dist was being installed if/when
>>  ${MK_DEBUG_FILES} != "no" before this commit.
> 
> This was intentional, see r279248.

Ok. It would have been nice if there had been a comment added noting that it 
was being unconditionally installed for a reason.

I’ll add a relevant comment to etc/mtree/Makefile and restore the previous 
behavior (it was silly having duplicative logic in two places, since 
etc/mtree/Makefile wasn’t actually being used).

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321434 - head

[Ngie Cooper (yaneurabeya)]

> On Jul 24, 2017, at 11:54, Bryan Drewery  wrote:
> 
> Author: bdrewery
> Date: Mon Jul 24 18:54:56 2017
> New Revision: 321434
> URL: https://svnweb.freebsd.org/changeset/base/321434
> 
> Log:
>  Add some .ORDER for distrib-dirs, distribute, distribution and distribute 
> world.

The why could have helped explain this:

'This helps fix parallel “make installworld” with newly created directories”

Thanks for fixing this ;)!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321426 - head/share/mk

[Ngie Cooper (yaneurabeya)]

> On Jul 24, 2017, at 10:53, Bryan Drewery  wrote:
> 
> Author: bdrewery
> Date: Mon Jul 24 17:53:45 2017
> New Revision: 321426
> URL: https://svnweb.freebsd.org/changeset/base/321426
> 
> Log:
>  cleanobj: Unhide removal of directory.
> 
>  MFC after:   2 weeks
>  Sponsored by:Dell EMC Isilon
> 
> Modified:
>  head/share/mk/bsd.obj.mk

Why? Dumping output of things like this is why “make -d l” is helpful.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321331 - head/share/mk

[Ngie Cooper (yaneurabeya)]

> On Jul 21, 2017, at 07:50, Ruslan Bukin  wrote:
> 
> Author: br
> Date: Fri Jul 21 14:50:32 2017
> New Revision: 321331
> URL: https://svnweb.freebsd.org/changeset/base/321331
> 
> Log:
>  Add warning flags for GCC 7.1.0 compiler.
> 
>  Sponsored by:DARPA, AFRL
> 
> Modified:
>  head/share/mk/bsd.sys.mk
> 
> Modified: head/share/mk/bsd.sys.mk
> ==
> --- head/share/mk/bsd.sys.mk  Fri Jul 21 14:14:47 2017(r321330)
> +++ head/share/mk/bsd.sys.mk  Fri Jul 21 14:50:32 2017(r321331)
> @@ -145,6 +145,23 @@ CWARNFLAGS+= -Wno-error=misleading-indentation   
> \
>   -Wno-error=unused-const-variable
> .endif
> 
> +# GCC 7.1.0
> +.if ${COMPILER_TYPE} == "gcc" && ${COMPILER_VERSION} >= 70100
> +CWARNFLAGS+= -Wno-error=deprecated   \
> + -Wno-error=pointer-compare  \
> + -Wno-error=format-truncation\
> + -Wno-error=implicit-fallthrough \
> + -Wno-error=expansion-to-defined \
> + -Wno-error=int-in-bool-context  \
> + -Wno-error=bool-operation   \
> + -Wno-error=format-overflow  \
> + -Wno-error=stringop-overflow\
> + -Wno-error=memset-elt-size  \
> + -Wno-error=int-in-bool-context  \
> + -Wno-error=unused-const-variable\
> + -Wno-error=nonnull
> +.endif

Hi Ruslan,
1. Fixing some of these warnings make sense — why are we ignoring them?
2. Why isn’t this being MFCed? gcc 7.1.0 is available on ^/stable/11 as 
well.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r321333 - head/lib/libc/tests/sys

[Ngie Cooper (yaneurabeya)]

> On Jul 21, 2017, at 9:14 AM, Bryan Drewery  wrote:
> 
> Author: bdrewery
> Date: Fri Jul 21 16:14:06 2017
> New Revision: 321333
> URL: https://svnweb.freebsd.org/changeset/base/321333
> 
> Log:
>  Properly set userid for truncate_test.
> 
>  MFC after:   1 week
>  Sponsored by:Dell EMC Isilon
> 
> Modified:
>  head/lib/libc/tests/sys/Makefile
> 
> Modified: head/lib/libc/tests/sys/Makefile
> ==
> --- head/lib/libc/tests/sys/Makefile  Fri Jul 21 15:09:24 2017
> (r321332)
> +++ head/lib/libc/tests/sys/Makefile  Fri Jul 21 16:14:06 2017
> (r321333)
> @@ -87,7 +87,7 @@ FILESGROUPS+=   truncate_test_FILES
> truncate_test_FILES=  truncate_test.root_owned
> truncate_test_FILESDIR=   ${TESTSDIR}
> truncate_test_FILESMODE= 0600
> -truncate_test_FILESOWNER= root
> +truncate_test_FILESOWN= root
> truncate_test_FILESGRP= wheel
> truncate_test_FILESPACKAGE=   ${PACKAGE}

Thanks!!
-Ngie



signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r321370 - head/lib/libsysdecode

[Ngie Cooper (yaneurabeya)]

> On Jul 22, 2017, at 5:51 AM, Kristof Provost  wrote:
> 
> Author: kp
> Date: Sat Jul 22 12:51:19 2017
> New Revision: 321370
> URL: https://svnweb.freebsd.org/changeset/base/321370
> 
> Log:
>  Handle WITH/WITHOUT_PF in libsysdecode
> 
>  Only filter out the PF ioctls if we're building without pf support.
>  Until now those were always filtered out, so truss did not show symbolic
>  names for pf ioctls.


Hi Kristof!
'MFC after: 1 week” should have probably be added to this commit.
This change should be MFCed to ^/stable/11 and probably ^/stable/10 (in 
usr.sbin/kdump on 10: 
https://svnweb.freebsd.org/base/stable/10/usr.bin/kdump/mkioctls?revision=291815=markup#l25
 
).
Thank you!
-Ngie


signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r321080 - head/usr.bin/getconf

[Ngie Cooper (yaneurabeya)]

> You timed out because you never answered my question about other platforms.

Hi Warner,
I thought I answered your questions here: 
https://reviews.freebsd.org/D10787#224691 ? If not, I would be more than happy 
to run the tests on other platforms once I gain access to them. I only have 
access to x86 platforms and made an educated guess on what should be done based 
on grep’ing around the source tree, googling, and finding Linux mailing list 
posts suggesting our copy of getconf(1) wasn’t supporting all of the platforms 
it should have been.
Thank you,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r320701 - in head: bin sbin usr.bin usr.sbin

[Ngie Cooper (yaneurabeya)]

> On Jul 6, 2017, at 10:32, Ngie Cooper  wrote:

…

>I have another change I'm going to propose soon which sorts SUBDIR output 
> (as a whole) again, as an opt-in feature in bsd.subdir.mk.
> Cheers,

Hi,
I just opened up the CR to re-add the sorting functionality in a 
generic way as an opt-in make variable: https://reviews.freebsd.org/D11628 .
Cheers!
-Ngie



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r320454 - head/share/zoneinfo

[Ngie Cooper (yaneurabeya)]

> On Jun 28, 2017, at 12:05, Cy Schubert  wrote:
> 
> Author: cy
> Date: Wed Jun 28 19:05:04 2017
> New Revision: 320454
> URL: https://svnweb.freebsd.org/changeset/base/320454
> 
> Log:
>  Allow parallel installworld (-j N) and poudriere installworld
>  (poudriere jail -c and poudriere jail -u) to proceed.
> 
>  Reviewed by: trasz@
>  Tested by:   trasz@, cy@
>  MFC after:   1 month
>  X-MFC-with:  r320362
> 
> Modified:
>  head/share/zoneinfo/Makefile
> 
> Modified: head/share/zoneinfo/Makefile
> ==
> --- head/share/zoneinfo/Makefile  Wed Jun 28 17:32:09 2017
> (r320453)
> +++ head/share/zoneinfo/Makefile  Wed Jun 28 19:05:04 2017
> (r320454)
> @@ -94,7 +94,7 @@ install-zoneinfo:
> .for f in ${TZS}
>   ${INSTALL} ${TAG_ARGS} \
>   -o ${BINOWN} -g ${BINGRP} -m ${NOBINMODE} \
> - ${TZBUILDDIR:C,^${.OBJDIR}/,,}/${f} 
> ${DESTDIR}/usr/share/zoneinfo/${f}
> + ${TZBUILDDIR}/${f} ${DESTDIR}/usr/share/zoneinfo/${f}
> .endfor
>   ${INSTALL} ${TAG_ARGS} -o ${BINOWN} -g ${BINGRP} -m ${NOBINMODE} \
>   ${CONTRIBDIR}/zone.tab ${DESTDIR}/usr/share/zoneinfo/

Thank you for getting this in!
In general, I would argue that for/.for loops in Make targets are bad 
like this I think that individual targets could in fact be created, and driven 
as a dependency of a top-level target, to avoid this issue and keep bdrewery’s 
intended change in r320362, e.g. something like,

install-zones: .PHONY
.for f in ${TZS}
install-zones: ${DESTDIR}/usr/share/zoneinfo/${f}
${DESTDIR}/usr/share/zoneinfo/${f}: ${f}
${INSTALL} ...
.endfor

I’m not incredibly sure based on this commit alone why this Makefile 
isn’t using FILES though… I’ll look at it closer when I have a spare minute.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r320388 - head/sys/arm64/include

[Ngie Cooper (yaneurabeya)]

> On Jun 27, 2017, at 10:06, Andrew Turner  wrote:

...

> I already fixed it in r320411.

Ok! The CI build is currently running, so *crosses fingers* hopefully 
everything will be fixed after this build on amd64/arm64.
Thanks!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r320388 - head/sys/arm64/include

[Ngie Cooper (yaneurabeya)]

> On Jun 26, 2017, at 15:32, Andrew Turner  wrote:
> 
> Author: andrew
> Date: Mon Jun 26 22:32:52 2017
> New Revision: 320388
> URL: https://svnweb.freebsd.org/changeset/base/320388
> 
> Log:
>  In _bswap16 and _bswap32 cast constant values to the appropriate type. This 
> is
>  similar to what is done in the x86 code.
> 
>  Sponsored by:DARPA, AFRL

Hi Andrew,
This change broke htons use in libsdp on arm64. From 
https://ci.freebsd.org/job/FreeBSD-head-aarch64-build/2932/console :

16:08:38 --- all_subdir_lib/libsdp ---
16:08:38 /usr/src/lib/libsdp/search.c:160:31: error: invalid operands to binary 
expression ('int' and 'uint8_t *' (aka 'unsigned char *'))
16:08:38 xpdu.pdu.len = htons(req_cs - ss->req);

16:08:38~^~

It might be a good idea to revert the change, fix the fallout, then 
recommit the change.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r320277 - head/usr.bin/mkuzip

[Ngie Cooper (yaneurabeya)]

> On Jun 23, 2017, at 16:51, Glen Barber  wrote:
> 
> On Fri, Jun 23, 2017 at 03:54:35PM -0700, Conrad Meyer wrote:
>>> @@ -58,12 +59,15 @@ mkuz_get_insize(struct mkuz_cfg *cfp)
>>>ffd = open(statfsbuf.f_mntfromname, O_RDONLY);
>>>if (ffd < 0) {
>>>warn("open(%s, O_RDONLY)", statfsbuf.f_mntfromname);
>>> +   close(ffd);
>> 
>> This one is wrong for obvious reasons.
>> 
> 
> It isn't obvious to me.  :)

open failed, so closing ffd == -1 is a programming error (but admittedly, 
pretty benign).
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r320222 - in stable/10: etc etc/cron.d etc/mtree etc/newsyslog.conf.d etc/pam.d etc/syslog.d tools/build/mk usr.sbin/cron/cron usr.sbin/cron/lib usr.sbin/syslogd

[Ngie Cooper (yaneurabeya)]

> On Jun 22, 2017, at 00:35, John Baldwin  wrote:

…

> Please revert the breakout of the existing config files.  I think that 
> splitting
> up the conf files is too disruptive of a change (POLA) for stable branches.

Done in r320229 and r320230.

> This was agreed to by other folks in the followup thread to the commits in
> head that you continue to ignore.


> In head please either fully split up the files or revert to a single file
> (another part of that thread you continue to ignore).

My eyes glazed over trying to read through all of the posts. I seem to 
have missed the point where a proposed change was made to split up the config 
files further. Could this please be summarized again?
Thank you,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r320132 - in head: . share/man/man5

[Ngie Cooper (yaneurabeya)]

> On Jun 19, 2017, at 8:44 PM, Emmanuel Vadot  wrote:
> 
> Author: manu
> Date: Tue Jun 20 03:44:21 2017
> New Revision: 320132
> URL: https://svnweb.freebsd.org/changeset/base/320132
> 
> Log:
>  Fix punctionation in UPDATING and regen src.conf after r320127
> 
>  Reported by: ngie

Thanks :)!
-Ngie


signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r319557 - head/sys/fs/ext2fs

[Ngie Cooper (yaneurabeya)]

> On Jun 3, 2017, at 18:16, Pedro Giffuni <p...@freebsd.org> wrote:
> 
> 
> On 03/06/2017 17:54, Ngie Cooper (yaneurabeya) wrote:
>>> On Jun 3, 2017, at 15:30, Conrad Meyer <c...@freebsd.org> wrote:
>>> 
>>> Author: cem
>>> Date: Sat Jun  3 22:30:30 2017
>>> New Revision: 319557
>>> URL: https://svnweb.freebsd.org/changeset/base/319557
>>> 
>>> Log:
>>>  ext2fs(4): Fix a number of broken flag checks
>>> 
>>>  Introduced in r319071.
>>> 
>>>  Reported by:   Coverity
>>>  CIDs:  1375847, 1375848, 1375849
>>>  Sponsored by:  Dell EMC Isilon
>> Did pjg review this?
> 
> I don't know about pjg, but I wasn't even aware about the Coverity issues. I 
> never added myself to the MAINTAINERS list so it's OK, I will just have the 
> code submitter confirm the fixes.

I meant pfg — sorry :/.

> Luckily this is code that has not been MFC'd because I don't consider it 
> bullet-proof and more eyes are always welcome.


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r319557 - head/sys/fs/ext2fs

[Ngie Cooper (yaneurabeya)]

> On Jun 3, 2017, at 15:30, Conrad Meyer  wrote:
> 
> Author: cem
> Date: Sat Jun  3 22:30:30 2017
> New Revision: 319557
> URL: https://svnweb.freebsd.org/changeset/base/319557
> 
> Log:
>  ext2fs(4): Fix a number of broken flag checks
> 
>  Introduced in r319071.
> 
>  Reported by: Coverity
>  CIDs:1375847, 1375848, 1375849
>  Sponsored by:Dell EMC Isilon

Did pjg review this?
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318441 - in head/etc: . cron.d

[Ngie Cooper (yaneurabeya)]

> On Jun 3, 2017, at 12:05, John Baldwin  wrote:

…

> Ping?  bapt@ agreed with this, but I haven't seen any other feedback?  If 
> others
> agree then we should either revert back to a single foo.conf or we should 
> fully
> split newsyslog.conf and syslogd.conf out to individual files.

Please, no. The single monolithic foo.conf idea may have made sense back in the 
day, but with the various knobs that are available for building the system, for 
ease-of-use/intuitiveness the feature makes sense.
I planned on introducing more of these files for other areas of the system that 
aren’t currently being handled— like bsnmpd’s logging, etc. I think the 
modularization makes sense.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r319551 - head/bin/dd/tests

[Ngie Cooper (yaneurabeya)]

> On Jun 3, 2017, at 11:29, Alan Somers  wrote:
> 
> Author: asomers
> Date: Sat Jun  3 18:29:18 2017
> New Revision: 319551
> URL: https://svnweb.freebsd.org/changeset/base/319551
> 
> Log:
>  Fix bin/dd/dd2_tests:seek_overflow on UFS and TMPFS
> 
>  Split the postive and negative parts into separate test cases.  The positive
>  test case can only run on ZFS, because only ZFS supports files that large.
> 
>  PR:  219757
>  Reported by: ngie
>  MFC after:   18 days
>  X-MFC-with:  319339

Thanks :)!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r319295 - head/usr.bin/mkimg/tests

[Ngie Cooper (yaneurabeya)]

> On Jun 1, 2017, at 13:06, Alexey Dokuchaev <da...@freebsd.org> wrote:
> 
> On Thu, Jun 01, 2017 at 11:27:18AM -0700, Ngie Cooper (yaneurabeya) wrote:
>> ...
>> I vote for not checking in the SHAs. If something fails (as someone else
>> mentioned on the lists), it's really hard to debug why it failed.
> 
> Like I've said, it's not "really" hard.  Check out previous version and
> generate image with correct hash, there is no strong reason to commit the
> image other than minor usability improvement of not having to check out
> previous version.
> 
> Then again, just my $.02 -- I'm not the one doing actual work here.

That’s assuming there aren’t endianness issues, and less intuitive 
issues (take the usr.sbin/sa tests for example — BLEH). With binary files like 
this, it’s best to do an abbreviated dump of sorts.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r319295 - head/usr.bin/mkimg/tests

[Ngie Cooper (yaneurabeya)]

> On Jun 1, 2017, at 10:16, Alexey Dokuchaev  wrote:
> 
> On Thu, Jun 01, 2017 at 09:58:38AM -0700, Simon J. Gerraty wrote:
>> One option would be to store a sha256 hash of the result.
>> If the image generated by the test hashes to the correct value - you
>> pass.
> 
> That should be preferred.  Storing binary blobs in VCS is bad practice.
> 
>> Of course if the test fails, you are completely in the dark as to
>> how/why.
> 
> Not really: you checkout previous version (which produces correct hash)
> and store generated image for later inspection and diffing.
> 
>> Is storing img-1x1-512-mbr.vmdk.gz any more palatable?
> 
> This is barely acceptable, but only if image was zerofied/sparsed prior
> to compression.  Also, the image should be of the minimal (sufficient)
> working size.

I vote for not checking in the SHAs. If something fails (as someone 
else mentioned on the lists), it’s really hard to debug why it failed.
I think marcel@’s proposal for hexdump -C text dumps are the best way 
to handle this.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316286 - in head: lib/libprocstat sys/kern sys/sys usr.bin/gcore usr.bin/procstat

[Ngie Cooper (yaneurabeya)]

> On Mar 30, 2017, at 11:21, Tycho Nightingale  wrote:
> 
> Author: tychon
> Date: Thu Mar 30 18:21:36 2017
> New Revision: 316286
> URL: https://svnweb.freebsd.org/changeset/base/316286
> 
> Log:
>  Add support for capturing 'struct ptrace_lwpinfo' for signals
>  resulting in a process dumping core in the corefile.
> 
>  Also extend procstat to view select members of 'struct ptrace_lwpinfo'
>  from the contents of the note.
> 
>  Sponsored by:Dell EMC Isilon

Hi Tycho,
Should this be MFCed?
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r319295 - head/usr.bin/mkimg/tests

[Ngie Cooper (yaneurabeya)]

> On May 31, 2017, at 10:03 PM, Brooks Davis  wrote:
> 
> On Wed, May 31, 2017 at 08:01:12AM +, Ngie Cooper wrote:
>> Author: ngie
>> Date: Wed May 31 08:01:12 2017
>> New Revision: 319295
>> URL: https://svnweb.freebsd.org/changeset/base/319295
>> 
>> Log:
>>  Update the usr.bin/mkimg golden test output files after ^/head@r319125
>> 
>>  ^/head@r319125 changed the location of the backup pmbr, requiring the
>>  output files to be regenerated, since they're binary disk dumps.
>> 
>>  The output files were regenerated with "make rebase"--fixed in
>>  ^/head@r319294.
> 
> These should not be stored uuencoded.  It serves no purpose other
> than bloating the repo and causing spammy commit mails like this one
> where we got a huge tail of garbage output.

Hi Brooks,
I’m not entirely sure why the files were uuencoded to be honest. I 
think that’s a good question for Marcel and some of the folks at Juniper, since 
they wrote the tool/tests.
Thanks!
-Ngie

PS I agree based on what little I know, but I don’t necessarily know what trade 
offs were made with writing these tests.


signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r319125 - head/usr.bin/mkimg

[Ngie Cooper (yaneurabeya)]

> On May 29, 2017, at 05:51, Emmanuel Vadot  wrote:
> 
> Author: manu
> Date: Mon May 29 12:51:02 2017
> New Revision: 319125
> URL: https://svnweb.freebsd.org/changeset/base/319125
> 
> Log:
>  mkimg: Correct an off by one error in the PMBR size
> 
>  The PMBR last sector should be number of sector - 1 (As stated in UEFI Spec
>  2.6 page 118 table 17).
>  This fixes warning printed by linux tools like parted or fdisk.

This commit broke all of the mkimg gpt tests. For example: 
https://ci.freebsd.org/job/FreeBSD-head-amd64-test/3280/testReport/junit/usr.bin.mkimg/mkimg_test/gpt_1x1_4096_qcow/
 .
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r319059 - head/tests/sys/file

[Ngie Cooper (yaneurabeya)]

> On May 28, 2017, at 01:55, Ngie Cooper  wrote:
> 
> Author: ngie
> Date: Sun May 28 08:55:32 2017
> New Revision: 319059
> URL: https://svnweb.freebsd.org/changeset/base/319059
> 
> Log:
>  Use an exit code of 1 instead of -1 for reasons noted in r319056
> 
>  MFC after:   3 days
>  Sponsored by:Dell EMC Isilon

Also: fix an invalid test with ftruncate.

Reported by:Coverity
CID:1296041


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318736 - in head: cddl/lib/libzfs contrib/compiler-rt/lib/sanitizer_common contrib/openbsm/libbsm include lib/libarchive lib/libc/gen lib/libc/include lib/libc/sys lib/libkvm lib/libm

[Ngie Cooper (yaneurabeya)]

> On May 24, 2017, at 09:26, Ed Maste  wrote:
> 
> On 24 May 2017 at 10:53, Pedro Giffuni  wrote:
>> 
>> There is now a bunch of coverity issues (OVERFLOW_BEFORE_WIDEN mostly)
>> tied to ino64_t. At least the following CIDs are related:
> 
> On a quick look the OVERFLOW_BEFORE_WIDEN issues are false positives
> in practice: for correct operation the product must fit within 32
> bits, because inodes were previously 32 bits. However, it does seem
> unusual to perform a multiplication with types that could overflow 32
> bits, and then store the result in a 64-bit variable. So it seems that
> a code change to eliminate the warning is likely reasonable.

Wouldn’t the proper fix be:
- Check the inode value for overflow?
- Cast to the smaller historical value?
Or:
- Widen the type for the smaller historical value to a larger value?
Thanks,
-Ngie

PS I’d have to see how we fixed this internally at Isilon. We might have a 
patch kicking around for this.


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318757 - head

[Ngie Cooper (yaneurabeya)]

> On May 24, 2017, at 11:10, O. Hartmann  wrote:
> 
> Am Wed, 24 May 2017 13:04:30 -0500
> Larry Rosenman  schrieb:

…

> I use the traditional "make" way (via portmaster)

There were some reports about needing to do “make clean” before “make 
install”. Memory serves me correctly there are ways in portmaster to bypass 
“make clean” before running “make install”. Could you please provide your 
portmasterrc file?
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318757 - head

[Ngie Cooper (yaneurabeya)]

> On May 23, 2017, at 14:33, Alexey Dokuchaev  wrote:
> 
> On Tue, May 23, 2017 at 08:25:49PM +, Ed Maste wrote:
>> New Revision: 318757
>> URL: https://svnweb.freebsd.org/changeset/base/318757
>> 
>> Log:
>>  Add note to UPDATING for ino64 to follow the standard upgrade process
>> ...
>> +20170523:
>> +The "ino64" 64-bit inode project has been committed, which extends
>> +a number of types to 64 bits.
> 
> For the dumb people among us, what is it all about?  E.g., what's so cool
> about it?

64-bit inodes: more addressable inodes for bigger filesystems/disks.
From 
https://lists.freebsd.org/pipermail/freebsd-fs/2017-April/024684.html :

“”"
Inodes are data structures corresponding to objects in a file system,
such as files and directories. FreeBSD has historically used 32-bit
values to identify inodes, which limits file systems to somewhat under
2^32 objects. Many modern file systems internally use 64-bit identifiers
and FreeBSD needs to follow suit to properly and fully support these
file systems.
“””

With Isilon OneFS we currently support multiples of petabytes of 
storage ( 
https://www.emc.com/collateral/hardware/white-papers/h10719-isilon-onefs-technical-overview-wp.pdf
 ), so this matters very much to us. It’s also useful for other file systems 
though (assuming they can scale that high).
Cheers,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318699 - head/lib/libc/gen

[Ngie Cooper (yaneurabeya)]

> On May 23, 2017, at 08:13, Benjamin Kaduk  wrote:
> 
> On Tue, May 23, 2017 at 1:55 AM, Ngie Cooper  wrote:
> Author: ngie
> Date: Tue May 23 06:55:51 2017
> New Revision: 318699
> URL: https://svnweb.freebsd.org/changeset/base/318699
> 
> Log:
>   directory(3): delete trailing whitespace and rewrite `E.g.` as `e.g.`
> 
>  FWIW, it is better-still written as "e.g.,", i.e., with trailing comma as 
> well as leading comma.

Fixed in r318753 — thanks :).
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318594 - in head: lib lib/libc++experimental tools/build/mk

[Ngie Cooper (yaneurabeya)]

> On May 21, 2017, at 11:34, Dimitry Andric  wrote:

…

> Hmm, I don't know how to tell the build system otherwise that I don't
> want any shared library.  It looks like NO_PIC is needed in bsd.lib.mk
> to force no .so to be built, though.

NO_PIC is the way to do this.

-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318364 - in head: contrib/libxo contrib/libxo/doc contrib/libxo/libxo contrib/libxo/tests/core contrib/libxo/tests/core/saved contrib/libxo/tests/gettext/saved contrib/libxo/xo lib/li

[Ngie Cooper (yaneurabeya)]

> On May 16, 2017, at 11:46, Phil Shafer  wrote:
> 
> Author: phil
> Date: Tue May 16 18:46:56 2017
> New Revision: 318364
> URL: https://svnweb.freebsd.org/changeset/base/318364
> 
> Log:
>  Import libxo-0.7.2; add xo_options.7.
> 
>  Submitted by:phil
>  Reviewed by: sjg
>  Approved by: sjg (mentor)

Relnotes: yes

Also, what does this upgrade entail, in terms of new features/risk?

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318354 - head/sys/amd64/amd64

[Ngie Cooper (yaneurabeya)]

> On May 16, 2017, at 9:20 AM, Conrad Meyer  wrote:
> 
> Author: cem
> Date: Tue May 16 16:20:22 2017
> New Revision: 318354
> URL: https://svnweb.freebsd.org/changeset/base/318354
> 
> Log:
>  Correct page frame mask constant used in pmap_change_attr_locked
> 
>  This was introduced in r290156.  It's present in 11.0, but not any 10.x
>  release unless someone decided to MFC it.

Hi Conrad,
It wasn’t MFCed according to 
https://svnweb.freebsd.org/base/stable/10/sys/amd64/amd64/pmap.c 
, so that’s 
ok.
Could you please MFC this change since you understand it/made it?
Thank you,
-Ngie


signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r318250 - in head: etc etc/newsyslog.conf.d etc/syslog.d tools/build/mk

[Ngie Cooper (yaneurabeya)]

> On May 15, 2017, at 10:47, Ngie Cooper (yaneurabeya) <yaneurab...@gmail.com> 
> wrote:
> 
> 
>> On May 15, 2017, at 10:43, Ian Lepore <i...@freebsd.org> wrote:
> 
> …
> 
>> That's only a good argument for keeping the lines in the monolithic
>> file if those lines will be ignored when a file in the .conf.d
>> directory provides conflicting config.  Otherwise my embedded product
>> that drops different rules for rotating /var/log/messages into .conf.d
>> STILL has to programmatically edit the monolithic file to remove the
>> standard rule(s).
> 
>   Bingo. This is part of the reason why I did this, apart from being 
> selfish in not wanting to handle a handful of unnecessary entries in 
> newsyslog/syslogd on all of my systems where I set these knobs to no.
> Literally all I did was `dd+p` in vim in the new files. I didn’t add or 
> subtract any overall entries.

And for context, we (Isilon) run newsyslog more frequently than upstream 
(sometimes every couple minutes, sometimes multiple times an hour/day), 
attributing to unnecessary creation and modification of log files (like this) 
that we (Isilon) don’t care about at all, which causes other potential issues 
with root media wear, potential for filesystem corruption, reduced number of 
available inodes/space because it allocates at least the frag size for a file, 
etc.

Plus with the number of entries we have in syslog.conf (an order of magnitude 
larger than upstream), having to walk a linked list to evaluate how to handle 
messages when putting them out in log files does tend to add up with all of the 
logging we do on our appliances (which is considerably more chatty than I’ve 
seen other appliance vendors that I’ve worked for).

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318250 - in head: etc etc/newsyslog.conf.d etc/syslog.d tools/build/mk

[Ngie Cooper (yaneurabeya)]

> On May 15, 2017, at 10:43, Ian Lepore  wrote:

…

> That's only a good argument for keeping the lines in the monolithic
> file if those lines will be ignored when a file in the .conf.d
> directory provides conflicting config.  Otherwise my embedded product
> that drops different rules for rotating /var/log/messages into .conf.d
> STILL has to programmatically edit the monolithic file to remove the
> standard rule(s).

Bingo. This is part of the reason why I did this, apart from being 
selfish in not wanting to handle a handful of unnecessary entries in 
newsyslog/syslogd on all of my systems where I set these knobs to no.
Literally all I did was `dd+p` in vim in the new files. I didn’t add or 
subtract any overall entries.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r318250 - in head: etc etc/newsyslog.conf.d etc/syslog.d tools/build/mk

[Ngie Cooper (yaneurabeya)]

> On May 13, 2017, at 9:51 AM, Alexey Dokuchaev  wrote:
> 
> On Sat, May 13, 2017 at 10:24:20AM -0600, Ian Lepore wrote:
>> ...
>> The evolution for years has been away from monolithic config files
>> containing a mashup of values for unrelated subsystems and towards
>> .conf.d directories containing many single-subject files.
> 
> This "evolution" had probably originated in people's minds who know little
> about software development and maintenance.  And FWIW, newsyslog files are
> not about "unrelated subsystems", it's about one subsystem responsible for
> log rotation.

This hasn’t really changed with moving to .conf.d. A single subsystem is 
managing a series of modular config files, instead of a single config file. I 
firmly believe that this was the right general approach to go.

> Speaking of "unrelated subsystems", /etc/rc.conf is a living manifestation
> of how "unrelated subsystems" can be configured in a single file and, mind
> you, everyone is being quite happy about it.

… except people have to bake in defaults in rc.d scripts for whether or not 
services should be disabled because they can’t put apache defaults in 
/etc/rc.conf . /etc/rc.conf isn’t managed via etcupdate or mergemaster, so I 
think this comparison is like apples to oranges.

>> The monolithic files are difficult to edit
> 
> Quite on the contrary: monolithic files are much easier to edit and keep
> track of by a human being (system operator).

I strongly disagree, having seen multiple configuration files a couple hundred 
lines long. It gets messy and for those who don’t understand how 
syslogd/newsyslog works (inevitably, these people are the ones that get charged 
with implementing daemons, and this is one of the pieces that needs to be done).

>> and otherwise manage programmatically, and especially difficult to manage
>> in terms of software packaging and software updates.
> 
> Please don't mix "difficult to edit" and "manage programmatically".  As I
> have said, having support for "include *.conf.d" makes sense for 3rd-party
> software (read: ports), but has little need for the base, and IMHO brings
> more maintenance burden than any benefit.

Can you please provide an example of how it’s more burdensome going to .conf.d? 
Personally, I think it’s a whole lot easier doing `rm -f 
/etc/newsyslog.d/amd.conf`, than it is to open up the file and edit out the amd 
entries, or invoke sed/something else to do the same thing.

Even ansible/chef/puppet would have to bake the configuration removal logic 
into its template files, which seems like a pain for folks (and the same logic 
would need to be implemented multiple times instead of once).

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r318250 - in head: etc etc/newsyslog.conf.d etc/syslog.d tools/build/mk

[Ngie Cooper (yaneurabeya)]

> On May 13, 2017, at 9:39 AM, Warner Losh  wrote:
> 
> On Sat, May 13, 2017 at 10:32 AM, Ngie Cooper  > wrote:
>> 
>>> On May 13, 2017, at 09:29, Ngie Cooper  wrote:
>>> 
>>> On May 13, 2017, at 08:37, Rodney W. Grimes 
>>>  wrote:
>>> 
> Author: ngie
> Date: Sat May 13 03:10:50 2017
> New Revision: 318250
> URL: https://svnweb.freebsd.org/changeset/base/318250
> 
> Log:
> Handle the logfiles in newsyslog and syslogd conditionally, based on
> src.conf(5) knobs
> 
> This will allow consumers of FreeBSD to use the unmodified configuration
> files out of the box more than previously.
 
 What about simply generating proper newsyslog.conf and syslog.conf based
 on the the MK_ values rather than change the visible administration
 interface that has finger memory, ansible, and puppet support?
>>> 
>>> - My employer doesn't use puppet on its appliances.
>>> - Files snapshotted in time bitrot (which was the case with our configs, 
>>> which is part of the reason behind this change.
> 
> couldn't you have an Isilion config file in the appropriate directory
> instead? And use the default newsyslog / syslog config files?

I’m not sure I’m in disagreement with this statement, but the fact is that 
there are a handful of config files, about 200 lines each, which drive 
newsyslog/syslogd on OneFS, with a fair amount of duplication. I’m trying to 
simplify the mess I’m presented with at work, one bit at a time.

>>> - It's really easy to screw up a mergemaster call if you edit the files, 
>>> and install the stock version which removes the edits.
>> 
>> Also, programmatically removing the entries means you have to bake the 
>> metadata into etc/Makefile, which is already complicated enough as-is.
> 
> Why do you care about removing them at all? They are no-ops if the
> files don't exist. Why not just always install all these files is
> where I'm going with this…

I don’t follow. Could you please restate this paragraph?
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r318008 - head/sys/modules

[Ngie Cooper (yaneurabeya)]

> On May 8, 2017, at 22:06, O. Hartmann  wrote:
> 
> On Tue, 9 May 2017 04:59:05 + (UTC)
> Ngie Cooper  wrote:

…

> Ooopsie ... ;-)
> 
> Something went wrong after I sucked in this commit just a few seconds ago,
> compiling kernel failed:

You’re fast. Fixed in r318009 5 minutes ago.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317744 - in head/usr.sbin/makefs: . ffs

[Ngie Cooper (yaneurabeya)]

> On May 8, 2017, at 11:24, Ngie Cooper (yaneurabeya) <yaneurab...@gmail.com> 
> wrote:
> 
>> 
>> On May 3, 2017, at 07:21, Ed Maste <ema...@freebsd.org> wrote:
>> 
>> Author: emaste
>> Date: Wed May  3 14:21:18 2017
>> New Revision: 317744
>> URL: https://svnweb.freebsd.org/changeset/base/317744
>> 
>> Log:
>> makefs: make buf generic
>> 
>> it has nothing to do with ffs and will eventually be moved.
>> gc sectorsize.
>> 
>> NetBSD versions:
>> ffs.c1.58
>> ffs/buf.c1.14 1.18
>> ffs/buf.h1.8
>> 
>> Obtained from:   NetBSD
>> Sponsored by:The FreeBSD Foundation
> 
> This commit broke building releases and a number of tests: 
> https://ci.freebsd.org/job/FreeBSD-head-amd64-test/3003/ .
> Thanks,
> -Ngie

I reverted the `sectorsize` portion of this change in r317967 to 
unbreak the makefs and the fstyp tests. It seems that ffs_validate(..) isn’t 
called in all cases properly, or the structure isn’t being initialized properly.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317744 - in head/usr.sbin/makefs: . ffs

[Ngie Cooper (yaneurabeya)]

> On May 3, 2017, at 07:21, Ed Maste  wrote:
> 
> Author: emaste
> Date: Wed May  3 14:21:18 2017
> New Revision: 317744
> URL: https://svnweb.freebsd.org/changeset/base/317744
> 
> Log:
>  makefs: make buf generic
> 
>  it has nothing to do with ffs and will eventually be moved.
>  gc sectorsize.
> 
>  NetBSD versions:
>  ffs.c1.58
>  ffs/buf.c1.14 1.18
>  ffs/buf.h1.8
> 
>  Obtained from:   NetBSD
>  Sponsored by:The FreeBSD Foundation

This commit broke building releases and a number of tests: 
https://ci.freebsd.org/job/FreeBSD-head-amd64-test/3003/ .
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317942 - in head/usr.bin/csplit: . tests

[Ngie Cooper (yaneurabeya)]

> On May 8, 2017, at 08:51, Conrad Meyer  wrote:
> 
> Author: cem
> Date: Mon May  8 15:51:29 2017
> New Revision: 317942
> URL: https://svnweb.freebsd.org/changeset/base/317942
> 
> Log:
>  csplit(1): Fix extraneous output in edge case
> 
>  When the input to csplit contains fewer lines than the number of matches
>  specified, extra output was mistakenly included in some output files.
> 
>  Fix the bug and add a simple ATF regression test.
> 
>  PR:  219024
>  Submitted by:J.R. Oldroyd 
> 
> Added:
>  head/usr.bin/csplit/tests/
>  head/usr.bin/csplit/tests/Makefile   (contents, props changed)
>  head/usr.bin/csplit/tests/csplit_test.sh   (contents, props changed)
> Modified:
>  head/usr.bin/csplit/Makefile
>  head/usr.bin/csplit/csplit.c

Conrad,
Please add new directory entries to etc/mtree/BSD.tests.dist and do a 
full buildworld/installworld cycle next time to avoid build breakage (this 
isn’t the first time this has happened). Fixed up in r317949.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317632 - head/sys

[Ngie Cooper (yaneurabeya)]

> On May 1, 2017, at 09:34, Warner Losh  wrote:
> 
> On Mon, May 1, 2017 at 10:30 AM, Rodney W. Grimes
>  wrote:
>>> Author: ngie
>>> Date: Mon May  1 05:59:52 2017
>>> New Revision: 317632
>>> URL: https://svnweb.freebsd.org/changeset/base/317632
>>> 
>>> Log:
>>>  Fix "make cscope-clean" when .OBJDIR already exists
>>> 
>>>  The cscope generated files are always put in .CURDIR .
>> 
>> If this is writing to src dir then it should be fixed,
>> or have we abandoned all hope of readonly src tree?
> 
> This is only for the cscope target, which isn't part of the normal build.
> 
> readonly src tree works great still….

Hi Rod,
I can’t personally vouch for readonly src trees working in all cases, 
but readonly src trees work well for installation purposes (I use readonly obj 
and src trees at work when installing over NFS).
As Warner noted, “make cscope” is a developer target that aids with 
building the cscope database in a consistent manner. It doesn’t conform to the 
existing “norms” of build targets in the tree; changing the behavior doesn’t 
seem prudent at this point in time because it would violate POLA.
Thanks!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317631 - head/sys

[Ngie Cooper (yaneurabeya)]

> On May 1, 2017, at 09:28, Rodney W. Grimes  
> wrote:
> 
>> Author: ngie
>> Date: Mon May  1 05:54:33 2017
>> New Revision: 317631
>> URL: https://svnweb.freebsd.org/changeset/base/317631
>> 
>> Log:
>>  Fix "make cscope" after r317411
> 
> This commit is disconnect netnatm module from build
> 
> ?  Mixed 2 commit messages perhaps?

No, it was intentional (I made the readers read between the lines as to 
why this commit was necessary, so mea culpa).
r317411 removed sys/netatm, which broke the “make cscope” target, as it 
tried to iterate down that directory with find and failed.
Thanks!
-Ngie

$ svn log -c r317411

r317411 | brooks | 2017-04-25 10:00:08 -0700 (Tue, 25 Apr 2017) | 4 lines

Remove directories made empty by NATM, EISA, and IEEE488 removals.

Reported by:ak

$ svn diff -c r317411 --summarize
D   sbin/atm/atmconfig
D   sbin/atm
D   sys/netnatm
D   sys/dev/eisa
D   sys/dev/fatm
D   sys/dev/hatm
D   sys/dev/patm/genrtab
D   sys/dev/patm
D   sys/dev/ahb
D   sys/dev/ieee488
D   sys/dev/en
D   sys/dev/utopia
D   sys/modules/fatm
D   sys/modules/hatm
D   sys/modules/patm
D   sys/modules/ahb
D   sys/modules/en
D   sys/modules/utopia
D   sys/modules/aic7xxx/ahc/ahc_eisa
D   sys/modules/netgraph/atm/atm
D   usr.sbin/bsnmpd/modules/snmp_atm


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317600 - in head/sys: amd64/vmm/amd modules/vmm

[Ngie Cooper (yaneurabeya)]

> On Apr 30, 2017, at 07:35, Matteo Riondato  wrote:
> 
> 
>> On Apr 29, 2017, at 10:08 PM, Anish Gupta  wrote:
>> 
>> Author: anish
>> Date: Sun Apr 30 02:08:46 2017
>> New Revision: 317600
>> URL: https://svnweb.freebsd.org/changeset/base/317600
>> 
>> Log:
>> Add AMD IOMMU/AMD-Vi support in bhyve for passthrough/direct assignment to 
>> VMs. To enable AMD-Vi, set hw.vmm.amdvi.enable=1.
> 
> This knob should likely be documented somewhere, and maybe also "Relnotes: 
> yes" ?

Good catch.

The knob should be documented in the driver manpage.

Thanks!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317290 - head/tools/regression/geom_gpt

[Ngie Cooper (yaneurabeya)]

> On Apr 22, 2017, at 14:09, Rodney W. Grimes  
> wrote:

…

> I had seen that already when I made my post, that docuement is not going
> to stop someone from going "Oh, these are out of order I am going to sort
> them since I am here"  They well then probably have issues due to your next
> statement about WARNS and go hum, what is that all about.  And either
> investigate and hopefully find the right thing, or do more wrong things.
> 
> When #includes are out of order for good reason the source code file should
> be market as such and not dependend on the near 0 likelyhood someone
> is going to go read a man page to find out why.


The issue you’re noting is no different from someone removing/shuffling around 
another header in the C file. If someone does that, the least they need to do 
is build test their changes, and ideally they should runtime test the changes 
as well.

WARNS is insurance against someone sorting headers and things breaking again, 
because gctl_dump will not be defined (per the compiler message noted in the 
PR). At which point the party should do “man gctl_dump” and see the comment 
about stdio.h being required for it:

$ man gctl_dump
...
SYNOPSIS
 /* stdio.h is only required for `gctl_dump` */
 #include 
 #include 
…

Put differently, this is no different of an issue than anything else and I 
really don’t see the value in adding a comment stating that the order is such 
because of gctl_dump needing stdio.h to be #include’d first.

Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r317290 - head/tools/regression/geom_gpt

[Ngie Cooper (yaneurabeya)]

> On Apr 22, 2017, at 13:06, Rodney W. Grimes  
> wrote:
> 
>> Author: ngie
>> Date: Sat Apr 22 20:00:52 2017
>> New Revision: 317290
>> URL: https://svnweb.freebsd.org/changeset/base/317290
>> 
>> Log:
>>  Fix -Wimplicit-function-declaration compilation warning by moving libgeom.h
>>  #include below the stdio.h #include.
>> 
>>  gctl_dump(3) needs stdio.h, per reasoning noted in r317289.
>> 
>>  MFC after:  5 weeks
>>  PR: 218809
>>  Submitted by:   Chang-Hsien Tsai 
>>  Sponsored by:   Dell EMC Isilon
>> 
>> Modified:
>>  head/tools/regression/geom_gpt/test.c
>> 
>> Modified: head/tools/regression/geom_gpt/test.c
>> ==
>> --- head/tools/regression/geom_gpt/test.cSat Apr 22 19:32:23 2017
>> (r317289)
>> +++ head/tools/regression/geom_gpt/test.cSat Apr 22 20:00:52 2017
>> (r317290)
>> @@ -29,12 +29,12 @@ __FBSDID("$FreeBSD$");
>> 
>> #include 
>> #include 
>> -#include 
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>> +#include 
> #include   /* This is not in alphabetic order per r317289 */
>> 
>> struct retval {
>>  struct retval *retval;
> 
> Might it be a good idea to mark this in the test.c file as to why
> these are not sorted in order to prevent regressions of this in
> the future?

Please see r317289. There was an omission in the documentation that allowed 
this to happen.

This also won’t happen again given that the test is now being compiled with 
WARNS?= 6 :).

-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316874 - head/sys/kern

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 20:12, Peter Wemm  wrote:
> 
> On Friday, April 14, 2017 07:36:55 PM Peter Wemm wrote:
>> On Friday, April 14, 2017 02:14:16 PM Ngie Cooper wrote:
 On Apr 14, 2017, at 14:10, Maxim Sobolev  wrote:
 
 Peter, Ngie,
 
 Looks like out of that refactoring came a logical bug that is present in
 the head, which causes syslod to first to shutdown the socket for
 reading
 and then try to select/recv on it (which is somewhat stupid). And that
 issue has been masked by shutdown() on datagram socket becoming
 effectively a NOP in 11 & head 20 months ago. It only affects head
 though, 11-stable still has the old code which does not include that
 half-closed socket into the select list. Attached patch is expected to
 fix head, Peter, it would be nice if you can give it a try (restoring
 latest changes into uipc_sockets.c) and let me know if it helps.
 
 Thanks!
>>> 
>>> CCing hrs@ for input as he did the refactoring.
>>> Thanks!
>>> -Ngie
>>> 
>>> PS LGTM with the change. Will wait for feedback from wemm@.
>> 
>> This is definitely not working.  I get ENOSPC  and listen queue overflows on
>> /var/run/logpriv now.
>> 
>> Grabbing an old 10.3 /usr/sbin/syslogd and placing it on the top of the 12.x
>> one worked fine, aside from the include statements.
> 
> This can't be right:
> if (SecureMode || res->ai_family == AF_LOCAL) {
>/* Forbid communication in secure mode. */
>if (shutdown(s, SHUT_RD) < 0 &&
>errno != ENOTCONN) {
>logerror("shutdown");
>if (!Debug)
>die(0);
>}
>dprintf("listening on socket\n");
>sl_recv = NULL;
>   }
> 
> This appears to disable unix domain sockets like /var/run/log and
> /var/run/logpriv.

ACK. This looks like a fun bug.

-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 20:05, Rodney W. Grimes  
> wrote:
> 
>>> 
>>> On Apr 14, 2017, at 18:49, Rodney W. Grimes 
>>>  wrote:
>>> 
 On Friday, April 14, 2017 07:41:48 PM Ngie Cooper wrote:
> Author: ngie
> Date: Fri Apr 14 19:41:48 2017
> New Revision: 316938
> URL: https://svnweb.freebsd.org/changeset/base/316938
> 
> Log:
> savecore: fix space calculation with respect to `minfree` in 
> check_space(..)
> 
> - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>   representable data to INT_MAX. Check the values received from
>   strtoll(3), trimming trailing whitespace off the end to maintain
>   POLA.
> - Use `KiB` instead of `kB` when describing free space, total space,
>   etc. I am now fully aware of `KiB` being the IEC standard for 1024
>   bytes and `kB` being the IEC standard for 1000 bytes.
 
 I will just rant lightly that no one actually uses this in the real world.
 
 Good lucking finding a "16 GiB" DIMM on crucial.com or a 4Kin drive.  A
 kilobyte is a power of 2.  The End.
 
 (Next up we'll have to rename 4k displays to
 4k)
>>> 
>>> Do we use KiB, MiB, GiB,... any place else in the system?  I cant think of
>>> a place we do this, so please, lets not start doing this here?
>> 
>> humanize_number(3) from libutil uses IEC units.
> 
> And how many things bother to use this library function?  Do the
> ones that do call it produce the traditional output that has been
> around for 40 years?
> 
>>> Yes, these are newer standards, perhaps some day we should make a global
>>> switch to them, but lets not start mixing and matching things.
>> 
>> I understand and agree. I?m not 100% sold on that one way or another, but
>> since I was going to redo the number representation in save core with
>> humanize_number(3), because reading `KiB` is not ideal
>^^^
> I hope we are not already reading KiB anyplace….

I meant it’s a lot harder for humans to read `KiB` instead of 
``.

>> usability wise, and I don?t want to reinvent the wheel normalizing numbers
>> and printing out the unit.
>> 
>> Perhaps there should be a flag baked into humanize_number, etc for parsing 
>> IEC vs non-IEC unit values?
> 
> I dont think it parses anything, but as far as producing strings from values
> it already has an IEC flag: HN_IEC_PREFIXES, please lets not use this flag,
> and if we are using it anyplace lets see if we can remove that use.

I don’t see it used anywhere in the tree, based on a quick grep.

> Also be careful, this function only accepts signed int 64, which means
> we are not gona be able to use this in all places that probably need
> this, so perhaps a larger can of paint for a bigger bike shead is needed?

I don’t necessarily follow the above statement 100%. Are you warning against 
mass-conversion to libutil (if so, I agree… this was just a case where it 
really helps readability in savecore(8))?

Thanks!
-Ngie



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 19:40, Ngie Cooper (yaneurabeya) <yaneurab...@gmail.com> 
> wrote:
> 
>> 
>> On Apr 14, 2017, at 18:49, Rodney W. Grimes <free...@pdx.rh.cn85.dnsmgr.net> 
>> wrote:
>> 
>>> On Friday, April 14, 2017 07:41:48 PM Ngie Cooper wrote:
>>>> Author: ngie
>>>> Date: Fri Apr 14 19:41:48 2017
>>>> New Revision: 316938
>>>> URL: https://svnweb.freebsd.org/changeset/base/316938
>>>> 
>>>> Log:
>>>> savecore: fix space calculation with respect to `minfree` in 
>>>> check_space(..)
>>>> 
>>>> - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>>>>   representable data to INT_MAX. Check the values received from
>>>>   strtoll(3), trimming trailing whitespace off the end to maintain
>>>>   POLA.
>>>> - Use `KiB` instead of `kB` when describing free space, total space,
>>>>   etc. I am now fully aware of `KiB` being the IEC standard for 1024
>>>>   bytes and `kB` being the IEC standard for 1000 bytes.
>>> 
>>> I will just rant lightly that no one actually uses this in the real world.
>>> 
>>> Good lucking finding a "16 GiB" DIMM on crucial.com or a 4Kin drive.  A
>>> kilobyte is a power of 2.  The End.
>>> 
>>> (Next up we'll have to rename 4k displays to
>>> 4k)
>> 
>> Do we use KiB, MiB, GiB,... any place else in the system?  I cant think of
>> a place we do this, so please, lets not start doing this here?
> 
> humanize_number(3) from libutil uses IEC units.
> 
>> Yes, these are newer standards, perhaps some day we should make a global
>> switch to them, but lets not start mixing and matching things.
> 
> I understand and agree. I’m not 100% sold on that one way or another, but 
> since I was going to redo the number representation in save core with 
> humanize_number(3), because reading `KiB` is not ideal 
> usability wise, and I don’t want to reinvent the wheel normalizing numbers 
> and printing out the unit.

*unit. —> *unit, KiB seemed like a logical next step after discussing it at 
long length in the CR.

-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 18:49, Rodney W. Grimes  
> wrote:
> 
>> On Friday, April 14, 2017 07:41:48 PM Ngie Cooper wrote:
>>> Author: ngie
>>> Date: Fri Apr 14 19:41:48 2017
>>> New Revision: 316938
>>> URL: https://svnweb.freebsd.org/changeset/base/316938
>>> 
>>> Log:
>>>  savecore: fix space calculation with respect to `minfree` in 
>>> check_space(..)
>>> 
>>>  - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>>>representable data to INT_MAX. Check the values received from
>>>strtoll(3), trimming trailing whitespace off the end to maintain
>>>POLA.
>>>  - Use `KiB` instead of `kB` when describing free space, total space,
>>>etc. I am now fully aware of `KiB` being the IEC standard for 1024
>>>bytes and `kB` being the IEC standard for 1000 bytes.
>> 
>> I will just rant lightly that no one actually uses this in the real world.
>> 
>> Good lucking finding a "16 GiB" DIMM on crucial.com or a 4Kin drive.  A
>> kilobyte is a power of 2.  The End.
>> 
>> (Next up we'll have to rename 4k displays to
>> 4k)
> 
> Do we use KiB, MiB, GiB,... any place else in the system?  I cant think of
> a place we do this, so please, lets not start doing this here?

humanize_number(3) from libutil uses IEC units.

> Yes, these are newer standards, perhaps some day we should make a global
> switch to them, but lets not start mixing and matching things.

I understand and agree. I’m not 100% sold on that one way or another, but since 
I was going to redo the number representation in save core with 
humanize_number(3), because reading `KiB` is not ideal 
usability wise, and I don’t want to reinvent the wheel normalizing numbers and 
printing out the unit.

Perhaps there should be a flag baked into humanize_number, etc for parsing IEC 
vs non-IEC unit values?

Thanks for the input :)!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 2:40 PM, John Baldwin  wrote:
> 
> On Friday, April 14, 2017 07:41:48 PM Ngie Cooper wrote:
>> Author: ngie
>> Date: Fri Apr 14 19:41:48 2017
>> New Revision: 316938
>> URL: https://svnweb.freebsd.org/changeset/base/316938
>> 
>> Log:
>>  savecore: fix space calculation with respect to `minfree` in check_space(..)
>> 
>>  - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>>representable data to INT_MAX. Check the values received from
>>strtoll(3), trimming trailing whitespace off the end to maintain
>>POLA.
>>  - Use `KiB` instead of `kB` when describing free space, total space,
>>etc. I am now fully aware of `KiB` being the IEC standard for 1024
>>bytes and `kB` being the IEC standard for 1000 bytes.
> 
> I will just rant lightly that no one actually uses this in the real world.
> 
> Good lucking finding a "16 GiB" DIMM on crucial.com  or 
> a 4Kin drive.  A
> kilobyte is a power of 2.  The End.
> 
> (Next up we'll have to rename 4k displays to
> 4k)


https://media0.giphy.com/media/7rj2ZgttvgomY/giphy.gif 
 :P.
-Ngie


signature.asc
Description: Message signed with OpenPGP

Re: svn commit: r316874 - head/sys/kern

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 14:10, Maxim Sobolev  wrote:
> 
> Peter, Ngie,
> 
> Looks like out of that refactoring came a logical bug that is present in the 
> head, which causes syslod to first to shutdown the socket for reading and 
> then try to select/recv on it (which is somewhat stupid). And that issue has 
> been masked by shutdown() on datagram socket becoming effectively a NOP in 11 
> & head 20 months ago. It only affects head though, 11-stable still has the 
> old code which does not include that half-closed socket into the select list. 
> Attached patch is expected to fix head, Peter, it would be nice if you can 
> give it a try (restoring latest changes into uipc_sockets.c) and let me know 
> if it helps.
> 
> Thanks!

CCing hrs@ for input as he did the refactoring.
Thanks!
-Ngie

PS LGTM with the change. Will wait for feedback from wemm@.


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 13:48, Larry Rosenman  wrote:

…

> No can do, the rest of the space is pool.

>_>… that sucks. This is part of the reason why I still make my swap partitions 
>greater than or equal to the amount of memory I have in systems :/..
The best recommendation I have is either: wait for compressed dump support, 
repartition/shuffle around data on your disks so the configuration fits, or try 
and get ccd to work *shrugs*.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 13:46, Slawa Olhovchenkov <s...@zxy.spb.ru> wrote:
> 
> On Fri, Apr 14, 2017 at 01:45:22PM -0700, Ngie Cooper (yaneurabeya) wrote:
> 
>> 
>>> On Apr 14, 2017, at 13:42, Slawa Olhovchenkov <s...@zxy.spb.ru> wrote:
>> 
>> …
>> 
>>> No, something like:
>>> % swapinfo
>>> Device  1K-blocks UsedAvail Capacity
>>> /dev/ada0p2  335544320 33554432 0%
>>> /dev/ada1p2  335544320 33554432 0%
>>> Total671088640 67108864 0%
>> 
>> Striping dumps across multiple swap devices isn’t supported. Sorry.
> 
> What about support in the future?

*shrugs* I don’t have a crystal ball to say yay or nay, but saving full dumps 
out to disk seems foolhardy to say the least. Shrinking down the dataset seems 
like a more sensible problem to solve than striping data across disks.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 13:43, Larry Rosenman <l...@lerctr.org> wrote:
> 
> On 4/14/17, 3:39 PM, "Ngie Cooper (yaneurabeya)" 
> <owner-svn-src-...@freebsd.org on behalf of yaneurab...@gmail.com> wrote:
> 
>> On Apr 14, 2017, at 13:37, Larry Rosenman <l...@lerctr.org> wrote:
>> 
>> On 4/14/17, 3:33 PM, "Ngie Cooper (yaneurabeya)" <yaneurab...@gmail.com> 
>> wrote:
>> 
>>> On Apr 14, 2017, at 13:26, Larry Rosenman <l...@lerctr.org> wrote:
>>> 
>>> On 4/14/17, 3:19 PM, "Ngie Cooper (yaneurabeya)" 
>>> <owner-svn-src-...@freebsd.org on behalf of yaneurab...@gmail.com> wrote:
>>> 
>>>> On Apr 14, 2017, at 13:14, Slawa Olhovchenkov <s...@zxy.spb.ru> wrote:
>>>> 
>>>> On Fri, Apr 14, 2017 at 01:49:51PM -0600, Alan Somers wrote:
>>>> 
>>>>> On Fri, Apr 14, 2017 at 1:41 PM, Ngie Cooper <n...@freebsd.org> wrote:
>>>>>> Author: ngie
>>>>>> Date: Fri Apr 14 19:41:48 2017
>>>>>> New Revision: 316938
>>>>>> URL: https://svnweb.freebsd.org/changeset/base/316938
>>>>>> 
>>>>>> Log:
>>>>>> savecore: fix space calculation with respect to `minfree` in 
>>>>>> check_space(..)
>>>>>> 
>>>>>> - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>>>>>> representable data to INT_MAX. Check the values received from
>>>>>> strtoll(3), trimming trailing whitespace off the end to maintain
>>>>>> POLA.
>>>>>> - Use `KiB` instead of `kB` when describing free space, total space,
>>>>>> etc. I am now fully aware of `KiB` being the IEC standard for 1024
>>>>>> bytes and `kB` being the IEC standard for 1000 bytes.
>>>>>> - Store available number of KiB in `available` so it can be more
>>>>>> easily queried and compared to ensure that there are enough KiB to
>>>>>> store the dump image on disk.
>>>>>> - Print out the reserved space on disk, per `minfree`, so end-users
>>>>>> can troubleshoot why check_space(..) is reporting that there isn't
>>>>>> enough free space.
>>>>>> 
>>>>>> MFC after:7 weeks
>>>>>> Reviewed by:  Anton Rang <r...@acm.com> (earlier diff), cem (earlier 
>>>>>> diff)
>>>>>> Tested with:  positive/negative cases (see review); make tinderbox
>>>>>> Sponsored by: Dell EMC Isilon
>>>>>> Differential Revision:D10379
>>>>> 
>>>>> The free space calculation is still uselessly conservative, because it
>>>>> doesn't account for the fact that core dumps will always be either
>>>>> spare or compressed.  The result is that savecore will frequently
>>>>> refuse to save corefiles even when there's plenty of space.  I
>>>>> proposed removing the space check altogether in
>>>>> https://reviews.freebsd.org/D2587.  However, I agreed to wait until
>>>>> after the compressed core dump feature was merged, because then mostly
>>>>> accurate space checks will be possible.  AFAIK the compressed core
>>>>> dump feature still hasn't been finished.
>>>> 
>>>> Is posible (in the future) to use multiple swaps (on multiple disks)
>>>> for save core dumps?
>>> 
>>>  Multiple swap devices is already handled by savecore(8), if one uses 
>>> fstab(5) or dumpon(8). Otherwise, you must invoke savecore(8) on individual 
>>> devices.
>>> 
>>>  As far as saving to multiple disks is concerned, I would hope that one is 
>>> using a redundancy capable filesystem (zfs) or RAID-like technology 
>>> (gmirror, graid, LSI Fusion’s RAID product line) to stripe and/or mirror 
>>> the data across multiple disks.
>> 
>>   …
>> 
>>> How do I use multiple devices to have the system dump on all of my swap?  I 
>>> got a message about not enough space, but there (I think) was enough 
>>> between multiple drives….
>> 
>>   Something like:
>> 
>>   - Create a zpool
>>   - Mount zpool to /crashdumps
>>   - Change dumpdir in /etc/rc.conf to be /crashdumps, e.g., echo 
>> ‘dumpdir=/crashdumps’
>> 
>>   ?
>>   HTH,
>>   -Ngie
>> 
>>   PS The issue with lack of space might be the issue that Alan brought up 
>> earlier with c

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 13:42, Slawa Olhovchenkov  wrote:

…

> No, something like:
> % swapinfo
> Device  1K-blocks UsedAvail Capacity
> /dev/ada0p2  335544320 33554432 0%
> /dev/ada1p2  335544320 33554432 0%
> Total671088640 67108864 0%

Striping dumps across multiple swap devices isn’t supported. Sorry.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 13:37, Larry Rosenman <l...@lerctr.org> wrote:
> 
> On 4/14/17, 3:33 PM, "Ngie Cooper (yaneurabeya)" <yaneurab...@gmail.com> 
> wrote:
> 
>> On Apr 14, 2017, at 13:26, Larry Rosenman <l...@lerctr.org> wrote:
>> 
>> On 4/14/17, 3:19 PM, "Ngie Cooper (yaneurabeya)" 
>> <owner-svn-src-...@freebsd.org on behalf of yaneurab...@gmail.com> wrote:
>> 
>>> On Apr 14, 2017, at 13:14, Slawa Olhovchenkov <s...@zxy.spb.ru> wrote:
>>> 
>>> On Fri, Apr 14, 2017 at 01:49:51PM -0600, Alan Somers wrote:
>>> 
>>>> On Fri, Apr 14, 2017 at 1:41 PM, Ngie Cooper <n...@freebsd.org> wrote:
>>>>> Author: ngie
>>>>> Date: Fri Apr 14 19:41:48 2017
>>>>> New Revision: 316938
>>>>> URL: https://svnweb.freebsd.org/changeset/base/316938
>>>>> 
>>>>> Log:
>>>>> savecore: fix space calculation with respect to `minfree` in 
>>>>> check_space(..)
>>>>> 
>>>>> - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>>>>>  representable data to INT_MAX. Check the values received from
>>>>>  strtoll(3), trimming trailing whitespace off the end to maintain
>>>>>  POLA.
>>>>> - Use `KiB` instead of `kB` when describing free space, total space,
>>>>>  etc. I am now fully aware of `KiB` being the IEC standard for 1024
>>>>>  bytes and `kB` being the IEC standard for 1000 bytes.
>>>>> - Store available number of KiB in `available` so it can be more
>>>>>  easily queried and compared to ensure that there are enough KiB to
>>>>>  store the dump image on disk.
>>>>> - Print out the reserved space on disk, per `minfree`, so end-users
>>>>>  can troubleshoot why check_space(..) is reporting that there isn't
>>>>>  enough free space.
>>>>> 
>>>>> MFC after:7 weeks
>>>>> Reviewed by:  Anton Rang <r...@acm.com> (earlier diff), cem (earlier diff)
>>>>> Tested with:  positive/negative cases (see review); make tinderbox
>>>>> Sponsored by: Dell EMC Isilon
>>>>> Differential Revision:D10379
>>>> 
>>>> The free space calculation is still uselessly conservative, because it
>>>> doesn't account for the fact that core dumps will always be either
>>>> spare or compressed.  The result is that savecore will frequently
>>>> refuse to save corefiles even when there's plenty of space.  I
>>>> proposed removing the space check altogether in
>>>> https://reviews.freebsd.org/D2587.  However, I agreed to wait until
>>>> after the compressed core dump feature was merged, because then mostly
>>>> accurate space checks will be possible.  AFAIK the compressed core
>>>> dump feature still hasn't been finished.
>>> 
>>> Is posible (in the future) to use multiple swaps (on multiple disks)
>>> for save core dumps?
>> 
>>   Multiple swap devices is already handled by savecore(8), if one uses 
>> fstab(5) or dumpon(8). Otherwise, you must invoke savecore(8) on individual 
>> devices.
>> 
>>   As far as saving to multiple disks is concerned, I would hope that one is 
>> using a redundancy capable filesystem (zfs) or RAID-like technology 
>> (gmirror, graid, LSI Fusion’s RAID product line) to stripe and/or mirror the 
>> data across multiple disks.
> 
>…
> 
>> How do I use multiple devices to have the system dump on all of my swap?  I 
>> got a message about not enough space, but there (I think) was enough between 
>> multiple drives….
> 
>Something like:
> 
>- Create a zpool
>- Mount zpool to /crashdumps
>- Change dumpdir in /etc/rc.conf to be /crashdumps, e.g., echo 
> ‘dumpdir=/crashdumps’
> 
>?
>HTH,
>-Ngie
> 
>PS The issue with lack of space might be the issue that Alan brought up 
> earlier with compressed dumps and overly conservative free space checks, or 
> it might be the fact that dumpdir (default: /var/crash) is full.
> 
> 
> I was talking about the actual crashdump to swap by the system.  /var/crash 
> has 10T of space (my root pool).

If your memory is bigger than your swap, you’re unfortunately not able to save 
the mini dump if the size of the saved pages exceed the space on swap. I think 
this is where markj’s compressed dumps feature will come in handy.
Thanks!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 13:26, Larry Rosenman <l...@lerctr.org> wrote:
> 
> On 4/14/17, 3:19 PM, "Ngie Cooper (yaneurabeya)" 
> <owner-svn-src-...@freebsd.org on behalf of yaneurab...@gmail.com> wrote:
> 
>> On Apr 14, 2017, at 13:14, Slawa Olhovchenkov <s...@zxy.spb.ru> wrote:
>> 
>> On Fri, Apr 14, 2017 at 01:49:51PM -0600, Alan Somers wrote:
>> 
>>> On Fri, Apr 14, 2017 at 1:41 PM, Ngie Cooper <n...@freebsd.org> wrote:
>>>> Author: ngie
>>>> Date: Fri Apr 14 19:41:48 2017
>>>> New Revision: 316938
>>>> URL: https://svnweb.freebsd.org/changeset/base/316938
>>>> 
>>>> Log:
>>>> savecore: fix space calculation with respect to `minfree` in 
>>>> check_space(..)
>>>> 
>>>> - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>>>>   representable data to INT_MAX. Check the values received from
>>>>   strtoll(3), trimming trailing whitespace off the end to maintain
>>>>   POLA.
>>>> - Use `KiB` instead of `kB` when describing free space, total space,
>>>>   etc. I am now fully aware of `KiB` being the IEC standard for 1024
>>>>   bytes and `kB` being the IEC standard for 1000 bytes.
>>>> - Store available number of KiB in `available` so it can be more
>>>>   easily queried and compared to ensure that there are enough KiB to
>>>>   store the dump image on disk.
>>>> - Print out the reserved space on disk, per `minfree`, so end-users
>>>>   can troubleshoot why check_space(..) is reporting that there isn't
>>>>   enough free space.
>>>> 
>>>> MFC after:7 weeks
>>>> Reviewed by:  Anton Rang <r...@acm.com> (earlier diff), cem (earlier diff)
>>>> Tested with:  positive/negative cases (see review); make tinderbox
>>>> Sponsored by: Dell EMC Isilon
>>>> Differential Revision:D10379
>>> 
>>> The free space calculation is still uselessly conservative, because it
>>> doesn't account for the fact that core dumps will always be either
>>> spare or compressed.  The result is that savecore will frequently
>>> refuse to save corefiles even when there's plenty of space.  I
>>> proposed removing the space check altogether in
>>> https://reviews.freebsd.org/D2587.  However, I agreed to wait until
>>> after the compressed core dump feature was merged, because then mostly
>>> accurate space checks will be possible.  AFAIK the compressed core
>>> dump feature still hasn't been finished.
>> 
>> Is posible (in the future) to use multiple swaps (on multiple disks)
>> for save core dumps?
> 
>Multiple swap devices is already handled by savecore(8), if one uses 
> fstab(5) or dumpon(8). Otherwise, you must invoke savecore(8) on individual 
> devices.
> 
>As far as saving to multiple disks is concerned, I would hope that one is 
> using a redundancy capable filesystem (zfs) or RAID-like technology (gmirror, 
> graid, LSI Fusion’s RAID product line) to stripe and/or mirror the data 
> across multiple disks.

…

> How do I use multiple devices to have the system dump on all of my swap?  I 
> got a message about not enough space, but there (I think) was enough between 
> multiple drives….

Something like:

- Create a zpool
- Mount zpool to /crashdumps
- Change dumpdir in /etc/rc.conf to be /crashdumps, e.g., echo 
‘dumpdir=/crashdumps’

?
HTH,
-Ngie

PS The issue with lack of space might be the issue that Alan brought up earlier 
with compressed dumps and overly conservative free space checks, or it might be 
the fact that dumpdir (default: /var/crash) is full.


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r316938 - head/sbin/savecore

[Ngie Cooper (yaneurabeya)]

> On Apr 14, 2017, at 13:14, Slawa Olhovchenkov  wrote:
> 
> On Fri, Apr 14, 2017 at 01:49:51PM -0600, Alan Somers wrote:
> 
>> On Fri, Apr 14, 2017 at 1:41 PM, Ngie Cooper  wrote:
>>> Author: ngie
>>> Date: Fri Apr 14 19:41:48 2017
>>> New Revision: 316938
>>> URL: https://svnweb.freebsd.org/changeset/base/316938
>>> 
>>> Log:
>>>  savecore: fix space calculation with respect to `minfree` in 
>>> check_space(..)
>>> 
>>>  - Use strtoll(3) instead of atoi(3), because atoi(3) limits the
>>>representable data to INT_MAX. Check the values received from
>>>strtoll(3), trimming trailing whitespace off the end to maintain
>>>POLA.
>>>  - Use `KiB` instead of `kB` when describing free space, total space,
>>>etc. I am now fully aware of `KiB` being the IEC standard for 1024
>>>bytes and `kB` being the IEC standard for 1000 bytes.
>>>  - Store available number of KiB in `available` so it can be more
>>>easily queried and compared to ensure that there are enough KiB to
>>>store the dump image on disk.
>>>  - Print out the reserved space on disk, per `minfree`, so end-users
>>>can troubleshoot why check_space(..) is reporting that there isn't
>>>enough free space.
>>> 
>>>  MFC after:7 weeks
>>>  Reviewed by:  Anton Rang  (earlier diff), cem (earlier diff)
>>>  Tested with:  positive/negative cases (see review); make tinderbox
>>>  Sponsored by: Dell EMC Isilon
>>>  Differential Revision:D10379
>> 
>> The free space calculation is still uselessly conservative, because it
>> doesn't account for the fact that core dumps will always be either
>> spare or compressed.  The result is that savecore will frequently
>> refuse to save corefiles even when there's plenty of space.  I
>> proposed removing the space check altogether in
>> https://reviews.freebsd.org/D2587.  However, I agreed to wait until
>> after the compressed core dump feature was merged, because then mostly
>> accurate space checks will be possible.  AFAIK the compressed core
>> dump feature still hasn't been finished.
> 
> Is posible (in the future) to use multiple swaps (on multiple disks)
> for save core dumps?

Multiple swap devices is already handled by savecore(8), if one uses fstab(5) 
or dumpon(8). Otherwise, you must invoke savecore(8) on individual devices.

As far as saving to multiple disks is concerned, I would hope that one is using 
a redundancy capable filesystem (zfs) or RAID-like technology (gmirror, graid, 
LSI Fusion’s RAID product line) to stripe and/or mirror the data across 
multiple disks.

Thanks!
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail