Re: svn commit: r368163 - in head: sbin/ifconfig sys/dev/if_wg sys/dev/if_wg/include sys/dev/if_wg/include/crypto sys/dev/if_wg/include/sys sys/dev/if_wg/include/zinc sys/dev/if_wg/module sys/dev/if_w

[Matthew Macy]

I worry about the incredible mess of header dependencies that the linuxkpi
is prone to introducing. I’m happy to review any proposed changes to that
effect, but do not feel like it’s important enough to make a priority.


Thanks.

-M

On Mon, Nov 30, 2020 at 08:33 Bjoern A. Zeeb 
wrote:

> On 30 Nov 2020, at 14:53, cglogic wrote:
>
> > So you propose to make it dependent on linuxkpi? What have to do a
> > user who does not compile linuxkpi, but wants to use if_wg?
>
> If we’d move the implementations into linuxkpi you’d have to compile
> it to use if_wg.
> So that this however is only a question.
>
> Effectively you are using parts of linuxkpi now already, just
> copy (duplicated) into local files under if_wg;  for as much as I
> can see for most of that would however not require linuxkpi to be
> loaded, just present at compile time like any other kernel headers.
>
> /bz
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r368163 - in head: sbin/ifconfig sys/dev/if_wg sys/dev/if_wg/include sys/dev/if_wg/include/crypto sys/dev/if_wg/include/sys sys/dev/if_wg/include/zinc sys/dev/if_wg/module sys/dev/if_w

[Matthew Macy]

Just what I put on the review. I’ll put together a man page the week
after next. Sorry for the delay.

-M

On Mon, Nov 30, 2020 at 10:16 Michael Tuexen  wrote:

>
>
> > On 29. Nov 2020, at 20:38, Matt Macy  wrote:
> >
> > Author: mmacy
> > Date: Sun Nov 29 19:38:03 2020
> > New Revision: 368163
> > URL: https://svnweb.freebsd.org/changeset/base/368163
> >
> > Log:
> >  Import kernel WireGuard support
> >
> >  Data path largely shared with the OpenBSD implementation by
> >  Matt Dunwoodie 
> >
> >  Reviewed by: gre...@freebsd.org
> >  MFC after:   1 month
> >  Sponsored by:Rubicon LLC, (Netgate)
> >  Differential Revision:   https://reviews.freebsd.org/D26137
> >
> Is there some documentation available somewhere how to setup a tunnel?
>
> Best regards
> Michael
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r367577 - in head: share/mk sys/conf tools/build/options

[Matthew Macy]

These flags aren't defined by default when building external kernel modules:

gmake[2]: Entering directory '/usr/home/matt/devel/ZoF/module'
env -u MAKEFLAGS make -C /home/matt/devel/ZoF/module -f Makefile.bsd -w
make[3]: Entering directory `/home/matt/devel/ZoF/module'
make[3]: "/usr/home/matt/devel/freebsd/sys/conf/kern.mk" line 233:
Malformed conditional (${MK_INIT_ALL_ZERO} == "yes")
make[3]: Fatal errors encountered -- cannot continue
make[3]: stopped in /home/matt/devel/ZoF/module
gmake[2]: *** [Makefile:53: modules-FreeBSD] Error 1
gmake[2]: Leaving directory '/usr/home/matt/devel/ZoF/module'

On Tue, Nov 10, 2020 at 11:15 AM Brooks Davis  wrote:
>
> Author: brooks
> Date: Tue Nov 10 19:15:13 2020
> New Revision: 367577
> URL: https://svnweb.freebsd.org/changeset/base/367577
>
> Log:
>   Support initializing stack variables on function entry
>
>   There are two options:
>- WITH_INIT_ALL_ZERO: Zero all variables on the stack.
>- WITH_INIT_ALL_PATTERN: Initialize variables with well-defined patterns.
>
>   The exact pattern are a compiler implementation detail and vary by type.
>   They are somewhat documented in the LLVM commit message:
>   https://reviews.llvm.org/rL349442
>   I've used WITH_INIT_ALL_* to match Microsoft's InitAll feature rather
>   than naming them after the LLVM specific compiler flags.
>
>   In a range of consumer products, options like these are used in
>   both debug and production builds with debugs builds using patterns
>   (intended to provoke crashes on use of uninitialized values) and
>   production using zeros (deemed more likely to lead to harmless
>   misbehavior or NULL-pointer dereferences).
>
>   Reviewed by:  emaste
>   Obtained from:CheriBSD
>   Sponsored by: DARPA
>   Differential Revision:https://reviews.freebsd.org/D27131
>
> Added:
>   head/tools/build/options/WITH_INIT_ALL_PATTERN   (contents, props changed)
>   head/tools/build/options/WITH_INIT_ALL_ZERO   (contents, props changed)
> Modified:
>   head/share/mk/bsd.compiler.mk
>   head/share/mk/bsd.lib.mk
>   head/share/mk/bsd.opts.mk
>   head/share/mk/bsd.prog.mk
>   head/sys/conf/kern.mk
>
> Modified: head/share/mk/bsd.compiler.mk
> ==
> --- head/share/mk/bsd.compiler.mk   Tue Nov 10 19:09:35 2020
> (r367576)
> +++ head/share/mk/bsd.compiler.mk   Tue Nov 10 19:15:13 2020
> (r367577)
> @@ -24,6 +24,7 @@
>  # - c++11: supports full (or nearly full) C++11 programming environment.
>  # - retpoline: supports the retpoline speculative execution vulnerability
>  #  mitigation.
> +# - init-all:  supports stack variable initialization.
>  #
>  # These variables with an X_ prefix will also be provided if XCC is set.
>  #
> @@ -214,7 +215,7 @@ ${X_}COMPILER_FEATURES= c++11 c++14
>  ${X_}COMPILER_FEATURES+=   c++17
>  .endif
>  .if ${${X_}COMPILER_TYPE} == "clang"
> -${X_}COMPILER_FEATURES+=   retpoline
> +${X_}COMPILER_FEATURES+=   retpoline init-all
>  .endif
>
>  .else
>
> Modified: head/share/mk/bsd.lib.mk
> ==
> --- head/share/mk/bsd.lib.mkTue Nov 10 19:09:35 2020(r367576)
> +++ head/share/mk/bsd.lib.mkTue Nov 10 19:15:13 2020(r367577)
> @@ -85,6 +85,25 @@ LDFLAGS+= -Wl,-zretpolineplt
>  .endif
>  .endif
>
> +# Initialize stack variables on function entry
> +.if ${MK_INIT_ALL_ZERO} == "yes"
> +.if ${COMPILER_FEATURES:Minit-all}
> +CFLAGS+= -ftrivial-auto-var-init=zero \
> +-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> +CXXFLAGS+= -ftrivial-auto-var-init=zero \
> +-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> +.else
> +.warning InitAll (zeros) requested but not support by compiler
> +.endif
> +.elif ${MK_INIT_ALL_PATTERN} == "yes"
> +.if ${COMPILER_FEATURES:Minit-all}
> +CFLAGS+= -ftrivial-auto-var-init=pattern
> +CXXFLAGS+= -ftrivial-auto-var-init=pattern
> +.else
> +.warning InitAll (pattern) requested but not support by compiler
> +.endif
> +.endif
> +
>  .if ${MK_DEBUG_FILES} != "no" && empty(DEBUG_FLAGS:M-g) && \
>  empty(DEBUG_FLAGS:M-gdwarf*)
>  CFLAGS+= ${DEBUG_FILES_CFLAGS}
>
> Modified: head/share/mk/bsd.opts.mk
> ==
> --- head/share/mk/bsd.opts.mk   Tue Nov 10 19:09:35 2020(r367576)
> +++ head/share/mk/bsd.opts.mk   Tue Nov 10 19:15:13 2020(r367577)
> @@ -71,6 +71,8 @@ __DEFAULT_NO_OPTIONS = \
>  BIND_NOW \
>  CCACHE_BUILD \
>  CTF \
> +INIT_ALL_PATTERN \
> +INIT_ALL_ZERO \
>  INSTALL_AS_USER \
>  PIE \
>  RETPOLINE \
> @@ -84,6 +86,10 @@ __DEFAULT_DEPENDENT_OPTIONS = \
>
>
>  .include 
> +
> +.if ${MK_INIT_ALL_PATTERN} == "yes" && ${MK_INIT_ALL_ZERO} == "yes"
> +.error WITH_INIT_ALL_PATTERN and WITH_INIT_ALL_ZERO are mutually exclusive.
> +.endif
>
>  #

Re: svn commit: r364806 - in head/sys/contrib/openzfs: include/os/freebsd/spl/sys module/os/freebsd/spl

[Matthew Macy]

Do not commit directly to sys/contrib. PR, vendor branch update, then merge.

On Wed, Aug 26, 2020 at 12:29 AM Toomas Soome  wrote:
>
> Author: tsoome
> Date: Wed Aug 26 07:29:17 2020
> New Revision: 364806
> URL: https://svnweb.freebsd.org/changeset/base/364806
>
> Log:
>   remove pragma ident lines
>
>   The #pragma ident is historical relict and not needed any more, this
>   pragma is actually unknown for common compilers and is only causing
>   trouble.
>
> Modified:
>   head/sys/contrib/openzfs/include/os/freebsd/spl/sys/acl_impl.h
>   head/sys/contrib/openzfs/include/os/freebsd/spl/sys/cmn_err.h
>   head/sys/contrib/openzfs/include/os/freebsd/spl/sys/extdirent.h
>   head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list.h
>   head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list_impl.h
>   head/sys/contrib/openzfs/include/os/freebsd/spl/sys/zmod.h
>   head/sys/contrib/openzfs/module/os/freebsd/spl/list.c
>
> Modified: head/sys/contrib/openzfs/include/os/freebsd/spl/sys/acl_impl.h
> ==
> --- head/sys/contrib/openzfs/include/os/freebsd/spl/sys/acl_impl.h  Wed 
> Aug 26 07:00:07 2020(r364805)
> +++ head/sys/contrib/openzfs/include/os/freebsd/spl/sys/acl_impl.h  Wed 
> Aug 26 07:29:17 2020(r364806)
> @@ -26,8 +26,6 @@
>  #ifndef _SYS_ACL_IMPL_H
>  #define_SYS_ACL_IMPL_H
>
> -#pragma ident  "%Z%%M% %I% %E% SMI"
> -
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
>
> Modified: head/sys/contrib/openzfs/include/os/freebsd/spl/sys/cmn_err.h
> ==
> --- head/sys/contrib/openzfs/include/os/freebsd/spl/sys/cmn_err.h   Wed 
> Aug 26 07:00:07 2020(r364805)
> +++ head/sys/contrib/openzfs/include/os/freebsd/spl/sys/cmn_err.h   Wed 
> Aug 26 07:29:17 2020(r364806)
> @@ -31,8 +31,6 @@
>  #ifndef _SYS_CMN_ERR_H
>  #define_SYS_CMN_ERR_H
>
> -#pragma ident  "%Z%%M% %I% %E% SMI"
> -
>  #if !defined(_ASM)
>  #include 
>  #endif
>
> Modified: head/sys/contrib/openzfs/include/os/freebsd/spl/sys/extdirent.h
> ==
> --- head/sys/contrib/openzfs/include/os/freebsd/spl/sys/extdirent.h Wed 
> Aug 26 07:00:07 2020(r364805)
> +++ head/sys/contrib/openzfs/include/os/freebsd/spl/sys/extdirent.h Wed 
> Aug 26 07:29:17 2020(r364806)
> @@ -26,8 +26,6 @@
>  #ifndef _SYS_EXTDIRENT_H
>  #define_SYS_EXTDIRENT_H
>
> -#pragma ident  "%Z%%M% %I% %E% SMI"
> -
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
>
> Modified: head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list.h
> ==
> --- head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list.h  Wed Aug 26 
> 07:00:07 2020(r364805)
> +++ head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list.h  Wed Aug 26 
> 07:29:17 2020(r364806)
> @@ -26,8 +26,6 @@
>  #ifndef_SYS_LIST_H
>  #define_SYS_LIST_H
>
> -#pragma ident  "%Z%%M% %I% %E% SMI"
> -
>  #include 
>
>  #ifdef __cplusplus
>
> Modified: head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list_impl.h
> ==
> --- head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list_impl.h Wed 
> Aug 26 07:00:07 2020(r364805)
> +++ head/sys/contrib/openzfs/include/os/freebsd/spl/sys/list_impl.h Wed 
> Aug 26 07:29:17 2020(r364806)
> @@ -27,8 +27,6 @@
>  #ifndef_SYS_LIST_IMPL_H
>  #define_SYS_LIST_IMPL_H
>
> -#pragma ident  "%Z%%M% %I% %E% SMI"
> -
>  #include 
>
>  #ifdef __cplusplus
>
> Modified: head/sys/contrib/openzfs/include/os/freebsd/spl/sys/zmod.h
> ==
> --- head/sys/contrib/openzfs/include/os/freebsd/spl/sys/zmod.h  Wed Aug 26 
> 07:00:07 2020(r364805)
> +++ head/sys/contrib/openzfs/include/os/freebsd/spl/sys/zmod.h  Wed Aug 26 
> 07:29:17 2020(r364806)
> @@ -27,8 +27,6 @@
>  #ifndef_ZMOD_H
>  #define_ZMOD_H
>
> -#pragma ident  "%Z%%M% %I% %E% SMI"
> -
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
>
> Modified: head/sys/contrib/openzfs/module/os/freebsd/spl/list.c
> ==
> --- head/sys/contrib/openzfs/module/os/freebsd/spl/list.c   Wed Aug 26 
> 07:00:07 2020(r364805)
> +++ head/sys/contrib/openzfs/module/os/freebsd/spl/list.c   Wed Aug 26 
> 07:29:17 2020(r364806)
> @@ -23,8 +23,6 @@
>   * Use is subject to license terms.
>   */
>
> -#pragma ident  "%Z%%M% %I% %E% SMI"
> -
>  /*
>   * Generic doubly-linked list implementation
>   */
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head

Re: svn commit: r362531 - in head: cddl/contrib/opensolaris/cmd/zfs cddl/contrib/opensolaris/lib/libzfs/common cddl/contrib/opensolaris/lib/libzfs_core/common sys/cddl/contrib/opensolaris/uts/common/f

[Matthew Macy]

Are you planning on MFCing this? That’s the main value to MFVs at this
point.

Thanks.

On Mon, Jun 22, 2020 at 23:42 Toomas Soome  wrote:

> Author: tsoome
> Date: Tue Jun 23 06:42:39 2020
> New Revision: 362531
> URL: https://svnweb.freebsd.org/changeset/base/362531
>
> Log:
>   MFOpenZFS: Add basic zfs ioc input nvpair validation
>
>   We want newer versions of libzfs_core to run against an existing
>   zfs kernel module (i.e. a deferred reboot or module reload after
>   an update).
>
>   Programmatically document, via a zfs_ioc_key_t, the valid arguments
>   for the ioc commands that rely on nvpair input arguments (i.e. non
>   legacy commands from libzfs_core). Automatically verify the expected
>   pairs before dispatching a command.
>
>   This initial phase focuses on the non-legacy ioctls. A follow-on
>   change can address the legacy ioctl input from the zfs_cmd_t.
>
>   The zfs_ioc_key_t for zfs_keys_channel_program looks like:
>
>   static const zfs_ioc_key_t zfs_keys_channel_program[] = {
>  {"program", DATA_TYPE_STRING,   0},
>  {"arg", DATA_TYPE_UNKNOWN,  0},
>  {"sync",DATA_TYPE_BOOLEAN_VALUE,ZK_OPTIONAL},
>  {"instrlimit",  DATA_TYPE_UINT64,   ZK_OPTIONAL},
>  {"memlimit",DATA_TYPE_UINT64,   ZK_OPTIONAL},
>   };
>
>   Introduce four input errors to identify specific input failures
>   (in addition to generic argument value errors like EINVAL, ERANGE,
>   EBADF, and E2BIG).
>
>   ZFS_ERR_IOC_CMD_UNAVAIL the ioctl number is not supported by kernel
>   ZFS_ERR_IOC_ARG_UNAVAIL an input argument is not supported by kernel
>   ZFS_ERR_IOC_ARG_REQUIRED a required input argument is missing
>   ZFS_ERR_IOC_ARG_BADTYPE an input argument has an invalid type
>
>   Reviewed by:  allanjude
>   Obtained from:OpenZFS
>   Sponsored by: Netflix, Klara Inc.
>   Differential Revision:https://reviews.freebsd.org/D25393
>
> Modified:
>   head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c
>   head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
>   head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c
>   head/cddl/contrib/opensolaris/lib/libzfs_core/common/libzfs_core.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>   head/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h
>
> Modified: head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c
>
> ==
> --- head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.cTue Jun 23
> 04:58:36 2020(r362530)
> +++ head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.cTue Jun 23
> 06:42:39 2020(r362531)
> @@ -7235,7 +7235,7 @@ zfs_do_bookmark(int argc, char **argv)
> fnvlist_free(nvl);
>
> if (ret != 0) {
> -   const char *err_msg;
> +   const char *err_msg = NULL;
> char errbuf[1024];
>
> (void) snprintf(errbuf, sizeof (errbuf),
> @@ -7259,11 +7259,13 @@ zfs_do_bookmark(int argc, char **argv)
> err_msg = "out of space";
> break;
> default:
> -   err_msg = "unknown error";
> +   (void) zfs_standard_error(g_zfs, ret, errbuf);
> break;
> }
> -   (void) fprintf(stderr, "%s: %s\n", errbuf,
> -   dgettext(TEXT_DOMAIN, err_msg));
> +   if (err_msg != NULL) {
> +   (void) fprintf(stderr, "%s: %s\n", errbuf,
> +   dgettext(TEXT_DOMAIN, err_msg));
> +   }
> }
>
> return (ret != 0);
> @@ -7280,7 +7282,7 @@ zfs_do_channel_program(int argc, char **argv)
> char c;
> char *progbuf, *filename, *poolname;
> size_t progsize, progread;
> -   nvlist_t *outnvl;
> +   nvlist_t *outnvl = NULL;
> uint64_t instrlimit = ZCP_DEFAULT_INSTRLIMIT;
> uint64_t memlimit = ZCP_DEFAULT_MEMLIMIT;
> boolean_t sync_flag = B_TRUE, json_output = B_FALSE;
> @@ -7420,7 +7422,8 @@ zfs_do_channel_program(int argc, char **argv)
>  * falling back on strerror() for an unexpected return
> code.
>  */
> char *errstring = NULL;
> -   if (nvlist_exists(outnvl, ZCP_RET_ERROR)) {
> +   const char *msg = gettext("Channel program execution
> failed");
> +   if (outnvl != NULL && nvlist_exists(outnvl,
> ZCP_RET_ERROR)) {
> (void) nvlist_lookup_string(outnvl,
> ZCP_RET_ERROR, );
> if (errstring == NULL)
> @@ -7449,12 +7452,11 @@ zfs_do_channel_program(int argc, char **argv)
> "programs must be run as root.";
> break;
> default:
> -   

Re: svn commit: r344817 - in head/sys: dev/e1000 net

[Matthew Macy]

This represents a misunderstanding of how defines are used. This left
the option open to the user to enable the use of larger than page size
buffers as it does enable better performance. Over the course of a
long uptime memory can get too fragmented. However, this left it open
to the end consumer.

I'd like to see this reverted with perhaps a better name for the
define and the addition of an explanatory comment.
Thanks.

-M

On Tue, Mar 5, 2019 at 11:13 AM Eric Joyner  wrote:
>
> Author: erj
> Date: Tue Mar  5 19:12:51 2019
> New Revision: 344817
> URL: https://svnweb.freebsd.org/changeset/base/344817
>
> Log:
>   Remove references to CONTIGMALLOC_WORKS in iflib and em
>
>   From Jake:
>   "The iflib_fl_setup() function tries to pick various buffer sizes based
>   on the max_frame_size value defined by the parent driver. However, this
>   code was wrapped under CONTIGMALLOC_WORKS, which was never actually
>   defined anywhere.
>
>   This same code pattern was used in if_em.c, likely trying to match
>   what iflib uses.
>
>   Since CONTIGMALLOC_WORKS is not defined, remove this dead code from
>   iflib_fl_setup and if_em.c
>
>   Given that various iflib drivers appear to be using a similar
>   calculation, it might be worth making this buffer size a value that the
>   driver can peek at in the future."
>
>   Submitted by: Jacob Keller 
>   Reviewed by:  shurd@
>   MFC after:1 week
>   Sponsored by: Intel Corporation
>   Differential Revision:https://reviews.freebsd.org/D19199
>
> Modified:
>   head/sys/dev/e1000/if_em.c
>   head/sys/net/iflib.c
>
> Modified: head/sys/dev/e1000/if_em.c
> ==
> --- head/sys/dev/e1000/if_em.c  Tue Mar  5 19:08:37 2019(r344816)
> +++ head/sys/dev/e1000/if_em.c  Tue Mar  5 19:12:51 2019(r344817)
> @@ -1276,15 +1276,8 @@ em_if_init(if_ctx_t ctx)
>  */
> if (adapter->hw.mac.max_frame_size <= 2048)
> adapter->rx_mbuf_sz = MCLBYTES;
> -#ifndef CONTIGMALLOC_WORKS
> else
> adapter->rx_mbuf_sz = MJUMPAGESIZE;
> -#else
> -   else if (adapter->hw.mac.max_frame_size <= 4096)
> -   adapter->rx_mbuf_sz = MJUMPAGESIZE;
> -   else
> -   adapter->rx_mbuf_sz = MJUM9BYTES;
> -#endif
> em_initialize_receive_unit(ctx);
>
> /* Use real VLAN Filter support? */
>
> Modified: head/sys/net/iflib.c
> ==
> --- head/sys/net/iflib.cTue Mar  5 19:08:37 2019(r344816)
> +++ head/sys/net/iflib.cTue Mar  5 19:12:51 2019(r344817)
> @@ -2187,17 +2187,8 @@ iflib_fl_setup(iflib_fl_t fl)
>  */
> if (sctx->isc_max_frame_size <= 2048)
> fl->ifl_buf_size = MCLBYTES;
> -#ifndef CONTIGMALLOC_WORKS
> else
> fl->ifl_buf_size = MJUMPAGESIZE;
> -#else
> -   else if (sctx->isc_max_frame_size <= 4096)
> -   fl->ifl_buf_size = MJUMPAGESIZE;
> -   else if (sctx->isc_max_frame_size <= 9216)
> -   fl->ifl_buf_size = MJUM9BYTES;
> -   else
> -   fl->ifl_buf_size = MJUM16BYTES;
> -#endif
> if (fl->ifl_buf_size > ctx->ifc_max_fl_buf_size)
> ctx->ifc_max_fl_buf_size = fl->ifl_buf_size;
> fl->ifl_cltype = m_gettype(fl->ifl_buf_size);
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344487 - in head/sys: conf gnu/gcov

[Matthew Macy]

World? It looks like there's a version mismatch between the kernel and modules.

-M

On Thu, Feb 28, 2019 at 7:11 PM Alan Somers  wrote:
>
> On Thu, Feb 28, 2019 at 6:40 PM Matthew Macy  wrote:
> >
> > to config add:
> > options LINDEBUGFS
> > options GCOV
> >
> > compile kernel with gcc (otherwise it will be a no-op)
> >
> > sysctl debug.gcov.enable=1
> >
> > mount -t debugfs debugfs /sys/kernel/debug
> >
> > (or wherever) and the output artifacts will appear under gcov/ > build path> - you need to be root to see the artifacts
> >
> > gcov can then generate the results as it would normally from the
> > profiling and the build time artifacts
> >
> > bug reports welcome
> >
> >
> > -M
>
> Here's a bug report: I rebuilt the kernel but boot fails with this
> error.  Do I need to rebuild world too?
>
> Mounting local filesystems:kldload: fdescfs.ko: lost base for reltab
> linker_load_file: /boot/kernel/fdescfs.ko - unsupported file type
> mount: fdescfs: Operation not supported by device
> Mounting /etc/fstab filesystems failed, will retry after root mount hold 
> release
> kldload: fdescfs.ko: lost base for reltab
> linker_load_file: /boot/kernel/fdescfs.ko - unsupported file type
> mount: fdescfs: Operation not supported by device
> .
> Mounting /etc/fstab filesystems failed, startup aborted
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344487 - in head/sys: conf gnu/gcov

[Matthew Macy]

to config add:
options LINDEBUGFS
options GCOV

compile kernel with gcc (otherwise it will be a no-op)

sysctl debug.gcov.enable=1

mount -t debugfs debugfs /sys/kernel/debug

(or wherever) and the output artifacts will appear under gcov/ - you need to be root to see the artifacts

gcov can then generate the results as it would normally from the
profiling and the build time artifacts

bug reports welcome


-M














On Thu, Feb 28, 2019 at 2:50 PM Alan Somers  wrote:
>
> On Sat, Feb 23, 2019 at 2:14 PM Matt Macy  wrote:
> >
> > Author: mmacy
> > Date: Sat Feb 23 21:14:00 2019
> > New Revision: 344487
> > URL: https://svnweb.freebsd.org/changeset/base/344487
> >
> > Log:
> >   gcov support
> >
> >   add gcov support and export results as files in debugfs
> >
> >   Reviewed by:  hps@
> >   MFC after:1 week
> >   Sponsored by: iX Systems
> >   Differential Revision:https://reviews.freebsd.org/D19260
> >
> > Added:
> >   head/sys/gnu/gcov/
> >   head/sys/gnu/gcov/gcc_4_7.c   (contents, props changed)
> >   head/sys/gnu/gcov/gcov.h   (contents, props changed)
> >   head/sys/gnu/gcov/gcov_fs.c   (contents, props changed)
> >   head/sys/gnu/gcov/gcov_subr.c   (contents, props changed)
> > Modified:
> >   head/sys/conf/files
> >   head/sys/conf/kern.mk
> >   head/sys/conf/kern.post.mk
> >   head/sys/conf/kern.pre.mk
> >   head/sys/conf/kmod.mk
> >   head/sys/conf/options
>
> On a more practical note, how does one use this new feature?  It looks
> like it could be quite useful.
> -Alan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344487 - in head/sys: conf gnu/gcov

[Matthew Macy]

On Tue, Feb 26, 2019 at 09:20 Rodney W. Grimes <
free...@pdx.rh.cn85.dnsmgr.net> wrote:

> > This has zero impact on the licensing disposition of the kernel as
> > distributed as it is only used for test kernels. Tests compiled with
> > coverage instrumentation run much slower than even debug, one would never
> > ship this.
>
> Shit happens, mistakes get made, and sadly the consequences for someone
> could be pretty sad.
>
> > You are very much in the minority being more concerned with ideological
> > purity than minimizing the decline in relevance of FreeBSD, much less
> > striving to increase its relevance.
>
> I am not in the minority when it comes to GPL code anyplace
> in our base system, did you not read what core said, did you
> not read the suggested revised license guideline text?
>
> This gcov code has to eventually go, sooner or later.


Unless there is a fully equivalent replacement, that will be another small
step towards Linux's complete hegemony.


>
> > On Tue, Feb 26, 2019 at 08:27 Rodney W. Grimes <
> > free...@pdx.rh.cn85.dnsmgr.net> wrote:
> >
> > > > On Mon, Feb 25, 2019 at 06:18:42PM -0800, Rodney W. Grimes wrote:
> > > > > > > The modest increase in activation energy for that task seems
> worth
> > > it
> > > > > > > for the short-term gains of reduced integration cost (this code
> > > will
> > > > > > > greatly improve our ZFS-on-Linux test coverage.)
> > > > > > >
> > > > > > > Rod rightly points out that we haven't accepted SPDX tags
> alone as
> > > > > > > license statements.  The standard GPL v2.0 boiler plate should
> be
> > > added
> > > > > > > to this file along side the tag.
> > > > > >
> > > > > > I've copied the full copyright attribution that is in the
> > > > > > corresponding files on Linux. Is there some reason why FreeBSD
> > > > > > requires the files to be inflated with the full license text
> where
> > > the
> > > > > > original lacks it?
> > > > >
> > > > > I think for a few reasons, I doubt you copied the whole
> distribution
> > > > > that this file came from, as I am sure that distribution included
> > > > > a LICENSE file.  Second if you actually read the GPL v2
> documentation
> > > > > and follow what it says it says you must do this, just because some
> > > > > one else does not follow the rules of what the GPL v2 says does not
> > > > > give us to knowingling not do it.  Third this is a particular
> > > dangerious
> > > > > area for BSD to be mixing a GPL code with its kernel, to my
> knowlege
> > > > > we have never had any gpl code in the kernel, no have we ever
> > > > > allowed it, but thats a seperate argument, that should be made.
> > > >
> > > > Would the arm64 DTS/DTB files count as "GPL code in the kernel?"
> > > >
> > > > I, too, would like less GPL in project, both in userland in kernel.
> > > > But, I can understand the desire for gcov. Note that I'm not
> > > > advocating either way that FreeBSD perform an action. ;)
> > >
> > > Didnt we just remove an inbase, compiling BSD licensed chunk of
> > > code called DRM and move it to ports.  So if that was possible
> > > this should be very rapidly applied here and this issue goes away.
> > >
> > > I am still shaking my head over this one.  Yes, there is some
> > > expediance to this.  Also could it not live on a project
> > > branch?  Like.. um.. the ZoL project branch?
> > >
> > > > Thanks,
> > > > Shawn Webb
> > > --
> > > Rod Grimes
> > > rgri...@freebsd.org
> > >
>
> --
> Rod Grimes
> rgri...@freebsd.org
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344487 - in head/sys: conf gnu/gcov

[Matthew Macy]

This has zero impact on the licensing disposition of the kernel as
distributed as it is only used for test kernels. Tests compiled with
coverage instrumentation run much slower than even debug, one would never
ship this.

You are very much in the minority being more concerned with ideological
purity than minimizing the decline in relevance of FreeBSD, much less
striving to increase its relevance.



On Tue, Feb 26, 2019 at 08:27 Rodney W. Grimes <
free...@pdx.rh.cn85.dnsmgr.net> wrote:

> > On Mon, Feb 25, 2019 at 06:18:42PM -0800, Rodney W. Grimes wrote:
> > > > > The modest increase in activation energy for that task seems worth
> it
> > > > > for the short-term gains of reduced integration cost (this code
> will
> > > > > greatly improve our ZFS-on-Linux test coverage.)
> > > > >
> > > > > Rod rightly points out that we haven't accepted SPDX tags alone as
> > > > > license statements.  The standard GPL v2.0 boiler plate should be
> added
> > > > > to this file along side the tag.
> > > >
> > > > I've copied the full copyright attribution that is in the
> > > > corresponding files on Linux. Is there some reason why FreeBSD
> > > > requires the files to be inflated with the full license text where
> the
> > > > original lacks it?
> > >
> > > I think for a few reasons, I doubt you copied the whole distribution
> > > that this file came from, as I am sure that distribution included
> > > a LICENSE file.  Second if you actually read the GPL v2 documentation
> > > and follow what it says it says you must do this, just because some
> > > one else does not follow the rules of what the GPL v2 says does not
> > > give us to knowingling not do it.  Third this is a particular
> dangerious
> > > area for BSD to be mixing a GPL code with its kernel, to my knowlege
> > > we have never had any gpl code in the kernel, no have we ever
> > > allowed it, but thats a seperate argument, that should be made.
> >
> > Would the arm64 DTS/DTB files count as "GPL code in the kernel?"
> >
> > I, too, would like less GPL in project, both in userland in kernel.
> > But, I can understand the desire for gcov. Note that I'm not
> > advocating either way that FreeBSD perform an action. ;)
>
> Didnt we just remove an inbase, compiling BSD licensed chunk of
> code called DRM and move it to ports.  So if that was possible
> this should be very rapidly applied here and this issue goes away.
>
> I am still shaking my head over this one.  Yes, there is some
> expediance to this.  Also could it not live on a project
> branch?  Like.. um.. the ZoL project branch?
>
> > Thanks,
> > Shawn Webb
>
> --
> Rod Grimes
> rgri...@freebsd.org
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r339618 - head/sys/compat/linuxkpi/common/include/linux

[Matthew Macy]

>
>
> > PowerPC barrier instructions are needed to prevent reordering.
> >
> > Correct. The current lkpi implementation also assumes that device
> > endian == host endian. The Linux generic accessors will do use endian
> > macros to byte swap where necessary.
>
> Yes, these functions are used to access little-endian registers so byte
> swapping is needed on big-endian machines.  For PowerPC Linux also defines
> functions to access big-endian registers, but we probably don't need those.
>

None of our currently supported drivers use those macros (most devices are
LE) so I’ve omitted them.

-M



> > The following change fixes radeon attach issues:
> >
> https://github.com/POWER9BSD/freebsd/commit/be6c98f5c2e2ed9a4935ac5b67c468b75f3b4457
>
> +/* prevent prefetching of coherent DMA data ahead of a dma-complete */
> +#ifndef __io_ar
> +#ifdef rmb
> +#define __io_ar()  rmb()
> +#else
> +#define __io_ar()  __compiler_membar();
> +#endif
> +#endif
> +
> +/* flush writes to coherent DMA data before possibly triggering a DMA
> read */
> +#ifndef __io_bw
> +#ifdef wmb
> +#define __io_bw()  wmb()
> +#else
> +#define __io_bw()  __compiler_membar();
> +#endif
> +#endif
>
> ...
>
>  static inline uint16_t
>  readw(const volatile void *addr)
>  {
> uint16_t v;
>
> -   __compiler_membar();
> -   v = *(const volatile uint16_t *)addr;
> -   __compiler_membar();
> +   __io_br();
> +   v = le16toh(__raw_readw(addr));
> +   __io_ar();
> return (v);
>  }
>
> For x86 rmb and wmb are defined as lfence and sfence instructions which
> shouldn't be necessary here.
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r339618 - head/sys/compat/linuxkpi/common/include/linux

[Matthew Macy]

Correct. This is just the generic case. We just need to define the __io
macros as __compiler_membar in x86/io.h

Cheers.
-M

On Sun, Nov 18, 2018 at 13:08 Tijl Coosemans  wrote:

> On Sun, 18 Nov 2018 12:10:25 -0800 Matthew Macy 
> wrote:
> >> Note that these functions are normally used on uncacheable memory which
> >> is strongly ordered on x86.  There should be no reordering at all.  On
> >> PowerPC barrier instructions are needed to prevent reordering.
> >
> > Correct. The current lkpi implementation also assumes that device
> > endian == host endian. The Linux generic accessors will do use endian
> > macros to byte swap where necessary.
>
> Yes, these functions are used to access little-endian registers so byte
> swapping is needed on big-endian machines.  For PowerPC Linux also defines
> functions to access big-endian registers, but we probably don't need those.
>
> > The following change fixes radeon attach issues:
> >
> https://github.com/POWER9BSD/freebsd/commit/be6c98f5c2e2ed9a4935ac5b67c468b75f3b4457
>
> +/* prevent prefetching of coherent DMA data ahead of a dma-complete */
> +#ifndef __io_ar
> +#ifdef rmb
> +#define __io_ar()  rmb()
> +#else
> +#define __io_ar()  __compiler_membar();
> +#endif
> +#endif
> +
> +/* flush writes to coherent DMA data before possibly triggering a DMA
> read */
> +#ifndef __io_bw
> +#ifdef wmb
> +#define __io_bw()  wmb()
> +#else
> +#define __io_bw()  __compiler_membar();
> +#endif
> +#endif
>
> ...
>
>  static inline uint16_t
>  readw(const volatile void *addr)
>  {
> uint16_t v;
>
> -   __compiler_membar();
> -   v = *(const volatile uint16_t *)addr;
> -   __compiler_membar();
> +   __io_br();
> +   v = le16toh(__raw_readw(addr));
> +   __io_ar();
> return (v);
>  }
>
> For x86 rmb and wmb are defined as lfence and sfence instructions which
> shouldn't be necessary here.
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r339618 - head/sys/compat/linuxkpi/common/include/linux

[Matthew Macy]

> Note that these functions are normally used on uncacheable memory which
> is strongly ordered on x86.  There should be no reordering at all.  On
> PowerPC barrier instructions are needed to prevent reordering.

Correct. The current lkpi implementation also assumes that device
endian == host endian. The Linux generic accessors will do use endian
macros to byte swap where necessary.

The following change fixes radeon attach issues:
https://github.com/POWER9BSD/freebsd/commit/be6c98f5c2e2ed9a4935ac5b67c468b75f3b4457
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r339618 - head/sys/compat/linuxkpi/common/include/linux

[Matthew Macy]

When looking at powerpc io.h raw and relaxed are not aliases, but it
appears that on x86, they are:
https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/io.h

Sorry for the noise. But let's starting moving the x86 specific
atomic.h and io.h under asm/x86.

Thanks.



On Sat, Nov 17, 2018 at 2:01 PM Matthew Macy  wrote:
>
> You should probably revert this. The implied understanding of the _relaxed 
> version is incorrect. compiler_membar is there to prevent instruction 
> reordering which is possible on FreeBSD because the accesses are done in C. 
> The relaxed variants still do not permit instruction reordering. On Linux 
> __compiler_member (referred to as barrier there) isn’t necessary in the mmio 
> accessors because they always use volatile asm which can’t be reordered. The 
> distinction between the relaxed and non relaxed variants is that the relaxed 
> variant lacks memory barriers (sync / lwsync / eieio on ppc, membar on sparc, 
> etc). Most of the time we don’t run in to problems on x86 because with TSO 
> the only reordering possible is writes that happen before reads can become 
> visible in memory after they occur in the instruction stream.
>
> Thanks.
> -M
>
> On Mon, Oct 22, 2018 at 13:55 Tijl Coosemans  wrote:
>>
>> Author: tijl
>> Date: Mon Oct 22 20:55:35 2018
>> New Revision: 339618
>> URL: https://svnweb.freebsd.org/changeset/base/339618
>>
>> Log:
>>   Define linuxkpi readq for 64-bit architectures.  It is used by drm-kmod.
>>   Currently the compiler picks up the definition in machine/cpufunc.h.
>>
>>   Add compiler memory barriers to read* and write*.  The Linux x86
>>   implementation of these functions uses inline asm with "memory" clobber.
>>   The Linux x86 implementation of read_relaxed* and write_relaxed* uses the
>>   same inline asm without "memory" clobber.
>>
>>   Implement ioread* and iowrite* in terms of read* and write* so they also
>>   have memory barriers.
>>
>>   Qualify the addr parameter in write* as volatile.
>>
>>   Like Linux, define macros with the same name as the inline functions.
>>
>>   Only define 64-bit versions on 64-bit architectures because generally
>>   32-bit architectures can't do atomic 64-bit loads and stores.
>>
>>   Regroup the functions a bit and add brief comments explaining what they do:
>>   - __raw_read*, __raw_write*: atomic, no barriers, no byte swapping
>>   - read_relaxed*, write_relaxed*: atomic, no barriers, little-endian
>>   - read*, write*: atomic, with barriers, little-endian
>>
>>   Add a comment that says our implementation of ioread* and iowrite*
>>   only handles MMIO and does not support port IO.
>>
>>   Reviewed by:  hselasky
>>   MFC after:3 days
>>
>> Modified:
>>   head/sys/compat/linuxkpi/common/include/linux/io.h
>>
>> Modified: head/sys/compat/linuxkpi/common/include/linux/io.h
>> ==
>> --- head/sys/compat/linuxkpi/common/include/linux/io.h  Mon Oct 22 20:22:33 
>> 2018(r339617)
>> +++ head/sys/compat/linuxkpi/common/include/linux/io.h  Mon Oct 22 20:55:35 
>> 2018(r339618)
>> @@ -38,153 +38,309 @@
>>  #include 
>>  #include 
>>
>> +/*
>> + * XXX This is all x86 specific.  It should be bus space access.
>> + */
>> +
>> +/* Access MMIO registers atomically without barriers and byte swapping. */
>> +
>> +static inline uint8_t
>> +__raw_readb(const volatile void *addr)
>> +{
>> +   return (*(const volatile uint8_t *)addr);
>> +}
>> +#define__raw_readb(addr)   __raw_readb(addr)
>> +
>> +static inline void
>> +__raw_writeb(uint8_t v, volatile void *addr)
>> +{
>> +   *(volatile uint8_t *)addr = v;
>> +}
>> +#define__raw_writeb(v, addr)   __raw_writeb(v, addr)
>> +
>> +static inline uint16_t
>> +__raw_readw(const volatile void *addr)
>> +{
>> +   return (*(const volatile uint16_t *)addr);
>> +}
>> +#define__raw_readw(addr)   __raw_readw(addr)
>> +
>> +static inline void
>> +__raw_writew(uint16_t v, volatile void *addr)
>> +{
>> +   *(volatile uint16_t *)addr = v;
>> +}
>> +#define__raw_writew(v, addr)   __raw_writew(v, addr)
>> +
>>  static inline uint32_t
>>  __raw_readl(const volatile void *addr)
>>  {
>> -   return *(const volatile uint32_t *)addr;
>> +   return (*(const volatile uint32_t *)addr);
>>  }
>> +#define__raw_readl(addr)

Re: svn commit: r339618 - head/sys/compat/linuxkpi/common/include/linux

[Matthew Macy]

You should probably revert this. The implied understanding of the _relaxed
version is incorrect. compiler_membar is there to prevent instruction
reordering which is possible on FreeBSD because the accesses are done in C.
The relaxed variants still do not permit instruction reordering. On Linux
__compiler_member (referred to as barrier there) isn’t necessary in the
mmio accessors because they always use volatile asm which can’t be
reordered. The distinction between the relaxed and non relaxed variants is
that the relaxed variant lacks memory barriers (sync / lwsync / eieio on
ppc, membar on sparc, etc). Most of the time we don’t run in to problems on
x86 because with TSO the only reordering possible is writes that happen
before reads can become visible in memory after they occur in the
instruction stream.

Thanks.
-M

On Mon, Oct 22, 2018 at 13:55 Tijl Coosemans  wrote:

> Author: tijl
> Date: Mon Oct 22 20:55:35 2018
> New Revision: 339618
> URL: https://svnweb.freebsd.org/changeset/base/339618
>
> Log:
>   Define linuxkpi readq for 64-bit architectures.  It is used by drm-kmod.
>   Currently the compiler picks up the definition in machine/cpufunc.h.
>
>   Add compiler memory barriers to read* and write*.  The Linux x86
>   implementation of these functions uses inline asm with "memory" clobber.
>   The Linux x86 implementation of read_relaxed* and write_relaxed* uses the
>   same inline asm without "memory" clobber.
>
>   Implement ioread* and iowrite* in terms of read* and write* so they also
>   have memory barriers.
>
>   Qualify the addr parameter in write* as volatile.
>
>   Like Linux, define macros with the same name as the inline functions.
>
>   Only define 64-bit versions on 64-bit architectures because generally
>   32-bit architectures can't do atomic 64-bit loads and stores.
>
>   Regroup the functions a bit and add brief comments explaining what they
> do:
>   - __raw_read*, __raw_write*: atomic, no barriers, no byte swapping
>   - read_relaxed*, write_relaxed*: atomic, no barriers, little-endian
>   - read*, write*: atomic, with barriers, little-endian
>
>   Add a comment that says our implementation of ioread* and iowrite*
>   only handles MMIO and does not support port IO.
>
>   Reviewed by:  hselasky
>   MFC after:3 days
>
> Modified:
>   head/sys/compat/linuxkpi/common/include/linux/io.h
>
> Modified: head/sys/compat/linuxkpi/common/include/linux/io.h
>
> ==
> --- head/sys/compat/linuxkpi/common/include/linux/io.h  Mon Oct 22
> 20:22:33 2018(r339617)
> +++ head/sys/compat/linuxkpi/common/include/linux/io.h  Mon Oct 22
> 20:55:35 2018(r339618)
> @@ -38,153 +38,309 @@
>  #include 
>  #include 
>
> +/*
> + * XXX This is all x86 specific.  It should be bus space access.
> + */
> +
> +/* Access MMIO registers atomically without barriers and byte swapping. */
> +
> +static inline uint8_t
> +__raw_readb(const volatile void *addr)
> +{
> +   return (*(const volatile uint8_t *)addr);
> +}
> +#define__raw_readb(addr)   __raw_readb(addr)
> +
> +static inline void
> +__raw_writeb(uint8_t v, volatile void *addr)
> +{
> +   *(volatile uint8_t *)addr = v;
> +}
> +#define__raw_writeb(v, addr)   __raw_writeb(v, addr)
> +
> +static inline uint16_t
> +__raw_readw(const volatile void *addr)
> +{
> +   return (*(const volatile uint16_t *)addr);
> +}
> +#define__raw_readw(addr)   __raw_readw(addr)
> +
> +static inline void
> +__raw_writew(uint16_t v, volatile void *addr)
> +{
> +   *(volatile uint16_t *)addr = v;
> +}
> +#define__raw_writew(v, addr)   __raw_writew(v, addr)
> +
>  static inline uint32_t
>  __raw_readl(const volatile void *addr)
>  {
> -   return *(const volatile uint32_t *)addr;
> +   return (*(const volatile uint32_t *)addr);
>  }
> +#define__raw_readl(addr)   __raw_readl(addr)
>
>  static inline void
> -__raw_writel(uint32_t b, volatile void *addr)
> +__raw_writel(uint32_t v, volatile void *addr)
>  {
> -   *(volatile uint32_t *)addr = b;
> +   *(volatile uint32_t *)addr = v;
>  }
> +#define__raw_writel(v, addr)   __raw_writel(v, addr)
>
> +#ifdef __LP64__
>  static inline uint64_t
>  __raw_readq(const volatile void *addr)
>  {
> -   return *(const volatile uint64_t *)addr;
> +   return (*(const volatile uint64_t *)addr);
>  }
> +#define__raw_readq(addr)   __raw_readq(addr)
>
>  static inline void
> -__raw_writeq(uint64_t b, volatile void *addr)
> +__raw_writeq(uint64_t v, volatile void *addr)
>  {
> -   *(volatile uint64_t *)addr = b;
> +   *(volatile uint64_t *)addr = v;
>  }
> +#define__raw_writeq(v, addr)   __raw_writeq(v, addr)
> +#endif
>
> -/*
> - * XXX This is all x86 specific.  It should be bus space access.
> - */
>  #definemmiowb()barrier()
>
> -#undef writel
> +/* Access little-endian MMIO registers atomically with memory barriers. */
> +
> 

Re: svn commit: r338172 - Now deprecating DRM

[Matthew Macy]

Rodney - we appreciate your emotional investment in the project. And as
convenient as it would be for me to take all the blame the imminent removal
of drm2 was communicated on public lists. Your soul contribution to the
discussion was to bemoan the fact that i386 would no longer have special
advantages over people using newer hardware. During this discussion you,
RE, or core could have chimed in with guidelines for a smooth deprecation
process. None of them did. To chime in now as if you were wounded by a
rogue actor is extremely disingenuous.  And as Kevin keeps point you keep
pointing at strawmen. This isn't how we'll grow the user base. (Assuming
that that's something you value as well).

And please don't forget that process exists to serve users not for its own
sake.

Thanks for all of your efforts on our behalf.

-M


On Thu, Aug 23, 2018 at 10:55 AM Rodney W. Grimes <
free...@pdx.rh.cn85.dnsmgr.net> wrote:

> [ Charset UTF-8 unsupported, converting... ]
> > On Wed, Aug 22, 2018 at 10:08 AM Rodney W. Grimes <
> > free...@pdx.rh.cn85.dnsmgr.net> wrote:
> >
> > > I think this deprecation is a rather serious deviation
> > > from the stated policy, in that 0 notification was
> > > made, core IMHO, has overstepped some boundaries in
> > > that respect.   These policies are promises to the
> > > downstream consumers, and violating them is very
> > > poor planning.
> > >
> >
> > Despite what the commit said, core didn't actually formally approve it
> > before the fact. That was one of the many miscommunications surrounding
> > this episode.
>
> Well I would say inlight of that fact a revert should be a no questions
> asked, doing much else risks build breakage within hours of code freeze.
>
>
> > I don't think you'll find anybody who would say this was well planned or
> > well executed.
>
> Agreed, so lets be simple in correcting it?
>
> From some investigation even the claim that "in base drm conflicts with
> ports drm" is not totally true, you just have to take a few steps to
> be sure you do not load the base versions, and do load the ports version.
>
> There is still time to revert this, add the gone_in(13) glue and have
> 12.0 go out in that state.
>
> I am concerned that if 1 person came forward right off the bat with
> this change, and we try to do something that takes care of just that
> issue we are going to have others come forward in time with similiar
> issues and we are going to look bad for failing to follow our own
> published guidelines.
>
> I see no smoking gun reason that this code has to die today,
> the ports are already setup with instructions on how to deal
> with the inbase drm.
>
> --
> Rod Grimes
> rgri...@freebsd.org
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r338172 - in head: . sys/amd64/conf sys/dev/drm sys/dev/drm2 sys/dev/drm2/i915 sys/dev/drm2/radeon sys/dev/drm2/radeon/reg_srcs sys/dev/drm2/ttm sys/i386/conf sys/modules sys/modules/d

[Matthew Macy]

Johannes - do you know off hand?
-M

On Wed, Aug 22, 2018 at 12:06 AM Andriy Gapon  wrote:

> On 22/08/2018 04:50, Matt Macy wrote:
> > Author: mmacy
> > Date: Wed Aug 22 01:50:12 2018
> > New Revision: 338172
> > URL: https://svnweb.freebsd.org/changeset/base/338172
> >
> > Log:
> >   Remove legacy drm and drm2 from tree
> >
> >   As discussed on the MLs drm2 conflicts with the ports' version and
> there
> >   is no upstream for most if not all of drm. Both have been merged in to
> >   a single port.
> ...
> >   Approved by: core@
>
> I am curious if this "panic: implment me" has been addressed?
> https://lists.freebsd.org/pipermail/freebsd-current/2018-May/069428.html
>
>
> --
> Andriy Gapon
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r338172 - in head: . sys/amd64/conf sys/dev/drm sys/dev/drm2 sys/dev/drm2/i915 sys/dev/drm2/radeon sys/dev/drm2/radeon/reg_srcs sys/dev/drm2/ttm sys/i386/conf sys/modules sys/modules/d

[Matthew Macy]

>
> Could you please create a stable/11 deprecation change.
>

What does that entail other than an update to UPDATING in stable/11?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

[Matthew Macy]

On Tue, Aug 21, 2018 at 16:52 Mark Johnston  wrote:

> On Tue, Aug 21, 2018 at 04:00:10PM -0700, Matthew Macy wrote:
> > Yes. See r338162. Thanks.
>
> You missed instances of the same bug in in_mcast.c and in6_mcast.c.



Thanks

>
>
> > On Tue, Aug 21, 2018 at 2:24 PM Gleb Smirnoff 
> wrote:
> > > On Wed, Aug 15, 2018 at 08:23:09PM +, Matt Macy wrote:
> > > M> @@ -3772,8 +3775,11 @@ if_delmulti_locked(struct ifnet *ifp, struct
> > > ifmultiad
> > > M>  ll_ifma->ifma_ifp = NULL;   /* XXX */
> > > M>  if (--ll_ifma->ifma_refcount == 0) {
> > > M>  if (ifp != NULL) {
> > > M> -CK_STAILQ_REMOVE(>if_multiaddrs,
> > > ll_ifma, ifmultiaddr,
> > > M> -ifma_link);
> > > M> +if (ll_ifma->ifma_flags &
> IFMA_F_ENQUEUED)
> > > {
> > > M> +
> > > CK_STAILQ_REMOVE(>if_multiaddrs, ll_ifma, ifmultiaddr,
> > > M> +ifma_link);
> > > M> +ifma->ifma_flags &=
> > > ~IFMA_F_ENQUEUED;
> > > M> +}
> > > M>  }
> > > M>  if_freemulti(ll_ifma);
> > > M>  }
> > >
> > > Coverity suggested there is a cut and paste mistake here, and it is
> > > compilable.
> > > After quick glance I tend to agree. Looks like flag is cleared on wrong
> > > ifma.
> > >
> > > --
> > > Gleb Smirnoff
> > >
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

[Matthew Macy]

Yes. See r338162. Thanks.
-M

On Tue, Aug 21, 2018 at 2:24 PM Gleb Smirnoff  wrote:

> On Wed, Aug 15, 2018 at 08:23:09PM +, Matt Macy wrote:
> M> @@ -3772,8 +3775,11 @@ if_delmulti_locked(struct ifnet *ifp, struct
> ifmultiad
> M>  ll_ifma->ifma_ifp = NULL;   /* XXX */
> M>  if (--ll_ifma->ifma_refcount == 0) {
> M>  if (ifp != NULL) {
> M> -CK_STAILQ_REMOVE(>if_multiaddrs,
> ll_ifma, ifmultiaddr,
> M> -ifma_link);
> M> +if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED)
> {
> M> +
> CK_STAILQ_REMOVE(>if_multiaddrs, ll_ifma, ifmultiaddr,
> M> +ifma_link);
> M> +ifma->ifma_flags &=
> ~IFMA_F_ENQUEUED;
> M> +}
> M>  }
> M>  if_freemulti(ll_ifma);
> M>  }
>
> Coverity suggested there is a cut and paste mistake here, and it is
> compilable.
> After quick glance I tend to agree. Looks like flag is cleared on wrong
> ifma.
>
> --
> Gleb Smirnoff
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

[Matthew Macy]

Sorry. I'll take a look.

On Fri, Aug 17, 2018 at 05:30 Andrey V. Elsukov  wrote:

> On 15.08.2018 23:23, Matt Macy wrote:
> > Author: mmacy
> > Date: Wed Aug 15 20:23:08 2018
> > New Revision: 337866
> > URL: https://svnweb.freebsd.org/changeset/base/337866
> >
> > Log:
> >   Fix in6_multi double free
> >
> >   This is actually several different bugs:
> >   - The code is not designed to handle inpcb deletion after interface
> deletion
> > - add reference for inpcb membership
> >   - The multicast address has to be removed from interface lists when
> the refcount
> > goes to zero OR when the interface goes away
> > - decouple list disconnect from refcount (v6 only for now)
> >   - ifmultiaddr can exist past being on interface lists
> > - add flag for tracking whether or not it's enqueued
> >   - deferring freeing moptions makes the incpb cleanup code simpler but
> opens the
> > door wider still to races
> > - call inp_gcmoptions synchronously after dropping the the inpcb lock
> >
> >   Fundamentally multicast needs a rewrite - but keep applying band-aids
> for now.
> >
> >   Tested by: kp
> >   Reported by: novel, kp, lwhsu
> >
> > Modified:
> >   head/sys/net/if.c
> >   head/sys/net/if_var.h
> >   head/sys/netinet/in_mcast.c
> >   head/sys/netinet/in_pcb.c
> >   head/sys/netinet/ip_carp.c
> >   head/sys/netinet6/in6_ifattach.c
> >   head/sys/netinet6/in6_mcast.c
> >   head/sys/netinet6/in6_var.h
> >   head/sys/netinet6/mld6.c
>
> Hi,
>
> After this commit my test machine panics just after boot finishes.
> Reverting this commit helps.
> Machine has two interfaces in failover lagg. One interface is not
> connected.
>
> FreeBSD 12.0-ALPHA2 (GENERIC) #2 r337961M: Fri Aug 17 14:54:48 MSK 2018
>
> # ifconfig
> em0: flags=8843 metric 0 mtu 1500
>
> options=209b
> ether 00:22:4d:6a:5e:b9
> media: Ethernet autoselect
> status: no carrier
> nd6 options=21
> re0: flags=8843 metric 0 mtu 1500
>
> options=8209b
> ether 00:22:4d:6a:5e:b9
> hwaddr 1c:bd:b9:de:0d:7d
> media: Ethernet autoselect (1000baseT )
> status: active
> nd6 options=21
> lagg0: flags=8843 metric 0 mtu 1500
>
> options=209b
> ether 00:22:4d:6a:5e:b9
> inet6 fe80::222:4dff:fe6a:5eb9%lagg0 prefixlen 64 scopeid 0x5
> inet 10.9.8.6 netmask 0xff00 broadcast 10.9.8.255
> laggproto failover lagghash l2,l3,l4
> laggport: em0 flags=1
> laggport: re0 flags=4
> groups: lagg
> media: Ethernet autoselect
> status: active
> nd6 options=21
>
> --
> WBR, Andrey V. Elsukov
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337730 - in head/lib/libpmc/pmu-events/arch/x86: . amdfam17h

[Matthew Macy]

The entries in the .json files were replicated and mapfile.csv lacked a
newline at the end of the file causing parse failures.
-M

On Mon, Aug 13, 2018 at 4:56 PM Ravi Pokala  wrote:

> Hi Matt,
>
> -Original Message-
> From:  on behalf of Matt Macy
> 
> Date: 2018-08-13, Monday at 16:46
> To: , , <
> svn-src-head@freebsd.org>
> Subject: svn commit: r337730 - in head/lib/libpmc/pmu-events/arch/x86: .
> amdfam17h
>
> > Author: mmacy
> > Date: Mon Aug 13 23:46:44 2018
> > New Revision: 337730
> > URL: https://svnweb.freebsd.org/changeset/base/337730
> >
> > Log:
> >   pmc amd17h: fix inputs to jevents
> >
> > Modified:
> >   head/lib/libpmc/pmu-events/arch/x86/amdfam17h/cache.json
> >   head/lib/libpmc/pmu-events/arch/x86/amdfam17h/core.json
> >   head/lib/libpmc/pmu-events/arch/x86/amdfam17h/floating-point.json
> >   head/lib/libpmc/pmu-events/arch/x86/amdfam17h/memory.json
> >   head/lib/libpmc/pmu-events/arch/x86/amdfam17h/other.json
> >   head/lib/libpmc/pmu-events/arch/x86/mapfile.csv
>
> What was the nature of the problem, and how was it fixed?
>
> Thanks,
>
> Ravi (rpokala@)
>
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r336593 - head/release

[Matthew Macy]

That would only fix it if svn weren't installed.

On Sun, Jul 22, 2018 at 9:31 AM, Colin Percival  wrote:
> On 07/22/18 08:04, Rodney W. Grimes wrote:
>>> Build still works, but you're assuming that developers only use svn.
>>
>> No, he correctly assumed that RELEASE engineering only uses svn/svnlite
>> for building release images.  This Makefile is for production of
>> official AMIs, official stuff always uses svn.
>
> I'm about to head mostly-AFK for the week, but one option here would be
> to change
>
> EC2_SVNBRANCH!=${SVN_CMD} info --show-item relative-url ${WORLDDIR} |
> sed -e 's/\^\///'
> EC2_SVNREV!=   ${SVN_CMD} info --show-item last-changed-revision ${WORLDDIR}
>
> to
>
> .if defined(SVN_CMD)
> EC2_SVNBRANCH!=${SVN_CMD} info --show-item relative-url ${WORLDDIR} |
> sed -e 's/\^\///'
> EC2_SVNREV!=   ${SVN_CMD} info --show-item last-changed-revision ${WORLDDIR}
> .else
> EC2_SVNBRANCH=  unknown
> EC2_SVNREV= unknown
> .endif
>
> I don't know if this will make any difference to anyone using this code,
> but it should at least silence the warnings for people who aren't checking
> code out from svn.
>
> Colin Percival
>
>>> make[1]: "/usr/home/mmacy/devel/freebsd/Makefile.inc1" line 343:
>>> SYSTEM_COMPILER: libclang will be built for bootstrapping a
>>> cross-compiler.
>>> make[1]: "/usr/home/mmacy/devel/freebsd/Makefile.inc1" line 348:
>>> SYSTEM_LINKER: libclang will be built for bootstrapping a
>>> cross-linker.
>>> svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
>>> svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
>>> make[2]: "/usr/home/mmacy/devel/freebsd/release/Makefile.ec2" line 19:
>>> warning: "/usr/local/bin/svn info --show-item last-changed-revision
>>> /usr/home/mmacy/devel/freebsd/release/.." returned non-zero status
>>> svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
>>> svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
>>> make[2]: "/usr/home/mmacy/devel/freebsd/release/Makefile.ec2" line 19:
>>> warning: "/usr/local/bin/svn info --show-item last-changed-revision
>>> /usr/home/mmacy/devel/freebsd/release/.." returned non-zero status
>>>
>>> On Sat, Jul 21, 2018 at 3:54 PM, Colin Percival  
>>> wrote:
 Author: cperciva
 Date: Sat Jul 21 22:54:43 2018
 New Revision: 336593
 URL: https://svnweb.freebsd.org/changeset/base/336593

 Log:
   Use svn or svnlite, or ${SVN_CMD} if set, for extracting the SVN branch
   and revision number announced in SNS notifications about new EC2 AMIs.

   While I'm here, incorporate that information into the AMI "description"
   fields, since it's more useful than simply echoing the information
   already provided via the AMI "name".

   Approved by:  gjb

 Modified:
   head/release/Makefile.ec2
>>   ^^
>>

 Modified: head/release/Makefile.ec2
 ==
 --- head/release/Makefile.ec2   Sat Jul 21 22:15:48 2018(r336592)
 +++ head/release/Makefile.ec2   Sat Jul 21 22:54:43 2018(r336593)
 @@ -5,6 +5,19 @@
  # Makefile for creating an EC2 AMI from a disk image.
  #

 +# Figure out where SVN is
 +.if !defined(SVN_CMD) || empty(SVN_CMD)
 +. for _P in /usr/bin /usr/local/bin
 +.  for _S in svn svnlite
 +.   if exists(${_P}/${_S})
 +SVN_CMD=   ${_P}/${_S}
 +.   endif
 +.  endfor
 +. endfor
 +.endif
 +EC2_SVNBRANCH!=${SVN_CMD} info --show-item relative-url 
 ${WORLDDIR} | sed -e 's/\^\///'
 +EC2_SVNREV!=   ${SVN_CMD} info --show-item last-changed-revision 
 ${WORLDDIR}
 +
  .if ${BRANCH} == "CURRENT" || ${BRANCH} == "STABLE" || ${BRANCH} == 
 "PRERELEASE"
  AMINAMESUFFIX!=date +-%Y-%m-%d
  .endif
 @@ -13,8 +26,6 @@ PUBLISH=  --public
  .endif
  .if defined(EC2SNSTOPIC) && !empty(EC2SNSTOPIC)
  EC2SNSREL= ${REVISION}-${BRANCH}
 -EC2_SVNBRANCH!=svn info --show-item relative-url ${WORLDDIR} | 
 sed -e 's/\^\///'
 -EC2_SVNREV!=   svn info --show-item last-changed-revision ${WORLDDIR}
  EC2SNSVERS=${EC2_SVNBRANCH}@${EC2_SVNREV}
  .endif

 @@ -60,7 +71,7 @@ ec2ami: cw-ec2 ${CW_EC2_PORTINSTALL}
 /usr/local/bin/bsdec2-image-upload ${PUBLISH} --sriov --ena \
 ${.OBJDIR}/ec2.raw \
 "${TYPE} ${REVISION}-${BRANCH}-${TARGET}${AMINAMESUFFIX}" \
 -   "${TYPE} ${REVISION}-${BRANCH}-${TARGET}" \
 +   "${TYPE}/${TARGET} ${EC2_SVNBRANCH}@${EC2_SVNREV}" \
 ${AWSREGION} ${AWSBUCKET} ${AWSKEYFILE} \
 ${EC2SNSTOPIC} ${EC2SNSREL} ${EC2SNSVERS}
 @touch ${.TARGET}

>>>
>>>
>>
>
> --
> Colin Percival
> Security Officer Emeritus, FreeBSD | The power to serve
> Founder, Tarsnap | www.tarsnap.com | Online backups for the truly 

Re: svn commit: r336593 - head/release

[Matthew Macy]

Build still works, but you're assuming that developers only use svn.

make[1]: "/usr/home/mmacy/devel/freebsd/Makefile.inc1" line 343:
SYSTEM_COMPILER: libclang will be built for bootstrapping a
cross-compiler.
make[1]: "/usr/home/mmacy/devel/freebsd/Makefile.inc1" line 348:
SYSTEM_LINKER: libclang will be built for bootstrapping a
cross-linker.
svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
make[2]: "/usr/home/mmacy/devel/freebsd/release/Makefile.ec2" line 19:
warning: "/usr/local/bin/svn info --show-item last-changed-revision
/usr/home/mmacy/devel/freebsd/release/.." returned non-zero status
svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
svn: E155007: '/usr/home/mmacy/devel/freebsd' is not a working copy
make[2]: "/usr/home/mmacy/devel/freebsd/release/Makefile.ec2" line 19:
warning: "/usr/local/bin/svn info --show-item last-changed-revision
/usr/home/mmacy/devel/freebsd/release/.." returned non-zero status

On Sat, Jul 21, 2018 at 3:54 PM, Colin Percival  wrote:
> Author: cperciva
> Date: Sat Jul 21 22:54:43 2018
> New Revision: 336593
> URL: https://svnweb.freebsd.org/changeset/base/336593
>
> Log:
>   Use svn or svnlite, or ${SVN_CMD} if set, for extracting the SVN branch
>   and revision number announced in SNS notifications about new EC2 AMIs.
>
>   While I'm here, incorporate that information into the AMI "description"
>   fields, since it's more useful than simply echoing the information
>   already provided via the AMI "name".
>
>   Approved by:  gjb
>
> Modified:
>   head/release/Makefile.ec2
>
> Modified: head/release/Makefile.ec2
> ==
> --- head/release/Makefile.ec2   Sat Jul 21 22:15:48 2018(r336592)
> +++ head/release/Makefile.ec2   Sat Jul 21 22:54:43 2018(r336593)
> @@ -5,6 +5,19 @@
>  # Makefile for creating an EC2 AMI from a disk image.
>  #
>
> +# Figure out where SVN is
> +.if !defined(SVN_CMD) || empty(SVN_CMD)
> +. for _P in /usr/bin /usr/local/bin
> +.  for _S in svn svnlite
> +.   if exists(${_P}/${_S})
> +SVN_CMD=   ${_P}/${_S}
> +.   endif
> +.  endfor
> +. endfor
> +.endif
> +EC2_SVNBRANCH!=${SVN_CMD} info --show-item relative-url ${WORLDDIR} 
> | sed -e 's/\^\///'
> +EC2_SVNREV!=   ${SVN_CMD} info --show-item last-changed-revision ${WORLDDIR}
> +
>  .if ${BRANCH} == "CURRENT" || ${BRANCH} == "STABLE" || ${BRANCH} == 
> "PRERELEASE"
>  AMINAMESUFFIX!=date +-%Y-%m-%d
>  .endif
> @@ -13,8 +26,6 @@ PUBLISH=  --public
>  .endif
>  .if defined(EC2SNSTOPIC) && !empty(EC2SNSTOPIC)
>  EC2SNSREL= ${REVISION}-${BRANCH}
> -EC2_SVNBRANCH!=svn info --show-item relative-url ${WORLDDIR} | sed 
> -e 's/\^\///'
> -EC2_SVNREV!=   svn info --show-item last-changed-revision ${WORLDDIR}
>  EC2SNSVERS=${EC2_SVNBRANCH}@${EC2_SVNREV}
>  .endif
>
> @@ -60,7 +71,7 @@ ec2ami: cw-ec2 ${CW_EC2_PORTINSTALL}
> /usr/local/bin/bsdec2-image-upload ${PUBLISH} --sriov --ena \
> ${.OBJDIR}/ec2.raw \
> "${TYPE} ${REVISION}-${BRANCH}-${TARGET}${AMINAMESUFFIX}" \
> -   "${TYPE} ${REVISION}-${BRANCH}-${TARGET}" \
> +   "${TYPE}/${TARGET} ${EC2_SVNBRANCH}@${EC2_SVNREV}" \
> ${AWSREGION} ${AWSBUCKET} ${AWSKEYFILE} \
> ${EC2SNSTOPIC} ${EC2SNSREL} ${EC2SNSVERS}
> @touch ${.TARGET}
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335879 - in head/sys: conf kern sys

[Matthew Macy]

On Sun, Jul 8, 2018 at 7:22 AM, Mark Johnston  wrote:
> On Tue, Jul 03, 2018 at 01:55:10AM +, Matt Macy wrote:
>> Author: mmacy
>> Date: Tue Jul  3 01:55:09 2018
>> New Revision: 335879
>> URL: https://svnweb.freebsd.org/changeset/base/335879
>>
>> Log:
>>   make critical_{enter, exit} inline
>>
>>   Avoid pulling in all of the  dependencies by
>>   automatically generating a stripped down thread_lite exporting
>>   only the fields of interest. The field declarations are type checked
>>   against the original and the offsets of the generated result is
>>   automatically checked.
>>
>>   kib has expressed disagreement and would have preferred to simply
>>   use genassym style offsets (which loses type check enforcement).
>>   jhb has expressed dislike of it due to header pollution and a
>>   duplicate structure. He would have preferred to just have defined
>>   thread in _thread.h. Nonetheless, he admits that this is the only
>>   viable solution at the moment.
>>
>>   The impetus for this came from mjg's D15331:
>>   "Inline critical_enter/exit for amd64"
>>
>>   Reviewed by: jeff
>>   Differential Revision: https://reviews.freebsd.org/D16078
>>
>> [...]
>> +#if defined(KLD_MODULE) || defined(KTR_CRITICAL) || !defined(_KERNEL) || 
>> defined(GENOFFSET)
>> +#define critical_enter() critical_enter_KBI()
>> +#define critical_exit() critical_exit_KBI()
>> +#else
>> +static __inline void
>> +critical_enter(void)
>> +{
>> + struct thread_lite *td;
>> +
>> + td = (struct thread_lite *)curthread;
>> + td->td_critnest++;
>
> Don't we need a compiler barrier here?
>

We definitely do. Not sure how that got lost :( Will fix.
-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335979 - in head: . lib/libkvm sys/kern sys/netinet sys/sys usr.bin/netstat usr.bin/sockstat

[Matthew Macy]

this breaks the MIPS builds.

On Thu, Jul 5, 2018 at 9:33 AM, Brooks Davis  wrote:
> On Thu, Jul 05, 2018 at 09:10:54AM -0700, Ravi Pokala wrote:
>> Hi Brooks,
>>
>> -Original Message-
>> From:  on behalf of Brooks Davis 
>> 
>> Date: 2018-07-05, Thursday at 06:13
>> To: , , 
>> 
>> Subject: svn commit: r335979 - in head: . lib/libkvm sys/kern sys/netinet 
>> sys/sys usr.bin/netstat usr.bin/sockstat
>>
>> > Author: brooks
>> > Date: Thu Jul  5 13:13:48 2018
>> > New Revision: 335979
>> > URL: https://svnweb.freebsd.org/changeset/base/335979
>> >
>> > Log:
>> >   Make struct xinpcb and friends word-size independent.
>> >
>> >   Replace size_t members with ksize_t (uint64_t) and pointer members
>> >   (never used as pointers in userspace, but instead as unique
>> >   idenitifiers) with kvaddr_t (uint64_t). This makes the structs
>> >   identical between 32-bit and 64-bit ABIs.
>> ...
>> > Modified: head/UPDATING
>> > ==
>> > --- head/UPDATING   Thu Jul  5 11:50:59 2018(r335978)
>> > +++ head/UPDATING   Thu Jul  5 13:13:48 2018(r335979)
>> > @@ -31,6 +31,14 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 12.x IS SLOW:
>> > disable the most expensive debugging functionality run
>> > "ln -s 'abort:false,junk:false' /etc/malloc.conf".)
>> >
>> > +20180705:
>> > +   The ABI of syscalls used by management tools like sockstat and
>> > +   netstat has been broken to allow 32-bit binaries to work on
>> > +   64-bit kernels without modification.
>>
>> Isn't that what the compat32 layer is for?
>
> compat32 isn't magic.  If one tried, one could hardly design structures
> to make 32-bit compat harder then the previous versions.  It's certainly
> possible to make work, but quite annoying.  Since the ABI of most these
> structures was already broken for 12, I chose this approach as it is
> quite trivial.
>
>> > These programs will need
>> > +   to match the kernel in order to function.  External programs may
>> > +   require minor modifications to accommodate a change of type in
>> > +   structures from pointers to 64-bit virtual addresses.
>>
>> Doesn't this contradict the earlier statement about letting things run 
>> unmodified?
>
> Unmodified post this commit.  We already don't support netstat and
> sockstat from 11 on 12 in any architecture combination.
>
> -- Brooks
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335916 - head/sys/conf

[Matthew Macy]

On Thu, Jul 5, 2018 at 8:54 AM, Konstantin Belousov  wrote:
> On Thu, Jul 05, 2018 at 07:56:22AM -0700, John Baldwin wrote:
>> On 7/4/18 7:22 AM, Konstantin Belousov wrote:
>> > On Tue, Jul 03, 2018 at 11:05:42PM +, Matt Macy wrote:
>> >> Author: mmacy
>> >> Date: Tue Jul  3 23:05:42 2018
>> >> New Revision: 335916
>> >> URL: https://svnweb.freebsd.org/changeset/base/335916
>> >>
>> >> Log:
>> >>   Enable MODULE_TIED by default for modules compiled with the kernel
>> > But why ?
>>
>> I think we should enable KLD_TIED to inline critical_* etc. for modules
>> built as part of a kernel that are installed alongside the kernel in 
>> /boot/.
>
>> I don't think we need to support modules built with kernel A loaded into 
>> kernel B.
>>
> This is the crusial point.  I do not object, but this this is a radical
> change from the previous mode of modules build.
>
> I do not want to put words in other person mouth, but I beliee that the
> original intent of KLD_TIED/MODULE_TIED was much more limited.  Only some
> specific modules were to be tied.


My intention was only to allow it for select modules compiled _with_
the kernel. Then there was a heated discussion about documentation and
how to communicate it's distinction with DECLARE_MODULE_TIED. In
response to which John said (assuming I understood correctly,
communication is hard) that it should be the default and specified
doing it the way I did it. That seemed reasonable - but I don't have
strong feelings one way or the other. I apologize if I misunderstood
any guidance I was provided.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335916 - head/sys/conf

[Matthew Macy]

I didn't want it to be global. Note that I did _exactly_ what John told me
to. I don't believe the tone or word choice is justified.

-M

On Tue, Jul 3, 2018 at 17:32 Warner Losh  wrote:

> And this is why I wanted it to conform to the current build paradigm in my
> review... I'll mop it up once Bryan is done...
>
> Warner
>
> On Tue, Jul 3, 2018, 6:05 PM Matt Macy  wrote:
>
>> Author: mmacy
>> Date: Tue Jul  3 23:05:42 2018
>> New Revision: 335916
>> URL: https://svnweb.freebsd.org/changeset/base/335916
>>
>> Log:
>>   Enable MODULE_TIED by default for modules compiled with the kernel
>>
>>   Requested by: jhb
>>
>> Modified:
>>   head/sys/conf/kern.post.mk
>>
>> Modified: head/sys/conf/kern.post.mk
>>
>> ==
>> --- head/sys/conf/kern.post.mk  Tue Jul  3 22:54:46 2018(r335915)
>> +++ head/sys/conf/kern.post.mk  Tue Jul  3 23:05:42 2018(r335916)
>> @@ -14,6 +14,7 @@ MKMODULESENV+=DESTDIR="${DESTDIR}"
>>  .endif
>>  SYSDIR?= ${S:C;^[^/];${.CURDIR}/&;:tA}
>>  MKMODULESENV+= KERNBUILDDIR="${.CURDIR}" SYSDIR="${SYSDIR}"
>> +MKMODULESENV+=  MODULE_TIED=yes
>>
>>  .if defined(CONF_CFLAGS)
>>  MKMODULESENV+= CONF_CFLAGS="${CONF_CFLAGS}"
>>
>>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335880 - in head/sys: conf kern

[Matthew Macy]

What is the "correct" way to make sure that offset.inc is visible to modules?

On Tue, Jul 3, 2018 at 10:39 AM, Bryan Drewery  wrote:
> On 7/2/2018 7:50 PM, Matt Macy wrote:
>> Author: mmacy
>> Date: Tue Jul  3 02:50:07 2018
>> New Revision: 335880
>> URL: https://svnweb.freebsd.org/changeset/base/335880
>>
>> Log:
>>   expose thread_lite definition to tied modules
>>
>> Modified:
>>   head/sys/conf/kmod.mk
>>   head/sys/kern/genoffset.sh
>>
>> Modified: head/sys/conf/kmod.mk
>> ==
>> --- head/sys/conf/kmod.mk Tue Jul  3 01:55:09 2018(r335879)
>> +++ head/sys/conf/kmod.mk Tue Jul  3 02:50:07 2018(r335880)
>> @@ -463,9 +463,10 @@ acpi_quirks.h: ${SYSDIR}/tools/acpi_quirks2h.awk ${SYS
>>  .endif
>>
>>  .if !empty(SRCS:Massym.inc) || !empty(DPSRCS:Massym.inc)
>> -CLEANFILES+= assym.inc genassym.o
>> -DEPENDOBJS+= genassym.o
>> +CLEANFILES+= assym.inc genassym.o genoffset.o
>> +DEPENDOBJS+= genassym.o genoffset.o
>>  assym.inc: genassym.o
>> +offset.inc: genoffset.o
>>  .if defined(KERNBUILDDIR)
>>  genassym.o: opt_global.h
>>  .endif
>> @@ -475,6 +476,12 @@ genassym.o: ${SYSDIR}/${MACHINE}/${MACHINE}/genassym.c
>>  genassym.o: ${SRCS:Mopt_*.h}
>>   ${CC} -c ${CFLAGS:N-flto:N-fno-common} \
>>   ${SYSDIR}/${MACHINE}/${MACHINE}/genassym.c
>> +offset.inc: ${SYSDIR}/kern/genoffset.sh genoffset.o
>> + sh ${SYSDIR}/kern/genoffset.sh genoffset.o > ${.TARGET}
>> +genoffset.o: ${SYSDIR}/${MACHINE}/${MACHINE}/genoffset.c
>> +genoffset.o: ${SRCS:Mopt_*.h}
>> + ${CC} -c ${CFLAGS:N-flto:N-fno-common} \
>> + ${SYSDIR}/${MACHINE}/${MACHINE}/genoffset.c
>
> Why is this using machine-specific genoffset.c but kern.post.mk uses the
> global genoffset.c?
>
> And what even generates the machine-specific genoffset.c file? I see no
> rule for it anywhere.
>
>>  .endif
>>
>>  .if defined(KERNBUILDDIR)
>>
>> Modified: head/sys/kern/genoffset.sh
>> ==
>> --- head/sys/kern/genoffset.shTue Jul  3 01:55:09 2018
>> (r335879)
>> +++ head/sys/kern/genoffset.shTue Jul  3 02:50:07 2018
>> (r335880)
>> @@ -39,7 +39,7 @@ work()
>>  {
>>   echo "#ifndef _OFFSET_INC_"
>>   echo "#define _OFFSET_INC_"
>> - echo "#if !defined(GENOFFSET) && !defined(KLD_MODULE)"
>> + echo "#if !defined(GENOFFSET) && (!defined(KLD_MODULE) || 
>> defined(KLD_TIED))"
>>   ${NM:='nm'} ${NMFLAGS} "$1" | ${AWK:='awk'} '
>>   / C .*_datatype_*/ {
>>   type = substr($3, match($3, "_datatype_") + 
>> length("_datatype_"))
>>
>
>
> --
> Regards,
> Bryan Drewery
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335879 - in head/sys: conf kern sys

[Matthew Macy]

genoffset_test tests that the offsets match up

On Tue, Jul 3, 2018 at 11:02 AM, Bryan Drewery  wrote:
> On 7/2/2018 6:55 PM, Matt Macy wrote:
>> Author: mmacy
>> Date: Tue Jul  3 01:55:09 2018
>> New Revision: 335879
>> URL: https://svnweb.freebsd.org/changeset/base/335879
>>
>> Log:
>>   make critical_{enter, exit} inline
>>
>>   Avoid pulling in all of the  dependencies by
>>   automatically generating a stripped down thread_lite exporting
>>   only the fields of interest. The field declarations are type checked
>>   against the original and the offsets of the generated result is
>>   automatically checked.
>>
>>   kib has expressed disagreement and would have preferred to simply
>>   use genassym style offsets (which loses type check enforcement).
>>   jhb has expressed dislike of it due to header pollution and a
>>   duplicate structure. He would have preferred to just have defined
>>   thread in _thread.h. Nonetheless, he admits that this is the only
>>   viable solution at the moment.
>>
>>   The impetus for this came from mjg's D15331:
>>   "Inline critical_enter/exit for amd64"
>>
>>   Reviewed by: jeff
>>   Differential Revision: https://reviews.freebsd.org/D16078
>>
>> Added:
>>   head/sys/kern/genoffset.c   (contents, props changed)
>>   head/sys/kern/genoffset.sh   (contents, props changed)
>>   head/sys/sys/kpilite.h   (contents, props changed)
>> Modified:
>>   head/sys/conf/kern.post.mk
>>   head/sys/conf/kern.pre.mk
>>   head/sys/kern/kern_switch.c
>>   head/sys/sys/assym.h
>>   head/sys/sys/systm.h
>>
>> Modified: head/sys/conf/kern.post.mk
>> ==
>> --- head/sys/conf/kern.post.mkMon Jul  2 22:59:29 2018
>> (r335878)
>> +++ head/sys/conf/kern.post.mkTue Jul  3 01:55:09 2018
>> (r335879)
>> @@ -185,13 +185,25 @@ hack.pico: Makefile
>>   ${CC} ${HACK_EXTRA_FLAGS} -nostdlib hack.c -o hack.pico
>>   rm -f hack.c
>>
>> -assym.inc: $S/kern/genassym.sh genassym.o
>> +offset.inc: $S/kern/genoffset.sh genoffset.o
>> + NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genoffset.sh genoffset.o > 
>> ${.TARGET}
>> +
>> +genoffset.o: $S/kern/genoffset.c
>> + ${CC} -c ${CFLAGS:N-flto:N-fno-common} $S/kern/genoffset.c
>> +
>> +genoffset_test.c: $S/kern/genoffset.c
>> + cp $S/kern/genoffset.c genoffset_test.c
>> +
>> +genoffset_test.o: genoffset_test.c offset.inc
>> + ${CC} -c ${CFLAGS:N-flto:N-fno-common} -DOFFSET_TEST genoffset_test.c
>> +
>> +assym.inc: $S/kern/genassym.sh genassym.o genoffset_test.o
>>   NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genassym.sh genassym.o > 
>> ${.TARGET}
>
> What's genoffset_test? Nothing seems to use it.
>
> --
> Regards,
> Bryan Drewery
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333590 - head/share/man/man9

[Matthew Macy]

On Mon, Jun 25, 2018 at 01:30 Andriy Gapon  wrote:

> On 25/06/2018 10:25, Ed Schouten wrote:
> > Hi Andriy, Matthew,
> >
> > 2018-06-24 23:36 GMT+02:00 Andriy Gapon :
> >> Perhaps a little application of google can help.
> >> [keywords: epoch based reclamation]
> >
> > Based on the man page, it wasn't clear that this refers to a generally
> > accepted construct or something that is specific to FreeBSD. Any
> > objections if I were to extend the man page as follows?
>
> Certainly not from me.
> Also, given that epoch(9) is a wrapper around (or based on) the
> implementation
> provided by Concurrency Kit, maybe we should have some kind of a gateway
> manual
> page for the latter.  That page would provide an introduction to
> Concurrency Kit
> and links to its resources.  Then epoch.9 could reference it.
>


There are man pages included with CK. Certain ports install them. For
reasons that he no longer recalls cognet chose to not install them. You'll
need to discuss it with him.

-M



> > Index: epoch.9
> > ===
> > --- epoch.9 (revision 335613)
> > +++ epoch.9 (working copy)
> > @@ -39,7 +39,7 @@
> >  .Nm epoch_wait ,
> >  .Nm epoch_call ,
> >  .Nm in_epoch ,
> > -.Nd kernel epoch based reclamation
> > +.Nd kernel Epoch Based Reclamation (EBR)
> >  .Sh SYNOPSIS
> >  .In sys/param.h
> >  .In sys/proc.h
> > @@ -191,3 +191,10 @@
> >  .Xr sleep 9 ,
> >  .Xr sx 9 ,
> >  .Xr timeout 9
> > +.Rs
> > +.%A K. Fraser
> > +.%T Practical lock-freedom
> > +.%D February 2004
> > +.%P 79-81
> > +.%U https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-579.pdf
> > +.Re
> >
>
>
> --
> Andriy Gapon
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333590 - head/share/man/man9

[Matthew Macy]

Sorry, I meant to respond.

http://concurrencykit.org/presentations/ebr.pdf

http://scalebsd.org/blog/2018/05/03/Why-is-munmap-single-threaded-and-what-can-we-do-about-it


http://scalebsd.org/blog/2018/06/16/UDP-and-epoch-for-liveness-guarantees


On Sun, Jun 24, 2018 at 13:56 Ed Schouten  wrote:

> Hi Matt,
>
> 2018-05-17 9:06 GMT+02:00 Ed Schouten :
> > Hi Matt,
> >
> > 2018-05-14 1:16 GMT+02:00 Matt Macy :
> >> +Epochs do not have any lock ordering issues. Entering and leaving
> >
> > It looks like the man page is missing some newlines between sentences.
> >
> > Quick question: How does this work relate to RCU
> > (https://en.wikipedia.org/wiki/Read-copy-update)? If there is any
> > relation, should we mention it in the documentation?
>
> I never got a response to this. Any thoughts?
>
> --
> Ed Schouten 
> Nuxi, 's-Hertogenbosch, the Netherlands
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335338 - in head/sys: amd64/conf conf dev/ixl modules modules/ixl modules/ixlv

[Matthew Macy]

OK. I've taken it out of NOTES until such time.
-M

On Mon, Jun 18, 2018 at 7:38 PM, Eric Joyner  wrote:
> It probably won't result in anything usable. I need to remove ixlvc.c from
> the build since the VF driver as a whole doesn't work atm.
>
> - Eric
>
> On Mon, Jun 18, 2018 at 7:05 PM Matthew Macy  wrote:
>>
>> This breaks universe / tinderbox on AMD64. You appear to have
>> committed parts of a patch to ixlvc.c.  I'm not quite clear on what
>> happened here and if just removing the '+' will produce something
>> usable.
>>
>>
>>
>> void
>> ixlv_configure_queues(struct ixlv_sc *sc)
>> {
>> device_tdev = sc->dev;
>> struct ixl_vsi*vsi = >vsi;
>> +if_softc_ctx_tscctx = iflib_get_softc_ctx(vsi->ctx);
>> +struct ixl_tx_queue*tx_que = vsi->tx_queues;
>> +struct ixl_rx_queue*rx_que = vsi->rx_queues;
>> struct tx_ring*txr;
>> struct rx_ring*rxr;
>> intlen, pairs;
>>
>> struct virtchnl_vsi_queue_config_info *vqci;
>> struct virtchnl_queue_pair_info *vqpi;
>>
>> +/* XXX: Linux PF driver wants matching ids in each tx/rx struct,
>> so both TX/RX
>> + * queues of a pair need to be configured */
>> +pairs = max(vsi->num_tx_queues, vsi->num_rx_queues);
>> len = sizeof(struct virtchnl_vsi_queue_config_info) +
>>(sizeof(struct virtchnl_queue_pair_info) * pairs);
>> vqci = malloc(len, M_DEVBUF, M_NOWAIT | M_ZERO);
>> if (!vqci) {
>> device_printf(dev, "%s: unable to allocate memory\n", __func__);
>> ixl_vc_schedule_retry(>vc_mgr);
>> return;
>> }
>> vqci->vsi_id = sc->vsi_res->vsi_id;
>> vqci->num_queue_pairs = pairs;
>> vqpi = vqci->qpair;
>> /* Size check is not needed here - HW max is 16 queue pairs, and we
>>  * can fit info for 31 of them into the AQ buffer before it overflows.
>>  */
>> +for (int i = 0; i < pairs; i++, tx_que++, rx_que++, vqpi++) {
>> +txr = _que->txr;
>> +rxr = _que->rxr;
>> +
>> vqpi->txq.vsi_id = vqci->vsi_id;
>> vqpi->txq.queue_id = i;
>> +vqpi->txq.ring_len = scctx->isc_ntxd[0];
>> +vqpi->txq.dma_ring_addr = txr->tx_paddr;
>>
>> On Mon, Jun 18, 2018 at 1:12 PM, Eric Joyner  wrote:
>> > Author: erj
>> > Date: Mon Jun 18 20:12:54 2018
>> > New Revision: 335338
>> > URL: https://svnweb.freebsd.org/changeset/base/335338
>> >
>> > Log:
>> >   ixl(4): Update to use iflib
>> >
>> >   Update the driver to use iflib in order to bring performance,
>> >   maintainability, and (hopefully) stability benefits to the driver.
>> >
>> >   The driver currently isn't completely ported; features that are
>> > missing:
>> >
>> >   - VF driver (ixlv)
>> >   - SR-IOV host support
>> >   - RDMA support
>> >
>> >   The plan is to have these re-added to the driver before the next
>> > FreeBSD release.
>> >
>> >   Reviewed by:  gallatin@
>> >   Contributions by: gallatin@, mmacy@, krzysztof.gala...@intel.com
>> >   Tested by:jeffrey.e.pie...@intel.com
>> >   MFC after:1 month
>> >   Sponsored by: Intel Corporation
>> >   Differential Revision:https://reviews.freebsd.org/D15577
>> >
>> > Added:
>> >   head/sys/dev/ixl/ixl_debug.h   (contents, props changed)
>> > Modified:
>> >   head/sys/amd64/conf/GENERIC
>> >   head/sys/conf/files.amd64
>> >   head/sys/dev/ixl/i40e_osdep.c
>> >   head/sys/dev/ixl/if_ixl.c
>> >   head/sys/dev/ixl/if_ixlv.c
>> >   head/sys/dev/ixl/ixl.h
>> >   head/sys/dev/ixl/ixl_pf.h
>> >   head/sys/dev/ixl/ixl_pf_i2c.c
>> >   head/sys/dev/ixl/ixl_pf_iov.c
>> >   head/sys/dev/ixl/ixl_pf_main.c
>> >   head/sys/dev/ixl/ixl_pf_qmgr.c
>> >   head/sys/dev/ixl/ixl_txrx.c
>> >   head/sys/dev/ixl/ixlv.h
>> >   head/sys/dev/ixl/ixlvc.c
>> >   head/sys/modules/Makefile
>> >   head/sys/modules/ixl/Makefile
>> >   head/sys/modules/ixlv/Makefile
>> >
>> > Modified: head/sys/amd64/conf/GENERIC
>> >
>> > ==
>> > --- head/sys/amd64/conf/GENERIC Mon Jun 18 19:53:11 2018
>> > (r335337)
>> > +++ head/sys

Re: svn commit: r335338 - in head/sys: amd64/conf conf dev/ixl modules modules/ixl modules/ixlv

[Matthew Macy]

This breaks universe / tinderbox on AMD64. You appear to have
committed parts of a patch to ixlvc.c.  I'm not quite clear on what
happened here and if just removing the '+' will produce something
usable.



void
ixlv_configure_queues(struct ixlv_sc *sc)
{
device_tdev = sc->dev;
struct ixl_vsi*vsi = >vsi;
+if_softc_ctx_tscctx = iflib_get_softc_ctx(vsi->ctx);
+struct ixl_tx_queue*tx_que = vsi->tx_queues;
+struct ixl_rx_queue*rx_que = vsi->rx_queues;
struct tx_ring*txr;
struct rx_ring*rxr;
intlen, pairs;

struct virtchnl_vsi_queue_config_info *vqci;
struct virtchnl_queue_pair_info *vqpi;

+/* XXX: Linux PF driver wants matching ids in each tx/rx struct,
so both TX/RX
+ * queues of a pair need to be configured */
+pairs = max(vsi->num_tx_queues, vsi->num_rx_queues);
len = sizeof(struct virtchnl_vsi_queue_config_info) +
   (sizeof(struct virtchnl_queue_pair_info) * pairs);
vqci = malloc(len, M_DEVBUF, M_NOWAIT | M_ZERO);
if (!vqci) {
device_printf(dev, "%s: unable to allocate memory\n", __func__);
ixl_vc_schedule_retry(>vc_mgr);
return;
}
vqci->vsi_id = sc->vsi_res->vsi_id;
vqci->num_queue_pairs = pairs;
vqpi = vqci->qpair;
/* Size check is not needed here - HW max is 16 queue pairs, and we
 * can fit info for 31 of them into the AQ buffer before it overflows.
 */
+for (int i = 0; i < pairs; i++, tx_que++, rx_que++, vqpi++) {
+txr = _que->txr;
+rxr = _que->rxr;
+
vqpi->txq.vsi_id = vqci->vsi_id;
vqpi->txq.queue_id = i;
+vqpi->txq.ring_len = scctx->isc_ntxd[0];
+vqpi->txq.dma_ring_addr = txr->tx_paddr;

On Mon, Jun 18, 2018 at 1:12 PM, Eric Joyner  wrote:
> Author: erj
> Date: Mon Jun 18 20:12:54 2018
> New Revision: 335338
> URL: https://svnweb.freebsd.org/changeset/base/335338
>
> Log:
>   ixl(4): Update to use iflib
>
>   Update the driver to use iflib in order to bring performance,
>   maintainability, and (hopefully) stability benefits to the driver.
>
>   The driver currently isn't completely ported; features that are missing:
>
>   - VF driver (ixlv)
>   - SR-IOV host support
>   - RDMA support
>
>   The plan is to have these re-added to the driver before the next FreeBSD 
> release.
>
>   Reviewed by:  gallatin@
>   Contributions by: gallatin@, mmacy@, krzysztof.gala...@intel.com
>   Tested by:jeffrey.e.pie...@intel.com
>   MFC after:1 month
>   Sponsored by: Intel Corporation
>   Differential Revision:https://reviews.freebsd.org/D15577
>
> Added:
>   head/sys/dev/ixl/ixl_debug.h   (contents, props changed)
> Modified:
>   head/sys/amd64/conf/GENERIC
>   head/sys/conf/files.amd64
>   head/sys/dev/ixl/i40e_osdep.c
>   head/sys/dev/ixl/if_ixl.c
>   head/sys/dev/ixl/if_ixlv.c
>   head/sys/dev/ixl/ixl.h
>   head/sys/dev/ixl/ixl_pf.h
>   head/sys/dev/ixl/ixl_pf_i2c.c
>   head/sys/dev/ixl/ixl_pf_iov.c
>   head/sys/dev/ixl/ixl_pf_main.c
>   head/sys/dev/ixl/ixl_pf_qmgr.c
>   head/sys/dev/ixl/ixl_txrx.c
>   head/sys/dev/ixl/ixlv.h
>   head/sys/dev/ixl/ixlvc.c
>   head/sys/modules/Makefile
>   head/sys/modules/ixl/Makefile
>   head/sys/modules/ixlv/Makefile
>
> Modified: head/sys/amd64/conf/GENERIC
> ==
> --- head/sys/amd64/conf/GENERIC Mon Jun 18 19:53:11 2018(r335337)
> +++ head/sys/amd64/conf/GENERIC Mon Jun 18 20:12:54 2018(r335338)
> @@ -240,8 +240,8 @@ device  em  # Intel 
> PRO/1000 Gigabit Ethernet Family
>  device ix  # Intel PRO/10GbE PCIE PF Ethernet
>  device ixv # Intel PRO/10GbE PCIE VF Ethernet
>  device ixl # Intel XL710 40Gbe PCIE Ethernet
> -optionsIXL_IW  # Enable iWARP Client 
> Interface in ixl(4)
> -device ixlv# Intel XL710 40Gbe VF PCIE Ethernet
> +#options   IXL_IW  # Enable iWARP Client 
> Interface in ixl(4)
> +#deviceixlv# Intel XL710 40Gbe VF PCIE 
> Ethernet
>  device le  # AMD Am7900 LANCE and Am79C9xx PCnet
>  device ti  # Alteon Networks Tigon I/II gigabit 
> Ethernet
>  device txp # 3Com 3cR990 (``Typhoon'')
>
> Modified: head/sys/conf/files.amd64
> ==
> --- head/sys/conf/files.amd64   Mon Jun 18 19:53:11 2018(r335337)
> +++ head/sys/conf/files.amd64   Mon Jun 18 20:12:54 2018(r335338)
> @@ -270,10 +270,10 @@ dev/ixl/ixl_pf_iov.c  optionalixl 
> pci  pci_iov \
> compile-with "${NORMAL_C} -I$S/dev/ixl"
>  dev/ixl/ixl_pf_i2c.c   optionalixl pci \
> compile-with "${NORMAL_C} 

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

[Matthew Macy]

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 2d6dc845938..35bcad41a59 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -2175,6 +2175,7 @@ soreceive_stream(struct socket *so, struct
sockaddr **psa, struct uio *uio,
struct sockbuf *sb;
struct mbuf *m, *n = NULL;

+   MPASS(controlp == NULL);
/* We only do stream sockets. */
if (so->so_type != SOCK_STREAM)
return (EINVAL);


mmacy@VogonPoetry [~|16:38|56] ssh -Y 192.168.1.40
Edit /etc/motd to change this login announcement.
mmacy@entropy [~|16:40|1] xterm &



#14 0x80bcf580 in soreceive_stream (so=0xf800441fb790,
psa=0xfe09b09b7758, uio=0xfe09b09b7790, mp0=0x0,
controlp=0xfe09b09b7778, flagsp=0xfe09b09b78f4)
at /usr/home/mmacy/devel/freebsd/sys/kern/uipc_socket.c:2178
#15 0x80bd0ba2 in soreceive (so=0xf800441fb790,
psa=0xfe09b09b7758, uio=0xfe09b09b7790, mp0=0x0,
controlp=0xfe09b09b7778, flagsp=0xfe09b09b78f4)
at /usr/home/mmacy/devel/freebsd/sys/kern/uipc_socket.c:2567
#16 0x80bd8e06 in kern_recvit (td=0xf8000c8ee000, s=3,
mp=0xfe09b09b78c8, fromseg=UIO_USERSPACE, controlp=0x0) at
/usr/home/mmacy/devel/freebsd/sys/kern/uipc_syscalls.c:945
#17 0x80bd93c7 in recvit (td=0xf8000c8ee000, s=3,
mp=0xfe09b09b78c8, namelenp=0x0) at
/usr/home/mmacy/devel/freebsd/sys/kern/uipc_syscalls.c:1053
#18 0x80bd9515 in sys_recvmsg (td=0xf8000c8ee000,
uap=0xf8000c8ee3c0) at
/usr/home/mmacy/devel/freebsd/sys/kern/uipc_syscalls.c:1169
#19 0x810cde03 in syscallenter (td=0xf8000c8ee000) at
/usr/home/mmacy/devel/freebsd/sys/amd64/amd64/../../kern/subr_syscall.c:135
#20 0x810cd64d in amd64_syscall (td=0xf8000c8ee000,
traced=0) at /usr/home/mmacy/devel/freebsd/sys/amd64/amd64/trap.c:1007

#16 0x80bd8e06 in kern_recvit (td=0xf8000c8ee000, s=3,
mp=0xfe09b09b78c8, fromseg=UIO_USERSPACE, controlp=0x0) at
/usr/home/mmacy/devel/freebsd/sys/kern/uipc_syscalls.c:945
945 error = soreceive(so, , , NULL,
(kgdb) list
940 #ifdef KTRACE
941 if (KTRPOINT(td, KTR_GENIO))
942 ktruio = cloneuio();
943 #endif
944 len = auio.uio_resid;
945 error = soreceive(so, , , NULL,
946 (mp->msg_control || controlp) ?  : NULL,
947 >msg_flags);
948 if (error != 0) {
949 if (auio.uio_resid != len && (error == ERESTART ||
(kgdb) p mp->msg_control
$1 = (void *) 0x7fffe2e0


int
sys_recvmsg(struct thread *td, struct recvmsg_args *uap)
{
struct msghdr msg;
struct iovec *uiov, *iov;
int error;

error = copyin(uap->msg, , sizeof (msg));
if (error != 0)
return (error);





TL;DR X is passing in a msghdr with a control pointer. It isn't used
for anything but returning EINVAL for it is a bug.


-M


On Thu, Jun 14, 2018 at 4:07 PM, Gleb Smirnoff  wrote:
>   Hi,
>
> On Mon, Jun 11, 2018 at 04:31:43PM +, Matt Macy wrote:
> M> Author: mmacy
> M> Date: Mon Jun 11 16:31:42 2018
> M> New Revision: 334960
> M> URL: https://svnweb.freebsd.org/changeset/base/334960
> M>
> M> Log:
> M>   soreceive_stream: correctly handle edge cases
> M>
> M>   - non NULL controlp is not an error, returning EINVAL
> M> would cause X forwarding to fail
>
> Can you please provide reference for a code that uses
> control messages with a stream socket?
>
> The removed return was in action since FreeBSD 7.0 and
> everything with X forwarding was working, AFAIK.
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r335094 - head/sys/ofed/drivers/infiniband/core

[Matthew Macy]

On Wed, Jun 13, 2018 at 4:47 PM, Ryan Libby  wrote:
> On Wed, Jun 13, 2018 at 4:30 PM, Matt Macy  wrote:
>> Author: mmacy
>> Date: Wed Jun 13 23:30:54 2018
>> New Revision: 335094
>> URL: https://svnweb.freebsd.org/changeset/base/335094
>>
>> Log:
>>   fix OFED build after r335053
>>
>> Modified:
>>   head/sys/ofed/drivers/infiniband/core/ib_user_mad.c
>>
>> Modified: head/sys/ofed/drivers/infiniband/core/ib_user_mad.c
>> ==
>> --- head/sys/ofed/drivers/infiniband/core/ib_user_mad.c Wed Jun 13 23:19:54 
>> 2018(r335093)
>> +++ head/sys/ofed/drivers/infiniband/core/ib_user_mad.c Wed Jun 13 23:30:54 
>> 2018(r335094)
>> @@ -130,7 +130,8 @@ struct ib_umad_packet {
>>
>>  static struct class *umad_class;
>>
>> -static const dev_t base_dev = MKDEV(IB_UMAD_MAJOR, IB_UMAD_MINOR_BASE);
>> +#define IBMKDEV(x, y)  (((dev_t)(x) << 32) | (unsigned)(y))
>> +static const dev_t base_dev = IBMKDEV(IB_UMAD_MAJOR, IB_UMAD_MINOR_BASE);
>>
>>  static DEFINE_SPINLOCK(port_lock);
>>  static DECLARE_BITMAP(dev_map, IB_UMAD_MAX_PORTS);
>>
>
> The scheme for major/minor encoding is different as of r335053.  Won't
> that matter?

Yes.

> In sys/ofed/drivers/infiniband/core/{ib_ucm.c,ib_uverbs_main.c} the
> pattern is to #define the MKDEV().  Following that would in
> ib_user_mad.c would also resolve this.  Or makedev could be
> re-macroized with the new scheme.

That's probably the best course of action.
Will follow up.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Fair. But it didn't actually work before. It's not clear anyone has
ever used it.

On Mon, Jun 11, 2018 at 9:54 AM, Mark Johnston  wrote:
> On Mon, Jun 11, 2018 at 04:31:43PM +, Matt Macy wrote:
>> Author: mmacy
>> Date: Mon Jun 11 16:31:42 2018
>> New Revision: 334960
>> URL: https://svnweb.freebsd.org/changeset/base/334960
>>
>> Log:
>>   soreceive_stream: correctly handle edge cases
>>
>>   - non NULL controlp is not an error, returning EINVAL
>> would cause X forwarding to fail
>>
>>   - MSG_PEEK and MSG_WAITALL are fairly exceptional, but we still
>> want to handle them - punt to soreceive_generic
>>
>> Modified:
>>   head/sys/kern/uipc_socket.c
>>
>> Modified: head/sys/kern/uipc_socket.c
>> ==
>> --- head/sys/kern/uipc_socket.c   Mon Jun 11 16:27:09 2018
>> (r334959)
>> +++ head/sys/kern/uipc_socket.c   Mon Jun 11 16:31:42 2018
>> (r334960)
>> @@ -2162,7 +2162,6 @@ release:
>>
>>  /*
>>   * Optimized version of soreceive() for stream (TCP) sockets.
>> - * XXXAO: (MSG_WAITALL | MSG_PEEK) isn't properly handled.
>>   */
>>  int
>>  soreceive_stream(struct socket *so, struct sockaddr **psa, struct uio *uio,
>> @@ -2177,12 +2176,14 @@ soreceive_stream(struct socket *so, struct sockaddr 
>> **
>>   return (EINVAL);
>>   if (psa != NULL)
>>   *psa = NULL;
>> - if (controlp != NULL)
>> - return (EINVAL);
>>   if (flagsp != NULL)
>>   flags = *flagsp &~ MSG_EOR;
>>   else
>>   flags = 0;
>> + if (flags & (MSG_PEEK|MSG_WAITALL))
>> + return (soreceive_generic(so, psa, uio, mp0, controlp, 
>> flagsp));
>> + if (controlp != NULL)
>> + *controlp = NULL;
>
> Now soreceive_stream() contains dead code to handle MSG_WAITALL, and a
> bunch of always-true checks for both flags.
>
> Changes to this code should be reviewed.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334827 - in head/sys: amd64/amd64 arm/arm dev/hwpmc i386/i386 kern mips/atheros mips/cavium powerpc/powerpc sys

[Matthew Macy]

On Sat, Jun 9, 2018 at 10:51 Mark Johnston  wrote:

> On Sat, Jun 09, 2018 at 08:11:15AM -0400, John Baldwin wrote:
> > On 6/8/18 12:34 PM, Matthew Macy wrote:
> > >> The fact that our NMI handler isn't re-entrant can lead to subtle
> > >> problems. If while executing the NMI handler we hit a dtrace
> > >> probe or DDB breakpoint, the iret executed upon return to the handler
> > >> will re-enable NMIs. Then, if a second NMI arrives before the handler
> > >> for the first has returned, the trapframe will be clobbered. Did you
> > >> rule out an issue like this?
> > >
> > > No, but it happened instantly on all CPUs an a non-debug kernel 100%
> > > of the time after I changed pmc_process_interrupt earlier this week.
> > > My voodoo fix now avoids it. What you're describing sounds episodic
> > > and doesn't sound like it would be fixed / worked around by my change.
> >
> > OTOH, a compiler bug will crop up in other places.  It is best to run
> > it to ground.  Can you describe what the bug was in more detail?
> > It would probably not be hard to come up with something you can run
> > creduce against to get down to a test case.  If you do that, the
> > LLVM folks are quite helpful and able at fixing the issue which fixes
> > it in more places than just here.
>
> The bug is the rdtscp() intrinsic added in r334746 is wrong. It was just
> copied from rdtsc(), but unlike rdtsc, rdtscp clobbers rcx, which is the
> register containing the tf pointer.
>
Thanks for identifying that. Are you going to update it?

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334827 - in head/sys: amd64/amd64 arm/arm dev/hwpmc i386/i386 kern mips/atheros mips/cavium powerpc/powerpc sys

[Matthew Macy]

> The fact that our NMI handler isn't re-entrant can lead to subtle
> problems. If while executing the NMI handler we hit a dtrace
> probe or DDB breakpoint, the iret executed upon return to the handler
> will re-enable NMIs. Then, if a second NMI arrives before the handler
> for the first has returned, the trapframe will be clobbered. Did you
> rule out an issue like this?

No, but it happened instantly on all CPUs an a non-debug kernel 100%
of the time after I changed pmc_process_interrupt earlier this week.
My voodoo fix now avoids it. What you're describing sounds episodic
and doesn't sound like it would be fixed / worked around by my change.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334827 - in head/sys: amd64/amd64 arm/arm dev/hwpmc i386/i386 kern mips/atheros mips/cavium powerpc/powerpc sys

[Matthew Macy]

On Fri, Jun 8, 2018 at 07:35 Mark Johnston  wrote:

> On Fri, Jun 08, 2018 at 04:58:03AM +, Matt Macy wrote:
> > Author: mmacy
> > Date: Fri Jun  8 04:58:03 2018
> > New Revision: 334827
> > URL: https://svnweb.freebsd.org/changeset/base/334827
> >
> > Log:
> >   hwpmc: simplify calling convention for hwpmc interrupt handling
> >
> >   pmc_process_interrupt takes 5 arguments when only 3 are needed.
> >   cpu is always available in curcpu and inuserspace can always be
> >   derived from the passed trapframe.
> >
> >   While facially a reasonable cleanup this change was motivated
> >   by the need to workaround a compiler bug.
>
> What is the compiler bug? Do you have disassembly of the subroutines in
> question?
>

We talked about this online. How would that help without engaging in a huge
diversion in to the toolchain? There is nothing wrong with the C code so I
resorted to a voodoo fix to get hwpmc working again. If you're volunteering
mjg or I or you can disassemble the code prior to my change.

>
-M

>
> >
> >   core2_intr(cpu, tf) ->
> > pmc_process_interrupt(cpu, ring, pmc, tf, inuserspace) ->
> >   pmc_add_sample(cpu, ring, pm, tf, inuserspace)
> >
> >   In the process of optimizing the tail call the tf pointer was getting
> >   clobbered:
> >
> >   (kgdb) up
> >   at /storage/mmacy/devel/freebsd/sys/dev/hwpmc/hwpmc_mod.c:4709
> >   4709
> pmc_save_kernel_callchain(ps->ps_pc,
> >   (kgdb) up
> >   1205error = pmc_process_interrupt(cpu, PMC_HR, pm,
> tf,
> >
> >   resulting in a crash in pmc_save_kernel_callchain.
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

> The main codepath which runs into this (... -> cache_lookup -> vhold) most
> definitely does not need the fence for production operation.
>
> What is unclear without audit is whether there are vhold users which need
> one. I think the patch is fine to go in if the other VI_FREE place gets a
> comment about a fence stemming from mtx_unlock and there is another one
> prior to the assertion explaining that this orders against v_iflag tests and
> may
> or may not be needed for other consumers.
>
> In general the code is *full* of data races and accidental reliance of
> ordering
> provided by amd64. Weeding this all out will be a painful exercise.
>
> Part of the problem is lack of primitives like READ_ONCE/WRITE_ONCE as
> seen in the linux kernel, someone should hack up equivalents.

CK of course has these. We can create a wrapper around them if we
don't want to use them directly.
-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334804 - in head/sys: kern modules/tcp modules/tcp/rack netinet netinet/tcp_stacks sys

[Matthew Macy]

>
> Okay. I believe there might be situations where we may want to still
> keep the 'default' stack alive. I know Windows doesn't yet use RACK when
> rtt is lesser than 10ms (or something like that), as an example.
>


Is there any reason RACK wouldn't work for tracking 10us RTTs? If we
know we know the peer doesn't do delack or have enough data in flight
and the other stack doesn't have broken LRO, could we just use this in
lieu of high resolution timestamps?

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Try r334756 and then send me your scripts for any panics you can produce.

-M

On Wed, Jun 6, 2018 at 7:12 PM, Matthew Macy  wrote:
> On Wed, Jun 6, 2018 at 12:03 PM, Peter Holm  wrote:
>> On Thu, May 17, 2018 at 05:59:35PM +, Matt Macy wrote:
>>> Author: mmacy
>>> Date: Thu May 17 17:59:35 2018
>>> New Revision: 333744
>>> URL: https://svnweb.freebsd.org/changeset/base/333744
>>>
>>> Log:
>>>   AF_UNIX: make unix socket locking finer grained
>>>
>>>   This change moves to using a reference count across lock drop / reacquire
>>>   to guarantee liveness.
>>>
>>>   Currently sends on unix sockets contend heavily on read locking the list 
>>> lock.
>>>   unix1_processes in will-it-scale peaks at 6 processes and then declines.
>>>
>>>   With this change I get a substantial improvement in number of operations 
>>> per second
>>>   with 96 processes:
>>>
>>>   x before
>>>   + after
>>>   N   Min   MaxMedian   Avg
>>> Stddev
>>>   x  11   1688420   1696389   1693578 1692766.3 
>>> 2971.1702
>>>   +  10  63417955  71030114  70662504  69576423 
>>> 2374684.6
>>>   Difference at 95.0% confidence
>>>   6.78837e+07 +/- 1.49463e+06
>>>   4010.22% +/- 88.4246%
>>>   (Student's t, pooled s = 1.63437e+06)
>>>
>>>   And even for 2 processes shows a ~18% improvement.
>>>   "Small" iron changes (1, 2, and 4 processes):
>>>
>>>   x before1
>>>   + after1.2
>>>   ++
>>>   |  + |
>>>   |   x  + |
>>>   |   x  + |
>>>   |   x  + |
>>>   |   x ++ |
>>>   |  xx ++ |
>>>   |x   x xx ++ |
>>>   |  |__A_M_AM||
>>>   ++
>>>   N   Min   MaxMedian   Avg
>>> Stddev
>>>   x  10   1131648   1197750 1197138.5 1190369.3 
>>> 20651.839
>>>   +  10   1203840   1205056   1204919 1204827.9 
>>> 353.27404
>>>   Difference at 95.0% confidence
>>>   14458.6 +/- 13723
>>>   1.21463% +/- 1.16683%
>>>   (Student's t, pooled s = 14605.2)
>>>
>>>   x before2
>>>   + after2.2
>>>   ++
>>>   |   +|
>>>   |   +|
>>>   |   +|
>>>   |   +|
>>>   |   +|
>>>   |   +|
>>>   |   x   +|
>>>   |   x   +|
>>>   | x xx  +|
>>>   |x  +|
>>>   |  |___AM_| A|
>>>   ++
>>>   N   Min   MaxMedian   Avg
>>> Stddev
>>>   x  10   1972843   2045866 2038186.5 2030443.8 
>>> 21367.694
>>>   +  10   2400853   2402196 2401043.5 2401172.7 
>>> 385.40024
>>>   Difference at 95.0% confidence
>>>   370729 +/- 14198.9
>>>   18.2585% +/- 0.826943%
>>>   (Student's t, pooled s = 15111.7)
>>>
&

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

[Matthew Macy]

On Wed, Jun 6, 2018 at 12:03 PM, Peter Holm  wrote:
> On Thu, May 17, 2018 at 05:59:35PM +, Matt Macy wrote:
>> Author: mmacy
>> Date: Thu May 17 17:59:35 2018
>> New Revision: 333744
>> URL: https://svnweb.freebsd.org/changeset/base/333744
>>
>> Log:
>>   AF_UNIX: make unix socket locking finer grained
>>
>>   This change moves to using a reference count across lock drop / reacquire
>>   to guarantee liveness.
>>
>>   Currently sends on unix sockets contend heavily on read locking the list 
>> lock.
>>   unix1_processes in will-it-scale peaks at 6 processes and then declines.
>>
>>   With this change I get a substantial improvement in number of operations 
>> per second
>>   with 96 processes:
>>
>>   x before
>>   + after
>>   N   Min   MaxMedian   AvgStddev
>>   x  11   1688420   1696389   1693578 1692766.3 2971.1702
>>   +  10  63417955  71030114  70662504  69576423 2374684.6
>>   Difference at 95.0% confidence
>>   6.78837e+07 +/- 1.49463e+06
>>   4010.22% +/- 88.4246%
>>   (Student's t, pooled s = 1.63437e+06)
>>
>>   And even for 2 processes shows a ~18% improvement.
>>   "Small" iron changes (1, 2, and 4 processes):
>>
>>   x before1
>>   + after1.2
>>   ++
>>   |  + |
>>   |   x  + |
>>   |   x  + |
>>   |   x  + |
>>   |   x ++ |
>>   |  xx ++ |
>>   |x   x xx ++ |
>>   |  |__A_M_AM||
>>   ++
>>   N   Min   MaxMedian   AvgStddev
>>   x  10   1131648   1197750 1197138.5 1190369.3 20651.839
>>   +  10   1203840   1205056   1204919 1204827.9 353.27404
>>   Difference at 95.0% confidence
>>   14458.6 +/- 13723
>>   1.21463% +/- 1.16683%
>>   (Student's t, pooled s = 14605.2)
>>
>>   x before2
>>   + after2.2
>>   ++
>>   |   +|
>>   |   +|
>>   |   +|
>>   |   +|
>>   |   +|
>>   |   +|
>>   |   x   +|
>>   |   x   +|
>>   | x xx  +|
>>   |x  +|
>>   |  |___AM_| A|
>>   ++
>>   N   Min   MaxMedian   AvgStddev
>>   x  10   1972843   2045866 2038186.5 2030443.8 21367.694
>>   +  10   2400853   2402196 2401043.5 2401172.7 385.40024
>>   Difference at 95.0% confidence
>>   370729 +/- 14198.9
>>   18.2585% +/- 0.826943%
>>   (Student's t, pooled s = 15111.7)
>>
>>   x before4
>>   + after4.2
>>   N   Min   MaxMedian   AvgStddev
>>   x  10   3986994   3991728 3990137.5 3989985.2 1300.0164
>>   +  10   470   4806664 4806116.5   4805194 1990.6625
>>   Difference at 95.0% confidence
>>   815209 +/- 1579.64
>>   20.4314% +/- 0.0421713%
>>   (Student's t, pooled s = 1681.19)
>>
>>   Tested by: pho
>>   Reported by:mjg
>>   Approved by:sbruno
>>   Sponsored by:   Limelight Networks
>>   Differential Revision:  https://reviews.freebsd.org/D15430
>>
>> Modified:
>>   head/sys/kern/uipc_usrreq.c
>>   head/sys/sys/unpcb.h
>>
>> Modified: head/sys/kern/uipc_usrreq.c
>> ==
>> --- head/sys/kern/uipc_usrreq.c   Thu May 17 17:57:41 2018
>> (r333743)
>
> This commit seems to cause this:
>
> 20180606 20:44:37 all (1/1): datagram.sh
> panic: _mtx_lock_sleep: recursed on non-recursive 

Re: svn commit: r334226 - head/lib/libpmcstat/pmu-events

[Matthew Macy]

pmu events hasn't been in libpmcstat for a while

On Wed, Jun 6, 2018 at 08:36 Bryan Drewery  wrote:

> On 6/5/18 12:32 AM, Kyle Evans wrote:
> > On Mon, Jun 4, 2018 at 10:11 PM, Kyle Evans  wrote:
> >> On Fri, May 25, 2018 at 4:46 PM, Bryan Drewery 
> wrote:
> >>> Author: bdrewery
> >>> Date: Fri May 25 21:46:07 2018
> >>> New Revision: 334226
> >>> URL: https://svnweb.freebsd.org/changeset/base/334226
> >>>
> >>> Log:
> >>>   Cleanup style
> >>>
> >>> Modified:
> >>>   head/lib/libpmcstat/pmu-events/Makefile
> >>>
> >>> Modified: head/lib/libpmcstat/pmu-events/Makefile
> >>>
> ==
> >>> --- head/lib/libpmcstat/pmu-events/Makefile Fri May 25 20:42:28
> 2018(r334225)
> >>> +++ head/lib/libpmcstat/pmu-events/Makefile Fri May 25 21:46:07
> 2018(r334226)
> >>> @@ -1,9 +1,10 @@
> >>>  # $FreeBSD$
> >>>
> >>> -PROG=jevents
> >>> -SRCS=jevents.c jsmn.c json.c
> >>> +PROG=  jevents
> >>> +SRCS=  jevents.c jsmn.c json.c
> >>>  CFLAGS+= -Wno-cast-qual
> >>> -.PATH: ${.CURDIR}
> >>> -build-tools: jevents
> >>>  MAN=
> >>> +
> >>> +build-tools: jevents
> >>> +
> >>>  .include 
> >>>
> >>
> >> Hi Bryan,
> >>
> >> Something about this appears to be subtly wrong, but I can't quite pin
> >> down what. I'm getting a failure in the lib32 build of libpmc:
> >>
> >> sh: ./pmu-events/jevents: not found
>
> I believe this was fixed by r334455.
>
> >>
> >> My observations are that ${OBJDIR}/lib/libpmc/pmu-events/jevents
> >> exists, as does ${SRCTOP}/lib/libpmc/pmu-events/jevents.
> >> ${OBJDIR}/obj-lib32/lib/libpmc, OTOH, only contains _INCSINS.meta and
> >> libpmc_events.c.meta. I'm guessing this is the problem- that
> >> obj-lib32/... doesn't have the built jevents, but it's not obvious to
> >> me why this is the case.
> >
> > Some additional context:
> >
> > SRCCONF is defined to a file that looks like this:
> >
> > WITH_BSD_GREP="YES"
> > WITHOUT_BSD_GREP_FASTMATCH="YES"
> > WITH_LOADER_LUA="YES"
> > WITHOUT_FORTH="YES"
> >
> > _SRC_ENV_CONF and __MAKE_CONF are set to /dev/null; all three set in
> > arguments to make(1). OBJDIR has been completely wiped before-hand.
> >
>
>
> --
> Regards,
> Bryan Drewery
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334701 - in head: lib/libpmc share/examples/hwpmc sys/dev/hwpmc sys/sys usr.sbin/pmc usr.sbin/pmcstat

[Matthew Macy]

 -DNO_CLEAN doesn't work with the file rename.

On Tue, Jun 5, 2018 at 10:14 PM, Cy Schubert  wrote:
> amd64
>
> ~cy
>
> In message  il.com>
> , Matthew Macy writes:
>> Which arch? Universe is passing for me (except for pre-existing
>> breakage of i386-LINT-NOINET).
>>
>> -M
>>
>> On Tue, Jun 5, 2018 at 8:53 PM, Cy Schubert  wrote
>> :
>> > In message <201806060248.w562m9tb083...@repo.freebsd.org>, Matt Macy
>> > writes:
>> >> Author: mmacy
>> >> Date: Wed Jun  6 02:48:09 2018
>> >> New Revision: 334701
>> >> URL: https://svnweb.freebsd.org/changeset/base/334701
>> >>
>> >> Log:
>> >>   hwpmc: add summary command and further metadata extensions
>> >>
>> >>   metadata changes:
>> >>   - log pmc sample rate with pmcallocate
>> >>   - log proc flags with thread / process logging
>> >> to identify user vs kernel threads
>> >>
>> >>   fixes:
>> >>   - use log cpuid to translate event id to event name
>> >>
>> >>   Implement rudimentary summary command to track sample
>> >>   counts by thread and process name within a pmc log.
>> >>
>> >>   % make -j4 buildkernel >& /dev/null &
>> >>   % sudo pmcstat -S unhalted_core_cycles -S llc-misses -O foo sleep 15
>> >>   % pmc summary foo
>> >>   cpu_clk_unhalted.thread_p_any:
>> >>   idle: 138108207162
>> >>   clang-6.0: 105336158004
>> >>   sh: 72340108510
>> >>   make: 8642012963
>> >>   kernel: 7754011631
>> >>   longest_lat_cache.miss:
>> >>   clang-6.0: 87502625
>> >>   sh: 40901227
>> >>   make: 5500165
>> >>   kernel: 3300099
>> >>   awk: 260
>> >>
>> >>   %  pmc summary -f ~/foo
>> >>   idx: 278 name: cpu_clk_unhalted.thread_p_any rate: 203
>> >>   idle: 69054
>> >>   clang-6.0: 52668
>> >>   sh: 36170
>> >>   make: 4321
>> >>   kernel: 3877
>> >>   hwpmc: proc(7445): 3319
>> >>   awk: 1289
>> >>   xargs: 357
>> >>   rand_harvestq: 181
>> >>   mtree: 102
>> >>   intr: 53
>> >>   zfskern: 31
>> >>   usb: 7
>> >>   pagedaemon: 4
>> >>   ntpd: 3
>> >>   syslogd: 1
>> >>   acpi_thermal: 1
>> >>   logger: 1
>> >>   syncer: 1
>> >>   snmptrapd: 1
>> >>   sleep: 1
>> >>   idx: 17 name: longest_lat_cache.miss rate: 13
>> >>   clang-6.0: 875
>> >>   sh: 409
>> >>   make: 55
>> >>   kernel: 33
>> >>   awk: 20
>> >>   hwpmc: proc(7445): 14
>> >>   xargs: 9
>> >>   idle: 8
>> >>   intr: 3
>> >>   zfskern: 2
>> >
>> > Hi Matt,
>> >
>> > I'm not sure if this is due to this commit or one of your others but
>> > it's broken the build.
>> >
>> > Sorry I can't do any more analysis but I need to get dinner on the
>> > table.
>> >
>> >
>> >
>> >
>> > --
>> > Cheers,
>> > Cy Schubert 
>> > FreeBSD UNIX: Web:  http://www.FreeBSD.org
>> >
>> > The need of the many outweighs the greed of the few.
>> >
>> >
>
> --
> Cheers,
> Cy Schubert 
> FreeBSD UNIX: Web:  http://www.FreeBSD.org
>
> The need of the many outweighs the greed of the few.
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334701 - in head: lib/libpmc share/examples/hwpmc sys/dev/hwpmc sys/sys usr.sbin/pmc usr.sbin/pmcstat

[Matthew Macy]

Which arch? Universe is passing for me (except for pre-existing
breakage of i386-LINT-NOINET).

-M

On Tue, Jun 5, 2018 at 8:53 PM, Cy Schubert  wrote:
> In message <201806060248.w562m9tb083...@repo.freebsd.org>, Matt Macy
> writes:
>> Author: mmacy
>> Date: Wed Jun  6 02:48:09 2018
>> New Revision: 334701
>> URL: https://svnweb.freebsd.org/changeset/base/334701
>>
>> Log:
>>   hwpmc: add summary command and further metadata extensions
>>
>>   metadata changes:
>>   - log pmc sample rate with pmcallocate
>>   - log proc flags with thread / process logging
>> to identify user vs kernel threads
>>
>>   fixes:
>>   - use log cpuid to translate event id to event name
>>
>>   Implement rudimentary summary command to track sample
>>   counts by thread and process name within a pmc log.
>>
>>   % make -j4 buildkernel >& /dev/null &
>>   % sudo pmcstat -S unhalted_core_cycles -S llc-misses -O foo sleep 15
>>   % pmc summary foo
>>   cpu_clk_unhalted.thread_p_any:
>>   idle: 138108207162
>>   clang-6.0: 105336158004
>>   sh: 72340108510
>>   make: 8642012963
>>   kernel: 7754011631
>>   longest_lat_cache.miss:
>>   clang-6.0: 87502625
>>   sh: 40901227
>>   make: 5500165
>>   kernel: 3300099
>>   awk: 260
>>
>>   %  pmc summary -f ~/foo
>>   idx: 278 name: cpu_clk_unhalted.thread_p_any rate: 203
>>   idle: 69054
>>   clang-6.0: 52668
>>   sh: 36170
>>   make: 4321
>>   kernel: 3877
>>   hwpmc: proc(7445): 3319
>>   awk: 1289
>>   xargs: 357
>>   rand_harvestq: 181
>>   mtree: 102
>>   intr: 53
>>   zfskern: 31
>>   usb: 7
>>   pagedaemon: 4
>>   ntpd: 3
>>   syslogd: 1
>>   acpi_thermal: 1
>>   logger: 1
>>   syncer: 1
>>   snmptrapd: 1
>>   sleep: 1
>>   idx: 17 name: longest_lat_cache.miss rate: 13
>>   clang-6.0: 875
>>   sh: 409
>>   make: 55
>>   kernel: 33
>>   awk: 20
>>   hwpmc: proc(7445): 14
>>   xargs: 9
>>   idle: 8
>>   intr: 3
>>   zfskern: 2
>
> Hi Matt,
>
> I'm not sure if this is due to this commit or one of your others but
> it's broken the build.
>
> Sorry I can't do any more analysis but I need to get dinner on the
> table.
>
>
>
>
> --
> Cheers,
> Cy Schubert 
> FreeBSD UNIX: Web:  http://www.FreeBSD.org
>
> The need of the many outweighs the greed of the few.
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334671 - in head/sys: net netinet netinet6 netipsec

[Matthew Macy]

This appears to have broken the NOINET build.

--- ip6_gre.o ---
cc -target i386-unknown-freebsd12.0
--sysroot=/home/mmacy/devel/build/home/mmacy/networking/i386.i386/tmp
-B/home/mmacy/devel/build/home/mmacy/networking/i386.i386/tmp/usr/bin
-O2 -pipe  -fno-strict-aliasing -Werror -D_KERNEL -DKLD_MODULE
-nostdinc   -DHAVE_KERNEL_OPTION_HEADERS -include
/home/mmacy/devel/build/home/mmacy/networking/i386.i386/sys/LINT-NOINET/opt_global.h
-I. -I/home/mmacy/networking/sys
-I/home/mmacy/networking/sys/contrib/ck/include -fno-common
-I/home/mmacy/devel/build/home/mmacy/networking/i386.i386/sys/LINT-NOINET
 -fno-builtin -MD  -MF.depend.ip6_gre.o -MTip6_gre.o -mno-mmx -mno-sse
-msoft-float -ffreestanding -fwrapv -fstack-protector -Wall
-Wredundant-decls -Wnested-externs -Wstrict-prototypes
-Wmissing-prototypes -Wpointer-arith -Wcast-qual -Wundef
-Wno-pointer-sign -D__printf__=__freebsd_kprintf__
-Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas
-Wno-error-tautological-compare -Wno-error-empty-body
-Wno-error-parentheses-equality -Wno-error-unused-function
-Wno-error-pointer-sign -Wno-error-shift-negative-value
-Wno-address-of-packed-member  -mno-aes -mno-avx  -std=iso9899:1999 -c
/home/mmacy/networking/sys/netinet6/ip6_gre.c -o ip6_gre.o
/home/mmacy/networking/sys/netinet6/ip6_gre.c:125:40: error: invalid
application of 'sizeof' to an incomplete type 'struct ip'
.min_length = sizeof(struct greip6) + sizeof(struct ip),
  ^ ~~~
/home/mmacy/networking/sys/netinet6/ip6_gre.c:125:54: note: forward
declaration of 'struct ip'
.min_length = sizeof(struct greip6) + sizeof(struct ip),
^
1 error generated.
--- if_ath_led.o --

On Tue, Jun 5, 2018 at 1:51 PM, Andrey V. Elsukov  wrote:
> Author: ae
> Date: Tue Jun  5 20:51:01 2018
> New Revision: 334671
> URL: https://svnweb.freebsd.org/changeset/base/334671
>
> Log:
>   Rework IP encapsulation handling code.
>
>   Currently it has several disadvantages:
>   - it uses single mutex to protect internal structures. It is used by
> data- and control- path, thus there are no parallelism at all.
>   - it uses single list to keep encap handlers for both INET and INET6
> families.
>   - struct encaptab keeps unneeded information (src, dst, masks, protosw),
> that isn't used by code in the source tree.
>   - matches are prioritized and when many tunneling interfaces are
> registered, encapcheck handler of each interface is invoked for each
> packet. The search takes O(n) for n interfaces. All this work is done
> with exclusive lock held.
>
>   What this patch includes:
>   - the datapath is converted to be lockless using epoch(9) KPI.
>   - struct encaptab now linked using CK_LIST.
>   - all unused fields removed from struct encaptab. Several new fields
> addedr: min_length is the minimum packet length, that encapsulation
> handler expects to see; exact_match is maximum number of bits, that
> can return an encapsulation handler, when it wants to consume a packet.
>   - IPv6 and IPv4 handlers are stored in separate lists;
>   - added new "encap_lookup_t" method, that will be used later. It is
> targeted to speedup lookup of needed interface, when gif(4)/gre(4) have
> many interfaces.
>   - the need to use protosw structure is eliminated. The only pr_input
> method was used from this structure, so I don't see the need to keep
> using it.
>   - encap_input_t method changed to avoid using mbuf tags to store softc
> pointer. Now it is passed directly trough encap_input_t method.
> encap_getarg() funtions is removed.
>   - all sockaddr structures and code that uses them removed. We don't have
> any code in the tree that uses them. All consumers use encap_attach_func()
> method, that relies on invoking of encapcheck() to determine the needed
> handler.
>   - introduced struct encap_config, it contains parameters of encap handler
> that is going to be registered by encap_attach() function.
>   - encap handlers are stored in lists ordered by exact_match value, thus
> handlers that need more bits to match will be checked first, and if
> encapcheck method returns exact_match value, the search will be stopped.
>   - all current consumers changed to use new KPI.
>
>   Reviewed by:  mmacy
>   Sponsored by: Yandex LLC
>   Differential Revision:https://reviews.freebsd.org/D15617
>
> Modified:
>   head/sys/net/if_gif.c
>   head/sys/net/if_gre.c
>   head/sys/net/if_gre.h
>   head/sys/net/if_me.c
>   head/sys/net/if_stf.c
>   head/sys/netinet/in_gif.c
>   head/sys/netinet/ip_encap.c
>   head/sys/netinet/ip_encap.h
>   head/sys/netinet/ip_gre.c
>   head/sys/netinet/ip_mroute.c
>   head/sys/netinet/pim_var.h
>   head/sys/netinet6/in6_gif.c
>   head/sys/netinet6/ip6_gre.c
>   head/sys/netinet6/ip6_mroute.c
>   head/sys/netinet6/pim6_var.h
>   

Re: svn commit: r334595 - in head: sys/dev/hwpmc sys/kern sys/sys usr.sbin/pmcstat

[Matthew Macy]

On Mon, Jun 4, 2018 at 5:08 AM, Konstantin Belousov  wrote:
> On Mon, Jun 04, 2018 at 01:10:23AM +, Matt Macy wrote:
>> @@ -2214,6 +2236,11 @@ pmc_hook_handler(struct thread *td, int function, void
>>
>>   pmc_capture_user_callchain(PCPU_GET(cpuid), PMC_HR,
>>   (struct trapframe *) arg);
>> +
>> + KASSERT(td->td_pinned == 1,
>> + ("[pmc,%d] invalid td_pinned value", __LINE__));
>> + sched_unpin();  /* Can migrate safely now. */
> sched_pin() is called from pmc_post_callchain_callback(), which is
> called from userret(). userret() is executed with interrupts and
> preemption enabled, so there is a non-trivial chance that the thread
> already migrated.
>
> In fact, I do not see a need to disable migration for the thread if user
> callchain is planned to be gathered. You only need to remember the cpu
> where the interrupt occured, to match it against the request.  Or are
> per-cpu PMC registers still accessed during callchain collection ?

The buffers are pcpu. Although it would in principle be safe in this
case since I
don't modify the read/write indices. However, I'd have to add another field for
the CPU and it doesn't handle the case of multiple migrations.

>
>> +int
>> +pmc_process_interrupt(int cpu, int ring, struct pmc *pm, struct trapframe 
>> *tf,
>> +int inuserspace)
>> +{
>> + struct thread *td;
>> +
>> + td = curthread;
>> + if ((pm->pm_flags & PMC_F_USERCALLCHAIN) &&
>> + td && td->td_proc &&
>> + (td->td_proc->p_flag & P_KPROC) == 0 &&
>> + !inuserspace) {
> I am curious why a lot of the pmc code checks for curthread != NULL and,
> like this fragment, for curproc != NULL.  I am sure that at least on x86,
> we never let curthread point to the garbage, even during the context
> switches.  NMI handler has the same cargo-cult check, BTW.

I didn't think they could be NULL, but have been cargo culting the
existing code.

> Also, please fix the indentation of the conditions block.


>
>> + atomic_add_int(>td_pmcpend, 1);
> You can use atomic_store_int() there, I believe,  Then there would be
> no locked op executed at all, on x86.

Storing a 1 would enable me to early terminate the loop.

>
>> @@ -375,6 +375,7 @@ struct thread {
>>   void*td_lkpi_task;  /* LinuxKPI task struct pointer */
>>   TAILQ_ENTRY(thread) td_epochq;  /* (t) Epoch queue. */
>>   epoch_section_t td_epoch_section; /* (t) epoch section object */
>> + int td_pmcpend;
> Why this member was not put into the zeroed region ?  Wouldn't a garbage
> there cause uneccessary ASTs ?

It would cause _1_ unnecessary check for callchains after initial
creation. Putting it in the zero area would break the ABI.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334104 - in head/sys: netinet sys

[Matthew Macy]

I've re-edited that code twice by request by others. I will amend it
again at some point to reflect this viewpoint.

On Sat, May 26, 2018 at 12:44 PM, Eric van Gyzen  wrote:
> On 05/23/2018 23:47, Gleb Smirnoff wrote:
>>
>> On Thu, May 24, 2018 at 06:44:20AM +0200, Mateusz Guzik wrote:
>> M> I fundamentally disagree with this part.
>> M>
>> M> If a known value of a given field is needed for assertion purposes, you
>> M> can add (possibly conditional) code setting this specific value. It
>> M> probably should not be zero if it can be helped.
>> M>
>> M> Conditional zeroing of the *whole* struct depending on invariants will
>> M> *hide* uninitialized memory read bugs - production kernel will have
>> M> whatever it happens to find, while *debug* kernel will guarantee to
>> M> have all the values zeroed. In fact the flag actively combats
>> redzoning.
>> M> if the resulting allocation is zeroed, poisoning is actively neutered.
>> M> But only if debug is enabled.
>> M>
>> M> That said, I find the change harmful.
>>
>> +1 on fundamentally disagree with M_ZERO_INVARIANTS. It makes the
>> INVARIANTS-enabled kernels to crash _later_ than production kernels,
>> since instead of uma_junk it places clean zeroes.
>
>
> Matt,
>
> Mateusz and Gleb raise very good points.  This operates contrary to the
> whole idea of INVARIANTS.  Please revisit this.
>
> Eric
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: Deorbiting i386

[Matthew Macy]

On Fri, May 25, 2018 at 9:24 PM, Kurt Lidl <l...@pix.net> wrote:
> On 5/24/18 3:22 PM, Matthew Macy wrote:
>>
>> i386 is definitely on the wane, but so long as it's used by more than
>> a handful of people it will be supported. All you need to know about
>> sparc64 vitality is that HEAD didn't boot for 3 months until last week.
>
>
> I stopped testing HEAD on sparc64 when the introduction if the iflib'd
> igb driver made booting on a sparc64 equipped with a dual-port igb
> PCI card stop working.

So we should have added an Ultra II off of ebay to our test matrix?
-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334118 - in head/sys: compat/linprocfs compat/linux compat/linuxkpi/common/include/linux dev/mlx5/mlx5_ib dev/wtap net net/altq netinet netinet/netdump netinet6 netpfil/pf nfs ofed/dr

[Matthew Macy]

Odd. I tested the same thing as did pho with regular interfaces. Will fix
asap.

On Fri, May 25, 2018 at 09:36 Mark Johnston  wrote:

> On Wed, May 23, 2018 at 09:02:15PM +, Matt Macy wrote:
> > Author: mmacy
> > Date: Wed May 23 21:02:14 2018
> > New Revision: 334118
> > URL: https://svnweb.freebsd.org/changeset/base/334118
> >
> > Log:
> >   UDP: further performance improvements on tx
> >
> >   Cumulative throughput while running 64
> > netperf -H $DUT -t UDP_STREAM -- -m 1
> >   on a 2x8x2 SKL went from 1.1Mpps to 2.5Mpps
> >
> >   Single stream throughput increases from 910kpps to 1.18Mpps
> >
> >   Baseline:
> >   https://people.freebsd.org/~mmacy/2018.05.11/udpsender2.svg
> >
> >   - Protect read access to global ifnet list with epoch
> >   https://people.freebsd.org/~mmacy/2018.05.11/udpsender3.svg
> >
> >   - Protect short lived ifaddr references with epoch
> >   https://people.freebsd.org/~mmacy/2018.05.11/udpsender4.svg
> >
> >   - Convert if_afdata read lock path to epoch
> >   https://people.freebsd.org/~mmacy/2018.05.11/udpsender5.svg
>
> After this change I can panic an INVARIANTS kernel like so. There is a
> freed ifaddr lingering on the lo0 ifnet's address list.
>
> # ifconfig lo0 127.0.0.2
> # ifconfig lo0 -alias 127.0.0.2
> # netstat -rn
> Routing tables
>
>
> Fatal trap 9: general protection fault while in kernel mode
> cpuid = 1; apic id = 01
> instruction pointer = 0x20:0x80605219
> stack pointer   = 0x28:0xfe4692e0
> frame pointer   = 0x28:0xfe4693d0
> code segment= base 0x0, limit 0xf, type 0x1b
> = DPL 0, pres 1, long 1, def32 0, gran 1
> processor eflags= interrupt enabled, resume, IOPL = 0
> current process = 735 (netstat)
> trap number = 9
> panic: general protection fault
> cpuid = 1
> time = 1527266002
> Uptime: 1m42s
> Dumping 216 out of 4079
> MB:..8%..15%..23%..37%..45%..52%..67%..74%..82%..97%
> Dump complete
> Consoles: userboot
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334189 - head/sys/contrib/ck/include

[Matthew Macy]

I asked for this, so working on a fix. Let me know if you're already
about to commit.

On Thu, May 24, 2018 at 3:14 PM, John Baldwin  wrote:
> On Thursday, May 24, 2018 09:38:18 PM Olivier Houchard wrote:
>> Author: cognet
>> Date: Thu May 24 21:38:18 2018
>> New Revision: 334189
>> URL: https://svnweb.freebsd.org/changeset/base/334189
>>
>> Log:
>>   Import CK as of commit 0f017230ccc86929f56bf44ef2dca93d7df8076b.
>>   This brings us the renaming of fields in ck_queue, so that our own
>>   LIST/SLIST/TAILQ/etc won't accidentally work with them.
>
> Thanks.
>
> --
> John Baldwin
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334128 - in head: . lib/libpmcstat lib/libpmcstat/pmu-events lib/libpmcstat/pmu-events/arch lib/libpmcstat/pmu-events/arch/arm64 lib/libpmcstat/pmu-events/arch/arm64/arm lib/libpmcsta

[Matthew Macy]

On Thu, May 24, 2018 at 3:36 PM, John Baldwin  wrote:
> On Thursday, May 24, 2018 04:30:06 AM Matt Macy wrote:
>> Author: mmacy
>> Date: Thu May 24 04:30:06 2018
>> New Revision: 334128
>> URL: https://svnweb.freebsd.org/changeset/base/334128
>>
>> Log:
>>   libpmcstat: compile in events based on json description
>
> Please provide more detail for commits like this in the future.  Some 
> questions
> I have some of which probably should have been answered in the log:
>
> - Why does this matter?  For example, does this provide counters for a CPUs
>   we don't currently support counters on?
> - Does this supplant the existing tables of counters in hwpmc or is this
>   a different set of counters (or how do these counter tables interact with
>   our existing tables in general)?
> - What is the origin of these json files?  What is the license?  These seem to
>   come straight from the Linux kernel which should be acknowledged via
>   Obtained from: at the very least.  It does seem that the jevents source is
>   explicitly BSD licensed (2 clause).  This is noted in the git logs in the
>   Linux tree.  However, I couldn't find a clear statement on the json files
>   themselves and what their license is.  Are you aware of a license for those
>   or do they fall under the GPLv2 "default" license in /COPYING of the Linux
>   source tree?

At least the Intel ones come from Intel where everything is BSD
licensed. If the others are GPL I'll move it all to a port.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: Deorbiting i386

[Matthew Macy]

On Thu, May 24, 2018 at 8:51 AM, Andrew Gallatin  wrote:
> On 05/23/18 20:09, Pedro Giffuni wrote:
>>
>> FWIW;
>>
>> On 23/05/2018 17:18, Cy Schubert wrote:
>>>
>>> In message <20180523202228.gc58...@spindle.one-eyed-alien.net>, Brooks
>>> Davis wr
>>> ites:


 --QRj9sO5tAVLaXnSD
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable

 On Thu, May 24, 2018 at 02:53:16AM +0700, Eugene Grosbein wrote:
>
> 24.05.2018 2:30, Cy Schubert wrote:
> =20
>>
>> Except for old computers and old software that segfaults on 64-bit,
>> how=

   many people still use i386?
>>
>> =20
>> Full disclosure: I'd like to see i386 deorbited before I retire.
>
> =20
> Plese don't. I routinely use FreeBSD11/i386 for cheap VPS hosts having
> le=

 ss than 2G memory
>
> because amd64 has noticeable overhead. I even have ZFS-only i386 VPS,
> her=

 e is live example with 1G only:
>
> =20
> Mem: 10M Active, 69M Inact, 230M Wired, 685M Free
> ARC: 75M Total, 1953K MFU, 31M MRU, 172K Anon, 592K Header, 42M Other
>   3500K Compressed, 29M Uncompressed, 8.61:1 Ratio
> Swap: 1024M Total, 1024M Free
> =20
> The VPS has only 20G of disk space and ZFS compression gives
> compressratio 2.22x for ports, 2.51x for src, 2.29x for obj
> and 1.95x for installed i386 system plus other software and data.

 I think we're quite a ways from being ready to axe i386.

 For VPS applications, we should probably get x32 support in place which
 should give us the best of both worlds.

 That said, we either need to rev the i386 ABI to use a 64-bit time_t or
 kill it in the not to distant future or we risk embedded systems failing
 in place in 2038.  If we assume a 15 year life for most equipment to
 fail electrically or mechanically that says FreeBSD 13 shouldn't support
 the current i386 ABI.
>>>
>>> Rereading this, I'm confused. FreeBSD 13? 2023? Either works for me,
>>> though 2023 is more reasonable and gives people more than enough time
>>> to migrate.
>>>
>>>
>> IMHO, we shouldn't at all plan to deorbit i386: it is a platform that is
>> very easy to test on Jenkins/bhyve. If we want to keep FreeBSD multiplatform
>> it is way easier to test and find bugs on i386 than on other 32 bit
>> platforms. It is fully functional and much more than historic value.
>>
>> X32 sadly didn't catch on linux on AFAICT, although I wouldn't object to
>> it appearing on FreeBSD.
>>
>> Pedro
>
> On the other hand, supporting all these architectures makes it rather
> challenging to make changes to the kernel & feel like you've tested
> them sufficiently.   Even build testing a header file change can take
> the better part of a day with make universe.  I'm quite jealous of
> Dragonfly's amd64 only approach for this reason.
>
> I realize this may be seen as ironic, as I'm one of the people that
> dragged FreeBSD into the multiplatform world with the FreeBSD/alpha
> port 20 years ago. On the other hand, I was happy to retire
> FreeBSD/alpha when the Alpha stopped being commercially viable.
> I always viewed NetBSD as the proper place for "historical" platforms
> to run BSD.   I wish we could retire some of the minor/historical
> platforms that we have now.
>

Additional platforms don't come without a cost. When I was doing
i386-xen and sun4v alc would routinely break the port and I was _fine_
with that. He shouldn't be slowing down for something marginal. When
it was clear to me that sun4v was never going to get much uptake I
asked that it be removed.

i386 is definitely on the wane, but so long as it's used by more than
a handful of people it will be supported. All you need to know about
sparc64 vitality is that HEAD didn't boot for 3 months until last
week.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

>
> False positives are compiler bugs.

No they're not. No more than missed optimization opportunities.
They're limitations in the control flow analysis.

>
> It does happen, with GCC more than with clang, that the compiler has too
> many bugs and it's a bad practice to pessimize code to work around them.

It doesn't pessimize the compiled output. It may make the code less
readable in the opinion of some.  I've found that 10% of the warnings
were actually legitimate issues. It's unfortunate that the majority
are just noise that have to be waded through.

>  At
> very least you should add a comment when adding unnecessary initializations,
> something like /* workaround GCC */, but dropping broken warnings is best.

That's legitimate.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Thu, May 24, 2018 at 8:58 AM, Warner Losh <i...@bsdimp.com> wrote:
>
>
> On Thu, May 24, 2018 at 12:53 AM, Matthew Macy <mm...@freebsd.org> wrote:
>>
>> On Wed, May 23, 2018 at 11:42 PM, Michael Tuexen
>> <michael.tue...@macmic.franken.de> wrote:
>> >> On 24. May 2018, at 08:36, Matthew Macy <mm...@freebsd.org> wrote:
>> >>
>> >> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>> >> <michael.tue...@macmic.franken.de> wrote:
>> >>>> On 24. May 2018, at 06:51, Matthew Macy <mm...@freebsd.org> wrote:
>> >>>>
>> >>>> Warnings find bugs PERIOD. Although most are not useful, I've found
>> >>> Some warnings indicate bugs, some warnings are just wrong. If you
>> >>> have a "may be used uninitialized" warning being a false positive, you
>> >>> may silences the warning by just set it to zero in the declaration and
>> >>> you silence it. Other compilers might then correctly report an
>> >>> assignment without affect...
>> >>
>> >> I have yet to see a double assignment be flagged as assignment without
>> >> effect. If it _does_ occur then we have to disable the warning on the
>> >> compiler that we have less faith in.
>> > Have seen it in the past in a difference project... But you miss my
>> > point:
>> >
>> > Not all warnings indicate bugs PERIOD. Some warning are just wrong...
>>
>> Have you read my follow up? _Many_ Many warnings are wrong. Please
>> respond to that on what the global policy should be. The value of any
>> one particular instance of a warning does not merit discussion.
>
>
> The global policy has never been 'fix all warnings no matter what.' It's
> been 'Look at the warning. If it's a false positive, use judgement about
> whether or not to stifle the compiler.' There are cases I've run into that
> it was impossible to silence the warnings (apart form adding command line
> stuff) for a particular bit of code. Do it one way gcc 4.2 complains. Do it
> another clang complains. appease both and gcc 6 had heart-burn.
>
> So don't gratuitously commit code that fixes warnings on gcc 8. If the
> warning points out a legitimate bug, then that's no brainer yes. If it's a
> false positive, then it's less clear and often times many factors may need
> to be weighed.

Non-actionable warnings are actively detrimental to workflow. They
hide real issues and lead to apathy by developers. If pacifying a
warning is considered undesirable it should be disabled by default
with perhaps a separate mode for enabling it.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Thu, May 24, 2018 at 8:54 AM, Warner Losh <i...@bsdimp.com> wrote:
>
>
> On Thu, May 24, 2018 at 12:36 AM, Matthew Macy <mm...@freebsd.org> wrote:
>>
>> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>> <michael.tue...@macmic.franken.de> wrote:
>> >> On 24. May 2018, at 06:51, Matthew Macy <mm...@freebsd.org> wrote:
>> >>
>> >> Warnings find bugs PERIOD. Although most are not useful, I've found
>> > Some warnings indicate bugs, some warnings are just wrong. If you
>> > have a "may be used uninitialized" warning being a false positive, you
>> > may silences the warning by just set it to zero in the declaration and
>> > you silence it. Other compilers might then correctly report an
>> > assignment without affect...
>>
>> I have yet to see a double assignment be flagged as assignment without
>> effect. If it _does_ occur then we have to disable the warning on the
>> compiler that we have less faith in.
>
>
> Coverity does exactly that.

:-(
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334128 - in head: . lib/libpmcstat lib/libpmcstat/pmu-events lib/libpmcstat/pmu-events/arch lib/libpmcstat/pmu-events/arch/arm64 lib/libpmcstat/pmu-events/arch/arm64/arm lib/libpmcsta

[Matthew Macy]

Ugh. Not intentional

On Thu, May 24, 2018 at 02:35 Sergey Kandaurov  wrote:

> On 24 May 2018 at 07:30, Matt Macy  wrote:
>
>> Author: mmacy
>> Date: Thu May 24 04:30:06 2018
>> New Revision: 334128
>> URL: https://svnweb.freebsd.org/changeset/base/334128
>>
>> Log:
>>   libpmcstat: compile in events based on json description
>>
> [...]
>
>
>>
>> Modified: head/Makefile
>>
>> ==
>> --- head/Makefile   Thu May 24 03:44:12 2018(r334127)
>> +++ head/Makefile   Thu May 24 04:30:06 2018(r334128)
>> @@ -481,7 +481,8 @@ worlds: .PHONY
>>  # existing system is.
>>  #
>>  .if make(universe) || make(universe_kernels) || make(tinderbox) ||
>> make(targets)
>> -TARGETS?=amd64 arm arm64 i386 mips powerpc riscv sparc64
>> +TARGETS?=amd64 i386 powerpc arm64
>> +#riscv arm  sparc64 mips
>>  _UNIVERSE_TARGETS= ${TARGETS}
>>  TARGET_ARCHES_arm?=arm armeb armv6 armv7
>>  TARGET_ARCHES_arm64?=  aarch64
>>
>
> Why? Looks like a debug leftover.
>
> --
> wbr,
> pluknet
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334134 - head/lib/libpmcstat

[Matthew Macy]

Bugs in the code I'd just imported because I imported from the wrong branch :-/

On Wed, May 23, 2018 at 11:50 PM, Cy Schubert  wrote:
> In message <201805240647.w4o6lesd071...@repo.freebsd.org>, Matt Macy
> writes:
>> Author: mmacy
>> Date: Thu May 24 06:47:40 2018
>> New Revision: 334134
>> URL: https://svnweb.freebsd.org/changeset/base/334134
>>
>> Log:
>>   libpmcstat: import aliases table and bug fixes
>
> Which bugs?
>
>>
>> Modified:
>>   head/lib/libpmcstat/libpmcstat_pmu_util.c
>>
>> Modified: head/lib/libpmcstat/libpmcstat_pmu_util.c
>> =
>> =
>> --- head/lib/libpmcstat/libpmcstat_pmu_util.c Thu May 24 06:47:04 2018
>>   (r334133)
>> +++ head/lib/libpmcstat/libpmcstat_pmu_util.c Thu May 24 06:47:40 2018
>>   (r334134)
>> @@ -42,6 +42,39 @@
>>  #include "pmu-events/pmu-events.h"
>>
>>  #if defined(__amd64__)
>> +struct pmu_alias {
>> + const char *pa_alias;
>> + const char *pa_name;
>> +};
>> +static struct pmu_alias pmu_alias_table[] = {
>> +{ "UNHALTED_CORE_CYCLES", "CPU_CLK_UNHALTED.THREAD_P_ANY"},
>> +{ "UNHALTED-CORE-CYCLES", "CPU_CLK_UNHALTED.THREAD_P_ANY"},
>> + { "LLC_MISSES", "LONGEST_LAT_CACHE.MISS"},
>> + { "LLC-MISSES", "LONGEST_LAT_CACHE.MISS"},
>> + { "LLC_REFERENCE", "LONGEST_LAT_CACHE.REFERENCE"},
>> + { "LLC-REFERENCE", "LONGEST_LAT_CACHE.REFERENCE"},
>> + { "LLC_MISS_RHITM", "mem_load_l3_miss_retired.remote_hitm"},
>> + { "LLC-MISS-RHITM", "mem_load_l3_miss_retired.remote_hitm"},
>> + { "RESOURCE_STALL", "RESOURCE_STALLS.ANY"},
>> + { "RESOURCE_STALLS_ANY", "RESOURCE_STALLS.ANY"},
>> + { "BRANCH_INSTRUCTION_RETIRED", "BR_INST_RETIRED.ALL_BRANCHES"},
>> + { "BRANCH-INSTRUCTION-RETIRED", "BR_INST_RETIRED.ALL_BRANCHES"},
>> + { "BRANCH_MISSES_RETIRED", "BR_MISP_RETIRED.ALL_BRANCHES"},
>> + { "BRANCH-MISSES-RETIRED", "BR_MISP_RETIRED.ALL_BRANCHES"},
>> + { NULL, NULL },
>> +};
>> +
>> +static const char *
>> +pmu_alias_get(const char *name)
>> +{
>> + struct pmu_alias *pa;
>> +
>> + for (pa = pmu_alias_table; pa->pa_alias != NULL; pa++)
>> + if (strcasecmp(name, pa->pa_alias) == 0)
>> + return (pa->pa_name);
>> + return (name);
>> +}
>> +
>>  struct pmu_event_desc {
>>   uint32_t ped_umask;
>>   uint32_t ped_event;
>> @@ -75,9 +108,12 @@ pmu_event_get(const char *event_name)
>>
>>   if ((pme = pmu_events_map_get()) == NULL)
>>   return (NULL);
>> - for (pe = pme->table; pe->name != NULL; pe++)
>> - if (strcmp(pe->name, event_name) == 0)
>> + for (pe = pme->table; pe->name || pe->desc || pe->event; pe++) {
>> + if (pe->name == NULL)
>> + continue;
>> + if (strcasecmp(pe->name, event_name) == 0)
>>   return (pe);
>> + }
>>   return (NULL);
>>  }
>>
>> @@ -100,7 +136,7 @@ pmu_parse_event(struct pmu_event_desc *ped, const char
>>   if (strcmp(key, "event") == 0)
>>   ped->ped_event = strtol(value, NULL, 16);
>>   if (strcmp(key, "period") == 0)
>> - ped->ped_umask = strtol(value, NULL, 10);
>> + ped->ped_period = strtol(value, NULL, 10);
>>   }
>>   free(event);
>>   return (0);
>> @@ -112,6 +148,7 @@ pmcstat_pmu_sample_rate_get(const char *event_name)
>>   const struct pmu_event *pe;
>>   struct pmu_event_desc ped;
>>
>> + event_name = pmu_alias_get(event_name);
>>   if ((pe = pmu_event_get(event_name)) == NULL)
>>   return (DEFAULT_SAMPLE_COUNT);
>>   if (pe->alias && (pe = pmu_event_get(pe->alias)) == NULL)
>>
>
> --
> Cheers,
> Cy Schubert 
> FreeBSD UNIX:     Web:  http://www.FreeBSD.org
>
> The need of the many outweighs the greed of the few.
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Wed, May 23, 2018 at 11:42 PM, Michael Tuexen
<michael.tue...@macmic.franken.de> wrote:
>> On 24. May 2018, at 08:36, Matthew Macy <mm...@freebsd.org> wrote:
>>
>> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>> <michael.tue...@macmic.franken.de> wrote:
>>>> On 24. May 2018, at 06:51, Matthew Macy <mm...@freebsd.org> wrote:
>>>>
>>>> Warnings find bugs PERIOD. Although most are not useful, I've found
>>> Some warnings indicate bugs, some warnings are just wrong. If you
>>> have a "may be used uninitialized" warning being a false positive, you
>>> may silences the warning by just set it to zero in the declaration and
>>> you silence it. Other compilers might then correctly report an
>>> assignment without affect...
>>
>> I have yet to see a double assignment be flagged as assignment without
>> effect. If it _does_ occur then we have to disable the warning on the
>> compiler that we have less faith in.
> Have seen it in the past in a difference project... But you miss my point:
>
> Not all warnings indicate bugs PERIOD. Some warning are just wrong...

Have you read my follow up? _Many_ Many warnings are wrong. Please
respond to that on what the global policy should be. The value of any
one particular instance of a warning does not merit discussion.
-M











, in this case the assignment should be added with
>>>>> a comment /* pacify gcc */.
>>>>>
>>>>> On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
>>>>> M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff <gleb...@freebsd.org> 
>>>>> wrote:
>>>>> M> > The initialization isn't useful.
>>>>> M>
>>>>> M> It silences a gcc warning. So yes it is. It's this exchange which is 
>>>>> not useful.
>>>>> M>
>>>>> M> -M
>>>>> M>
>>>>> M>
>>>>> M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
>>>>> M> > M> Talk to the gcc devs. The warning is useful even if there are 
>>>>> false positives.
>>>>> M> > M>
>>>>> M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
>>>>> <gleb...@freebsd.org> wrote:
>>>>> M> > M> >   Hi,
>>>>> M> > M> >
>>>>> M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
>>>>> M> > M> > M> Author: mmacy
>>>>> M> > M> > M> Date: Sat May 19 05:10:51 2018
>>>>> M> > M> > M> New Revision: 333860
>>>>> M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
>>>>> M> > M> > M>
>>>>> M> > M> > M> Log:
>>>>> M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
>>>>> actually initialized
>>>>> M> > M> > M>
>>>>> M> > M> > M> Modified:
>>>>> M> > M> > M>   head/sys/kern/kern_sendfile.c
>>>>> M> > M> > M>
>>>>> M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
>>>>> M> > M> > M> 
>>>>> ==
>>>>> M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 
>>>>> 2018(r333859)
>>>>> M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 
>>>>> 2018(r333860)
>>>>> M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
>>>>> sf_io *sfio, o
>>>>> M> > M> > M>  }
>>>>> M> > M> > M>
>>>>> M> > M> > M>  for (int i = 0; i < npages;) {
>>>>> M> > M> > M> -int j, a, count, rv;
>>>>> M> > M> > M> +int j, a, count, rv __unused;
>>>>> M> > M> > M>
>>>>> M> > M> > M>  /* Skip valid pages. */
>>>>> M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
>>>>> PAGE_MASK,
>>>>> M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
>>>>> M> > M> > M>  if (space == 0) {
>>>>> M> > M> > M>  sfio = NULL;
>>>>> M> > M> > M>  nios = 0;
>>>>> M> > M> > M> +npages = 0;
>>>>> M> > M> > M>  goto prepend_header;
>>>>> M> > M> > M>  }
>>>>> M> > M> > M>  hdr_uio = NULL;
>>>>> M> > M> >
>>>>> M> > M> > This initialization is redundant and a compiler warning if 
>>>>> exists is wrong.
>>>>> M> > M> >
>>>>> M> > M> > If we jump down to prepend_header with nios == 0, we won't ever 
>>>>> use npages.
>>>>> M> > M> >
>>>>> M> > M> > --
>>>>> M> > M> > Gleb Smirnoff
>>>>> M> >
>>>>> M> > --
>>>>> M> > Gleb Smirnoff
>>>>>
>>>>> --
>>>>> Gleb Smirnoff
>>>>
>>>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Wed, May 23, 2018 at 10:25 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
> On Wed, May 23, 2018 at 10:13:25PM -0700, Matthew Macy wrote:
> M> On Wed, May 23, 2018 at 10:07 PM, Gleb Smirnoff <gleb...@freebsd.org> 
> wrote:
> M> > Can you please explain the bug supposed to be fixed by r333860 QUESTION 
> MARK
> M>
> M> Did I say it fixed a bug? Or are you saying we should just turn off
> M> compiler warnings because Gleb Smirnoff knows better?
>
> You just did say "Warnings find bugs PERIOD." This implicitly claimed
> that shutting this one fixed a bug.
>
> Indeed, I am saying that in this particular case Gleb Smirnoff knows
> better than gcc8 does. However, I'm not saying to turn off compiler warnings.
> Never said that. I am saying (several times already) not to insert redundant
> code to shut up a warning that is false.

I'm not saying you're right or wrong. On initial inspection it
appeared uninitialized. I may be wrong. I never asserted that you were
wrong. What I'm trying to understand is what your desired policy with
respect to warnings is. If the author knows that a warning is false he
should

a) Disable that warning for the entire build because it is wrong in one case?

b) Edit the Makefile to disable the warning for that file?

c) Leave the warning enabled and just permit a noisy build where the
warning potentially drowns out real issues?

Right now you're discussing a single line in a single file when in
fact this is a kernel wide policy issue. All three of these choices
seem less desirable than selectively placating the compiler as I have
en masse for much of the tree.


-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
<michael.tue...@macmic.franken.de> wrote:
>> On 24. May 2018, at 06:51, Matthew Macy <mm...@freebsd.org> wrote:
>>
>> Warnings find bugs PERIOD. Although most are not useful, I've found
> Some warnings indicate bugs, some warnings are just wrong. If you
> have a "may be used uninitialized" warning being a false positive, you
> may silences the warning by just set it to zero in the declaration and
> you silence it. Other compilers might then correctly report an
> assignment without affect...

I have yet to see a double assignment be flagged as assignment without
effect. If it _does_ occur then we have to disable the warning on the
compiler that we have less faith in.
-M


>
> Best regards
> Michael
>> quite a number of real issues from compiling with gcc8.
>>
>> If you want to _actually_ be helpful fix these:
>> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log
>>
>> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log
>>
>> On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
>>> Let me repeat again. The warning is a false positive, and thus assignment
>>> isn't useful. I'm not worried about a single instruction, more about
>>> polluting the code.
>>>
>>> If the warning was escalated to build error, and we did carry about
>>> building with gcc8, in this case the assignment should be added with
>>> a comment /* pacify gcc */.
>>>
>>> On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
>>> M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff <gleb...@freebsd.org> 
>>> wrote:
>>> M> > The initialization isn't useful.
>>> M>
>>> M> It silences a gcc warning. So yes it is. It's this exchange which is not 
>>> useful.
>>> M>
>>> M> -M
>>> M>
>>> M>
>>> M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
>>> M> > M> Talk to the gcc devs. The warning is useful even if there are false 
>>> positives.
>>> M> > M>
>>> M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
>>> <gleb...@freebsd.org> wrote:
>>> M> > M> >   Hi,
>>> M> > M> >
>>> M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
>>> M> > M> > M> Author: mmacy
>>> M> > M> > M> Date: Sat May 19 05:10:51 2018
>>> M> > M> > M> New Revision: 333860
>>> M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
>>> M> > M> > M>
>>> M> > M> > M> Log:
>>> M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
>>> actually initialized
>>> M> > M> > M>
>>> M> > M> > M> Modified:
>>> M> > M> > M>   head/sys/kern/kern_sendfile.c
>>> M> > M> > M>
>>> M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
>>> M> > M> > M> 
>>> ==
>>> M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018  
>>>   (r333859)
>>> M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018  
>>>   (r333860)
>>> M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
>>> sf_io *sfio, o
>>> M> > M> > M>  }
>>> M> > M> > M>
>>> M> > M> > M>  for (int i = 0; i < npages;) {
>>> M> > M> > M> -int j, a, count, rv;
>>> M> > M> > M> +int j, a, count, rv __unused;
>>> M> > M> > M>
>>> M> > M> > M>  /* Skip valid pages. */
>>> M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
>>> PAGE_MASK,
>>> M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
>>> M> > M> > M>  if (space == 0) {
>>> M> > M> > M>  sfio = NULL;
>>> M> > M> > M>  nios = 0;
>>> M> > M> > M> +npages = 0;
>>> M> > M> > M>  goto prepend_header;
>>> M> > M> > M>  }
>>> M> > M> > M>  hdr_uio = NULL;
>>> M> > M> >
>>> M> > M> > This initialization is redundant and a compiler warning if exists 
>>> is wrong.
>>> M> > M> >
>>> M> > M> > If we jump down to prepend_header with nios == 0, we won't ever 
>>> use npages.
>>> M> > M> >
>>> M> > M> > --
>>> M> > M> > Gleb Smirnoff
>>> M> >
>>> M> > --
>>> M> > Gleb Smirnoff
>>>
>>> --
>>> Gleb Smirnoff
>>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Warnings find bugs PERIOD. Although most are not useful, I've found
quite a number of real issues from compiling with gcc8.

If you want to _actually_ be helpful fix these:
https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log

https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log

On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
> Let me repeat again. The warning is a false positive, and thus assignment
> isn't useful. I'm not worried about a single instruction, more about
> polluting the code.
>
> If the warning was escalated to build error, and we did carry about
> building with gcc8, in this case the assignment should be added with
> a comment /* pacify gcc */.
>
> On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
> M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
> M> > The initialization isn't useful.
> M>
> M> It silences a gcc warning. So yes it is. It's this exchange which is not 
> useful.
> M>
> M> -M
> M>
> M>
> M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> M> > M> Talk to the gcc devs. The warning is useful even if there are false 
> positives.
> M> > M>
> M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff <gleb...@freebsd.org> 
> wrote:
> M> > M> >   Hi,
> M> > M> >
> M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> > M> > M> Author: mmacy
> M> > M> > M> Date: Sat May 19 05:10:51 2018
> M> > M> > M> New Revision: 333860
> M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M> > M> > M>
> M> > M> > M> Log:
> M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
> actually initialized
> M> > M> > M>
> M> > M> > M> Modified:
> M> > M> > M>   head/sys/kern/kern_sendfile.c
> M> > M> > M>
> M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
> M> > M> > M> 
> ==
> M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
> (r333859)
> M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
> (r333860)
> M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
> sf_io *sfio, o
> M> > M> > M>  }
> M> > M> > M>
> M> > M> > M>  for (int i = 0; i < npages;) {
> M> > M> > M> -int j, a, count, rv;
> M> > M> > M> +int j, a, count, rv __unused;
> M> > M> > M>
> M> > M> > M>  /* Skip valid pages. */
> M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
> PAGE_MASK,
> M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
> M> > M> > M>  if (space == 0) {
> M> > M> > M>  sfio = NULL;
> M> > M> > M>  nios = 0;
> M> > M> > M> +npages = 0;
> M> > M> > M>  goto prepend_header;
> M> > M> > M>  }
> M> > M> > M>  hdr_uio = NULL;
> M> > M> >
> M> > M> > This initialization is redundant and a compiler warning if exists 
> is wrong.
> M> > M> >
> M> > M> > If we jump down to prepend_header with nios == 0, we won't ever use 
> npages.
> M> > M> >
> M> > M> > --
> M> > M> > Gleb Smirnoff
> M> >
> M> > --
> M> > Gleb Smirnoff
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334129 - head/sys/amd64/conf

[Matthew Macy]

On Wed, May 23, 2018 at 10:32 PM, Ravi Pokala  wrote:
> -Original Message-
> From:  on behalf of Matt Macy 
> 
> Date: 2018-05-23, Wednesday at 21:31
> To: , , 
> 
> Subject: svn commit: r334129 - head/sys/amd64/conf
>
>> Author: mmacy
>> Date: Thu May 24 04:31:53 2018
>> New Revision: 334129
>> URL: https://svnweb.freebsd.org/changeset/base/334129
>>
>> Log:
>>   take NUMA out
>>
>> Modified:
>>   head/sys/amd64/conf/GENERIC-NODEBUG
>>
>> Modified: head/sys/amd64/conf/GENERIC-NODEBUG
>> ==
>> --- head/sys/amd64/conf/GENERIC-NODEBUG   Thu May 24 04:30:06 2018   
>>  (r334128)
>> +++ head/sys/amd64/conf/GENERIC-NODEBUG   Thu May 24 04:31:53 2018   
>>  (r334129)
>> @@ -38,4 +38,3 @@ nooptions   BUF_TRACKING
>>  nooptions   DEADLKRES
>>  nooptions   FULL_BUF_TRACKING
>>
>> -options  NUMA
>
> Why?


I ^C'd the prior commit too late.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Wed, May 23, 2018 at 10:07 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
> Can you please explain the bug supposed to be fixed by r333860 QUESTION MARK

Did I say it fixed a bug? Or are you saying we should just turn off
compiler warnings because Gleb Smirnoff knows better?

-M


> On Wed, May 23, 2018 at 09:51:34PM -0700, Matthew Macy wrote:
> M> Warnings find bugs PERIOD. Although most are not useful, I've found
> M> quite a number of real issues from compiling with gcc8.
> M>
> M> If you want to _actually_ be helpful fix these:
> M> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log
> M>
> M> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log
> M>
> M> On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
> M> > Let me repeat again. The warning is a false positive, and thus assignment
> M> > isn't useful. I'm not worried about a single instruction, more about
> M> > polluting the code.
> M> >
> M> > If the warning was escalated to build error, and we did carry about
> M> > building with gcc8, in this case the assignment should be added with
> M> > a comment /* pacify gcc */.
> M> >
> M> > On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
> M> > M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff <gleb...@freebsd.org> 
> wrote:
> M> > M> > The initialization isn't useful.
> M> > M>
> M> > M> It silences a gcc warning. So yes it is. It's this exchange which is 
> not useful.
> M> > M>
> M> > M> -M
> M> > M>
> M> > M>
> M> > M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> M> > M> > M> Talk to the gcc devs. The warning is useful even if there are 
> false positives.
> M> > M> > M>
> M> > M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
> <gleb...@freebsd.org> wrote:
> M> > M> > M> >   Hi,
> M> > M> > M> >
> M> > M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> > M> > M> > M> Author: mmacy
> M> > M> > M> > M> Date: Sat May 19 05:10:51 2018
> M> > M> > M> > M> New Revision: 333860
> M> > M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M> > M> > M> > M>
> M> > M> > M> > M> Log:
> M> > M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
> actually initialized
> M> > M> > M> > M>
> M> > M> > M> > M> Modified:
> M> > M> > M> > M>   head/sys/kern/kern_sendfile.c
> M> > M> > M> > M>
> M> > M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
> M> > M> > M> > M> 
> ==
> M> > M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 
> 2018(r333859)
> M> > M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 
> 2018(r333860)
> M> > M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
> sf_io *sfio, o
> M> > M> > M> > M>  }
> M> > M> > M> > M>
> M> > M> > M> > M>  for (int i = 0; i < npages;) {
> M> > M> > M> > M> -int j, a, count, rv;
> M> > M> > M> > M> +int j, a, count, rv __unused;
> M> > M> > M> > M>
> M> > M> > M> > M>  /* Skip valid pages. */
> M> > M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
> PAGE_MASK,
> M> > M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
> M> > M> > M> > M>  if (space == 0) {
> M> > M> > M> > M>  sfio = NULL;
> M> > M> > M> > M>  nios = 0;
> M> > M> > M> > M> +npages = 0;
> M> > M> > M> > M>  goto prepend_header;
> M> > M> > M> > M>  }
> M> > M> > M> > M>  hdr_uio = NULL;
> M> > M> > M> >
> M> > M> > M> > This initialization is redundant and a compiler warning if 
> exists is wrong.
> M> > M> > M> >
> M> > M> > M> > If we jump down to prepend_header with nios == 0, we won't 
> ever use npages.
> M> > M> > M> >
> M> > M> > M> > --
> M> > M> > M> > Gleb Smirnoff
> M> > M> >
> M> > M> > --
> M> > M> > Gleb Smirnoff
> M> >
> M> > --
> M> > Gleb Smirnoff
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r334104 - in head/sys: netinet sys

[Matthew Macy]

On Wed, May 23, 2018 at 11:52 AM, John Baldwin  wrote:
> On Wednesday, May 23, 2018 05:00:05 PM Matt Macy wrote:
>> Author: mmacy
>> Date: Wed May 23 17:00:05 2018
>> New Revision: 334104
>> URL: https://svnweb.freebsd.org/changeset/base/334104
>>
>> Log:
>>   epoch: allow for conditionally asserting that the epoch context fields
>>   are unused by zeroing on INVARIANTS builds
>
> Is M_ZERO really so bad that you need to make it conditional?

In this case not at all. It's only exercised by sysctl handlers. I'm
mostly responding to an inquiry by jtl. However, gratuitous M_ZERO
usage does have a cumulative adverse performance impact.

> I would probably have preferred something like 'M_ZERO_INVARIANTS'
> instead perhaps (or M_ZERO_EPOCH) that only controls M_ZERO and is
> still or'd with M_WAITOK or M_NOWAIT.

Yes. I like that better too. Thanks.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
> The initialization isn't useful.

It silences a gcc warning. So yes it is. It's this exchange which is not useful.

-M


> On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> M> Talk to the gcc devs. The warning is useful even if there are false 
> positives.
> M>
> M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff <gleb...@freebsd.org> wrote:
> M> >   Hi,
> M> >
> M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> > M> Author: mmacy
> M> > M> Date: Sat May 19 05:10:51 2018
> M> > M> New Revision: 333860
> M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M> > M>
> M> > M> Log:
> M> > M>   sendfile: annotate unused value and ensure that npages is actually 
> initialized
> M> > M>
> M> > M> Modified:
> M> > M>   head/sys/kern/kern_sendfile.c
> M> > M>
> M> > M> Modified: head/sys/kern/kern_sendfile.c
> M> > M> 
> ==
> M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
> (r333859)
> M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
> (r333860)
> M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io 
> *sfio, o
> M> > M>  }
> M> > M>
> M> > M>  for (int i = 0; i < npages;) {
> M> > M> -int j, a, count, rv;
> M> > M> +int j, a, count, rv __unused;
> M> > M>
> M> > M>  /* Skip valid pages. */
> M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
> M> > M> @@ -688,6 +688,7 @@ retry_space:
> M> > M>  if (space == 0) {
> M> > M>  sfio = NULL;
> M> > M>  nios = 0;
> M> > M> +npages = 0;
> M> > M>  goto prepend_header;
> M> > M>  }
> M> > M>  hdr_uio = NULL;
> M> >
> M> > This initialization is redundant and a compiler warning if exists is 
> wrong.
> M> >
> M> > If we jump down to prepend_header with nios == 0, we won't ever use 
> npages.
> M> >
> M> > --
> M> > Gleb Smirnoff
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Talk to the gcc devs. The warning is useful even if there are false positives.

On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff  wrote:
>   Hi,
>
> On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> Author: mmacy
> M> Date: Sat May 19 05:10:51 2018
> M> New Revision: 333860
> M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M>
> M> Log:
> M>   sendfile: annotate unused value and ensure that npages is actually 
> initialized
> M>
> M> Modified:
> M>   head/sys/kern/kern_sendfile.c
> M>
> M> Modified: head/sys/kern/kern_sendfile.c
> M> 
> ==
> M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
> (r333859)
> M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
> (r333860)
> M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, o
> M>  }
> M>
> M>  for (int i = 0; i < npages;) {
> M> -int j, a, count, rv;
> M> +int j, a, count, rv __unused;
> M>
> M>  /* Skip valid pages. */
> M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
> M> @@ -688,6 +688,7 @@ retry_space:
> M>  if (space == 0) {
> M>  sfio = NULL;
> M>  nios = 0;
> M> +npages = 0;
> M>  goto prepend_header;
> M>  }
> M>  hdr_uio = NULL;
>
> This initialization is redundant and a compiler warning if exists is wrong.
>
> If we jump down to prepend_header with nios == 0, we won't ever use npages.
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Thanks.

On Wed, May 23, 2018 at 11:40 AM, Mark Linimon <lini...@lonesome.com> wrote:
> On Wed, May 23, 2018 at 09:56:00AM -0700, Matthew Macy wrote:
>> Thanks updated for 1200064. Someone(tm) needs to do 1200063.
>
> done.
>
> mcl
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Wed, May 23, 2018 at 9:43 AM, John Baldwin  wrote:
> On Wednesday, May 23, 2018 06:15:55 AM Matt Macy wrote:
>> Author: mmacy
>> Date: Wed May 23 06:15:55 2018
>> New Revision: 334074
>> URL: https://svnweb.freebsd.org/changeset/base/334074
>>
>> Log:
>>   Bump FreeBSD_version after r333813
>>
>> Modified:
>>   head/sys/sys/param.h
>
> FYI, these values are documented in
> head/en_US.ISO8859-1/books/porters-handbook/versions/chapter.xml in the doc
> repository.


Thanks updated for 1200064. Someone(tm) needs to do 1200063.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Track

On Wed, May 23, 2018 at 06:53 Ian Lepore  wrote:

> On Wed, 2018-05-23 at 06:15 +, Matt Macy wrote:
> > Author: mmacy
> > Date: Wed May 23 06:15:55 2018
> > New Revision: 334074
> > URL: https://svnweb.freebsd.org/changeset/base/334074
> >
> > Log:
> >   Bump FreeBSD_version after r333813
> >
> > Modified:
> >   head/sys/sys/param.h
> >
> > Modified: head/sys/sys/param.h
> > 
> ==
> > --- head/sys/sys/param.h  Wed May 23 04:09:01 2018(r334073)
> > +++ head/sys/sys/param.h  Wed May 23 06:15:55 2018(r334074)
> > @@ -60,7 +60,7 @@
> >   *   in the range 5 to 9.
> >   */
> >  #undef __FreeBSD_version
> > -#define __FreeBSD_version 1200063/* Master, propagated to newvers */
> > +#define __FreeBSD_version 1200064/* Master, propagated to newvers */
> >
> >  /*
> >   * __FreeBSD_kernel__ indicates that this system uses the kernel of
> FreeBSD,
> >
>
> Nothing about this commit or the commit message for r333813 says why a
> version bump would be warranted. New user-visible features?  API
> change? The diff for r333813 doesn't appear to contain either of those.
>

Out of tree drivers need to conditionally compile for the change of the
multicast address list in ifnet from TAILQ to CK_STAILQ




-- Ian
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333968 - in head/sys: netinet netinet6

[Matthew Macy]

On Tue, May 22, 2018 at 00:33 Eitan Adler  wrote:

> On 21 May 2018 at 01:34, Matt Macy  wrote:
> > Author: mmacy
> > Date: Mon May 21 08:34:10 2018
> > New Revision: 333968
> > URL: https://svnweb.freebsd.org/changeset/base/333968
> >
> > Log:
> >   in(6)_mcast: Expand out vnet set / restore macro so that they work in
> a conditional block
>
> Why not simply add missing {} s or modifying the macro to use "do { }
> while(false)"  ?
>

It declares variables which would go out of scope.


> --
> Eitan Adler
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333911 - head/sys/netinet

[Matthew Macy]

On Mon, May 21, 2018 at 6:15 AM, Jonathan Looney  wrote:
> On Sat, May 19, 2018 at 10:27 PM, Matt Macy  wrote:
>>
>> +   il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb
>> *), M_TEMP, M_WAITOK|M_ZERO);
>> +   inp_list = il->il_inp_list;
>
>
> Why does this need M_ZERO?

It allows me to assert that it hasn't already been passed to
epoch_call. However, it doesn't make sense to pass it on
non-INVARIANTS builds.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Bear in mind that prior to my change most functions would call it
without ever using it on non-debug builds.

On Mon, May 21, 2018 at 9:54 AM, Eric van Gyzen  wrote:
> On 05/19/2018 00:09, Matt Macy wrote:
>> @@ -1663,16 +1655,18 @@ static int
>>  umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi, uint32_t owner,
>>  const char *wmesg, struct abs_timeout *timo, bool shared)
>>  {
>> - struct umtxq_chain *uc;
>>   struct thread *td, *td1;
>>   struct umtx_q *uq1;
>>   int error, pri;
>> +#ifdef INVARIANTS
>> + struct umtxq_chain *uc;
>>
>> + uc = umtxq_getchain(>pi_key);
>> +#endif
>>   error = 0;
>>   td = uq->uq_thread;
>>   KASSERT(td == curthread, ("inconsistent uq_thread"));
>> - uc = umtxq_getchain(>uq_key);
>> - UMTXQ_LOCKED_ASSERT(uc);
>> + UMTXQ_LOCKED_ASSERT(umtxq_getchain(>uq_key));
>
> Couldn't this line stay as it was?
>
> UMTXQ_LOCKED_ASSERT(uc);
>
> With the current code, we're calling umtxq_getchain() once more than
> necessary.  Also, the casual reader might be confused by calling it with
> two different arguments.
>
> Eric
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

Good point. Will fix.

On Mon, May 21, 2018 at 9:54 AM, Eric van Gyzen  wrote:
> On 05/19/2018 00:09, Matt Macy wrote:
>> @@ -1663,16 +1655,18 @@ static int
>>  umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi, uint32_t owner,
>>  const char *wmesg, struct abs_timeout *timo, bool shared)
>>  {
>> - struct umtxq_chain *uc;
>>   struct thread *td, *td1;
>>   struct umtx_q *uq1;
>>   int error, pri;
>> +#ifdef INVARIANTS
>> + struct umtxq_chain *uc;
>>
>> + uc = umtxq_getchain(>pi_key);
>> +#endif
>>   error = 0;
>>   td = uq->uq_thread;
>>   KASSERT(td == curthread, ("inconsistent uq_thread"));
>> - uc = umtxq_getchain(>uq_key);
>> - UMTXQ_LOCKED_ASSERT(uc);
>> + UMTXQ_LOCKED_ASSERT(umtxq_getchain(>uq_key));
>
> Couldn't this line stay as it was?
>
> UMTXQ_LOCKED_ASSERT(uc);
>
> With the current code, we're calling umtxq_getchain() once more than
> necessary.  Also, the casual reader might be confused by calling it with
> two different arguments.
>
> Eric
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333968 - in head/sys: netinet netinet6

[Matthew Macy]

Looking closer I think the ifp should always be set. I'll just add an
assert to that effect and make it non-conditional.
-M

On Mon, May 21, 2018 at 1:55 AM, Marko Zec  wrote:
> On Mon, 21 May 2018 08:34:10 +
> Matt Macy  wrote:
>
>> Author: mmacy
>> Date: Mon May 21 08:34:10 2018
>> New Revision: 333968
>> URL: https://svnweb.freebsd.org/changeset/base/333968
>>
>> Log:
>>   in(6)_mcast: Expand out vnet set / restore macro so that they work
>> in a conditional block
>>   Reported by:zec at fer.hr
>>
>> Modified:
>>   head/sys/netinet/in_mcast.c
>>   head/sys/netinet6/in6_mcast.c
>>
>> Modified: head/sys/netinet/in_mcast.c
>> ==
>> --- head/sys/netinet/in_mcast.c   Mon May 21 07:12:06
>> 2018  (r333967) +++ head/sys/netinet/in_mcast.c   Mon May
>> 21 08:34:10 2018  (r333968) @@ -653,6 +653,7 @@
>> inm_release(struct in_multi *inm) {
>>   struct ifmultiaddr *ifma;
>>   struct ifnet *ifp;
>> + struct vnet *saved_vnet;
>>
>>   CTR2(KTR_IGMPV3, "%s: refcount is %d", __func__,
>> inm->inm_refcount); MPASS(inm->inm_refcount == 0);
>> @@ -663,14 +664,16 @@ inm_release(struct in_multi *inm)
>>
>>   /* XXX this access is not covered by IF_ADDR_LOCK */
>>   CTR2(KTR_IGMPV3, "%s: purging ifma %p", __func__, ifma);
>> - if (ifp)
>> - CURVNET_SET(ifp->if_vnet);
>> + if (ifp) {
>> + saved_vnet = curvnet;
>> + curvnet = ifp->if_vnet;
>> + }
>
> Uhmm...  please don't do this, for at least two reasons:
>
> 1) so far we could identify all VNET context switches by tracking
> CURVNET_SET / _RESTORE macros.  With this change a non-standard hack is
> introduced, opening the door for more alternative / creative variations
> to come, thus increasing the entropy
>
> 2) CURVNET_* macros provide elementary capability of tracking vnet
> recursions, and much more importantly, forgotten or missed context
> restores.  Your change breaks this tracking chain.
>
> Is there a reason one could not extend struct in_multi with a struct
> vnet *inm_vnet entry, populate it on struct in_multi creation, and be
> done with all curvnet woes in a clean way?
>
> Marko
>
>
>>   inm_purge(inm);
>>   free(inm, M_IPMADDR);
>>
>>   if_delmulti_ifma_flags(ifma, 1);
>>   if (ifp) {
>> - CURVNET_RESTORE();
>> + curvnet = saved_vnet;
>>   if_rele(ifp);
>>   }
>>  }
>> @@ -1666,6 +1669,7 @@ inp_gcmoptions(epoch_context_t ctx)
>>   struct in_mfilter   *imf;
>>   struct in_multi *inm;
>>   struct ifnet *ifp;
>> + struct vnet *saved_vnet;
>>   size_t   idx, nmships;
>>
>>   imo =  __containerof(ctx, struct ip_moptions, imo_epoch_ctx);
>> @@ -1677,11 +1681,13 @@ inp_gcmoptions(epoch_context_t ctx)
>>   imf_leave(imf);
>>   inm = imo->imo_membership[idx];
>>   ifp = inm->inm_ifp;
>> - if (ifp)
>> - CURVNET_SET(ifp->if_vnet);
>> + if (ifp) {
>> + saved_vnet = curvnet;
>> + curvnet = ifp->if_vnet;
>> + }
>>   (void)in_leavegroup(inm, imf);
>>   if (ifp)
>> - CURVNET_RESTORE();
>> + curvnet = saved_vnet;
>>   if (imf)
>>   imf_purge(imf);
>>   }
>>
>> Modified: head/sys/netinet6/in6_mcast.c
>> ==
>> --- head/sys/netinet6/in6_mcast.c Mon May 21 07:12:06
>> 2018  (r333967) +++ head/sys/netinet6/in6_mcast.c Mon
>> May 21 08:34:10 2018  (r333968) @@ -524,6 +524,7 @@
>> in6m_release(struct in6_multi *inm) {
>>   struct ifmultiaddr *ifma;
>>   struct ifnet *ifp;
>> + struct vnet *saved_vnet;
>>
>>   CTR2(KTR_MLD, "%s: refcount is %d", __func__,
>> inm->in6m_refcount);
>> @@ -539,14 +540,16 @@ in6m_release(struct in6_multi *inm)
>>   KASSERT(ifma->ifma_protospec == NULL,
>>   ("%s: ifma_protospec != NULL", __func__));
>>
>> - if (ifp)
>> - CURVNET_SET(ifp->if_vnet);
>> + if (ifp) {
>> + saved_vnet = curvnet;
>> + curvnet = ifp->if_vnet;
>> + }
>>   in6m_purge(inm);
>>   free(inm, M_IP6MADDR);
>>
>>   if_delmulti_ifma_flags(ifma, 1);
>>   if (ifp) {
>> - CURVNET_RESTORE();
>> + curvnet = saved_vnet;
>>   if_rele(ifp);
>>   }
>>  }
>> @@ -1628,6 +1631,7 @@ inp_gcmoptions(epoch_context_t ctx)
>>   struct in6_mfilter  *imf;
>>   struct in6_multi *inm;
>>   struct ifnet *ifp;
>> + struct vnet *saved_vnet;
>>   size_t   idx, nmships;
>>
>>   imo =  __containerof(ctx, struct ip6_moptions,
>> imo6_epoch_ctx); @@ -1639,11 +1643,13 @@
>> inp_gcmoptions(epoch_context_t ctx) im6f_leave(imf);
>> 

Re: svn commit: r333968 - in head/sys: netinet netinet6

[Matthew Macy]

Sorry about that

On Mon, May 21, 2018 at 04:59 Hans Petter Selasky  wrote:

> On 05/21/18 13:49, Ed Maste wrote:
> > After r333968 the build is also broken on all archs but amd64 and i386.
>
> It looks like amd64 and i386 build with VIMAGE enabled by default, while
> the others not. 12-current. So both VIMAGE and non-VIMAGE should be tested.
>
> --HPS
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333967 - head/sys/netinet

[Matthew Macy]

On Mon, May 21, 2018 at 00:47 Marko Zec  wrote:

> On Mon, 21 May 2018 07:12:06 +
> Matt Macy  wrote:
>
> > Author: mmacy
> > Date: Mon May 21 07:12:06 2018
> > New Revision: 333967
> > URL: https://svnweb.freebsd.org/changeset/base/333967
> >
> > Log:
> >   ensure that vnet is set when doing in_leavegroup
> >
> > Modified:
> >   head/sys/netinet/in_mcast.c
> >
> > Modified: head/sys/netinet/in_mcast.c
> >
> ==
> > --- head/sys/netinet/in_mcast.c   Mon May 21 05:20:23
> > 2018  (r333966) +++ head/sys/netinet/in_mcast.c   Mon May
> > 21 07:12:06 2018  (r333967) @@ -1664,6 +1664,8 @@
> > inp_gcmoptions(epoch_context_t ctx) {
> >   struct ip_moptions *imo;
> >   struct in_mfilter   *imf;
> > + struct in_multi *inm;
> > + struct ifnet *ifp;
> >   size_t   idx, nmships;
> >
> >   imo =  __containerof(ctx, struct ip_moptions, imo_epoch_ctx);
> > @@ -1673,7 +1675,13 @@ inp_gcmoptions(epoch_context_t ctx)
> >   imf = imo->imo_mfilters ? >imo_mfilters[idx] :
> > NULL; if (imf)
> >   imf_leave(imf);
> > - (void)in_leavegroup(imo->imo_membership[idx], imf);
> > + inm = imo->imo_membership[idx];
> > + ifp = inm->inm_ifp;
> > + if (ifp)
> > + CURVNET_SET(ifp->if_vnet);
>
> Unfortunately, this won't work because CURVNET_SET() expands to a
> sequence of declarations and assignments which are NOT enclosed in a
> single block.  Instead, only the first statement in CURVNET_SET()
> sequence, which is an assert, will be executed conditionally only if
> ifp != NULL, while the rest of the CURVNET_SET() body will fall out of
> the scope of the if (ifp) conditional.
>

That's pretty counter to the way macros are done _everywhere_ else in the
kernel. You should probably fix that.


> I'd recommend backing out this patch, and instead extending the struct
> ip_moptions with an struct vnet * entry, which would be populated
> before scheduling inp_gcmoptions().  Then CURVNET_SET(imo->imo_vnet)
> could be called only once (and unconditionally) in gcmoptions(),
> instead of (attempts at) doing this multiple times in a for loop.
>
> Marko
>
>
> > + (void)in_leavegroup(inm, imf);
> > + if (ifp)
> > + CURVNET_RESTORE();
> >   if (imf)
> >   imf_purge(imf);
> >   }
> >
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333967 - head/sys/netinet

[Matthew Macy]

Thanks

On Mon, May 21, 2018 at 00:47 Marko Zec  wrote:

> On Mon, 21 May 2018 07:12:06 +
> Matt Macy  wrote:
>
> > Author: mmacy
> > Date: Mon May 21 07:12:06 2018
> > New Revision: 333967
> > URL: https://svnweb.freebsd.org/changeset/base/333967
> >
> > Log:
> >   ensure that vnet is set when doing in_leavegroup
> >
> > Modified:
> >   head/sys/netinet/in_mcast.c
> >
> > Modified: head/sys/netinet/in_mcast.c
> >
> ==
> > --- head/sys/netinet/in_mcast.c   Mon May 21 05:20:23
> > 2018  (r333966) +++ head/sys/netinet/in_mcast.c   Mon May
> > 21 07:12:06 2018  (r333967) @@ -1664,6 +1664,8 @@
> > inp_gcmoptions(epoch_context_t ctx) {
> >   struct ip_moptions *imo;
> >   struct in_mfilter   *imf;
> > + struct in_multi *inm;
> > + struct ifnet *ifp;
> >   size_t   idx, nmships;
> >
> >   imo =  __containerof(ctx, struct ip_moptions, imo_epoch_ctx);
> > @@ -1673,7 +1675,13 @@ inp_gcmoptions(epoch_context_t ctx)
> >   imf = imo->imo_mfilters ? >imo_mfilters[idx] :
> > NULL; if (imf)
> >   imf_leave(imf);
> > - (void)in_leavegroup(imo->imo_membership[idx], imf);
> > + inm = imo->imo_membership[idx];
> > + ifp = inm->inm_ifp;
> > + if (ifp)
> > + CURVNET_SET(ifp->if_vnet);
>
> Unfortunately, this won't work because CURVNET_SET() expands to a
> sequence of declarations and assignments which are NOT enclosed in a
> single block.  Instead, only the first statement in CURVNET_SET()
> sequence, which is an assert, will be executed conditionally only if
> ifp != NULL, while the rest of the CURVNET_SET() body will fall out of
> the scope of the if (ifp) conditional.
>
> I'd recommend backing out this patch, and instead extending the struct
> ip_moptions with an struct vnet * entry, which would be populated
> before scheduling inp_gcmoptions().  Then CURVNET_SET(imo->imo_vnet)
> could be called only once (and unconditionally) in gcmoptions(),
> instead of (attempts at) doing this multiple times in a for loop.
>
> Marko
>
>
> > + (void)in_leavegroup(inm, imf);
> > + if (ifp)
> > + CURVNET_RESTORE();
> >   if (imf)
> >   imf_purge(imf);
> >   }
> >
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333924 - head/sys/fs/nfsclient

[Matthew Macy]

I don't understand why you can't install the gcc8 package but here you go:
https://people.freebsd.org/~mmacy/gcc8logs/nfs-GENERIC-NODEBUG.log
https://people.freebsd.org/~mmacy/gcc8logs/nfs-GENERIC.log


On Sun, May 20, 2018 at 5:17 PM, Rick Macklem <rmack...@uoguelph.ca> wrote:
> I'm not in a situation where I can use gcc, but if you email me the
> list of warnings, I can look at them.
>
> rick
>
> ________
> From: Matthew Macy <mm...@freebsd.org>
> Sent: Sunday, May 20, 2018 7:16:31 PM
> To: Rick Macklem
> Cc: src-committ...@freebsd.org; svn-src-...@freebsd.org; 
> svn-src-head@freebsd.org
> Subject: Re: svn commit: r333924 - head/sys/fs/nfsclient
>
> Actually if you could clean up the NFS code that would be great. It's
> _full_ of set but not used warnings. And anyone else reading this if
> you could clean up other areas. I've mostly cleaned up kern / net /
> netinet / iflib drivers.
>
>
> both
> WITHOUT_FORMAT_EXTENSIONS=  XCC=/usr/local/bin/gcc8 make -j buildkernel
>
> and
> WITHOUT_FORMAT_EXTENSIONS=  XCC=/usr/local/bin/gcc8 make -j
> buildkernel KERNCONF=GENERIC-NODEBUG
>
> Thanks.
>
> On Sun, May 20, 2018 at 4:09 PM, Matthew Macy <mm...@freebsd.org> wrote:
>> gcc8
>>
>> On Sun, May 20, 2018 at 15:14 Rick Macklem <rmack...@uoguelph.ca> wrote:
>>>
>>> Matt Macy wrote:
>>> >Author: mmacy
>>> >Date: Sun May 20 06:14:12 2018
>>> >New Revision: 333924
>>> >URL: https://svnweb.freebsd.org/changeset/base/333924
>>> >
>>> >Log:
>>> >  nfsclient: warnings cleanups
>>> Just wondering what compiler you are using. I haven't seen warnings for
>>> these?
>>> FYI, for the first two cases, ncookie is always set before it is used.
>>> For the third, "rflags" is set but not used. I sometimes leave code like
>>> this in
>>> the tree since I might need those argument flags later and might not
>>> remember
>>> how to get them. In this case, similar code in other functions set
>>> "rflags" in the
>>> same way, so it shouldn't be hard to crib the assignment from there.
>>> As such, I don't have a problem with deleting the code.
>>>
>>> rick
>>> ... the commit patch ...
>>> Modified:
>>>   head/sys/fs/nfsclient/nfs_clcomsubs.c
>>>   head/sys/fs/nfsclient/nfs_clrpcops.c
>>>
>>> Modified: head/sys/fs/nfsclient/nfs_clcomsubs.c
>>>
>>> ==
>>> --- head/sys/fs/nfsclient/nfs_clcomsubs.c   Sun May 20 05:59:42 2018
>>> (r333923)
>>> +++ head/sys/fs/nfsclient/nfs_clcomsubs.c   Sun May 20 06:14:12 2018
>>> (r333924)
>>> @@ -354,7 +354,7 @@ nfsm_uiombuflist(struct uio *uiop, int siz, struct mbu
>>> char *uiocp;
>>> struct mbuf *mp, *mp2, *firstmp;
>>> int xfer, left, mlen;
>>> -   int uiosiz, clflg, rem;
>>> +   int uiosiz, clflg;
>>> char *tcp;
>>>
>>> KASSERT(uiop->uio_iovcnt == 1, ("nfsm_uiotombuf: iovcnt != 1"));
>>> @@ -363,7 +363,6 @@ nfsm_uiombuflist(struct uio *uiop, int siz, struct mbu
>>> clflg = 1;
>>> else
>>> clflg = 0;
>>> -   rem = NFSM_RNDUP(siz) - siz;
>>> if (clflg != 0)
>>> NFSMCLGET(mp, M_WAITOK);
>>> else
>>>
>>> Modified: head/sys/fs/nfsclient/nfs_clrpcops.c
>>>
>>> ==
>>> --- head/sys/fs/nfsclient/nfs_clrpcops.cSun May 20 05:59:42 2018
>>> (r333923)
>>> +++ head/sys/fs/nfsclient/nfs_clrpcops.cSun May 20 06:14:12 2018
>>> (r333924)
>>> @@ -2845,7 +2845,7 @@ nfsrpc_readdir(vnode_t vp, struct uio *uiop,
>>> nfsuint64
>>> KASSERT(uiop->uio_iovcnt == 1 &&
>>> (uio_uio_resid(uiop) & (DIRBLKSIZ - 1)) == 0,
>>> ("nfs readdirrpc bad uio"));
>>> -
>>> +   ncookie.lval[0] = ncookie.lval[1] = 0;
>>> /*
>>>  * There is no point in reading a lot more than uio_resid, however
>>>  * adding one additional DIRBLKSIZ makes sense. Since uio_resid
>>> @@ -3288,6 +3288,7 @@ nfsrpc_readdirplus(vnode_t vp, struct uio *uiop,
>>> nfsui
>>> KASSERT(uiop->uio_iovcnt == 1 &&
>>> 

Re: svn commit: r333924 - head/sys/fs/nfsclient

[Matthew Macy]

Actually if you could clean up the NFS code that would be great. It's
_full_ of set but not used warnings. And anyone else reading this if
you could clean up other areas. I've mostly cleaned up kern / net /
netinet / iflib drivers.


both
WITHOUT_FORMAT_EXTENSIONS=  XCC=/usr/local/bin/gcc8 make -j buildkernel

and
WITHOUT_FORMAT_EXTENSIONS=  XCC=/usr/local/bin/gcc8 make -j
buildkernel KERNCONF=GENERIC-NODEBUG

Thanks.

On Sun, May 20, 2018 at 4:09 PM, Matthew Macy <mm...@freebsd.org> wrote:
> gcc8
>
> On Sun, May 20, 2018 at 15:14 Rick Macklem <rmack...@uoguelph.ca> wrote:
>>
>> Matt Macy wrote:
>> >Author: mmacy
>> >Date: Sun May 20 06:14:12 2018
>> >New Revision: 333924
>> >URL: https://svnweb.freebsd.org/changeset/base/333924
>> >
>> >Log:
>> >  nfsclient: warnings cleanups
>> Just wondering what compiler you are using. I haven't seen warnings for
>> these?
>> FYI, for the first two cases, ncookie is always set before it is used.
>> For the third, "rflags" is set but not used. I sometimes leave code like
>> this in
>> the tree since I might need those argument flags later and might not
>> remember
>> how to get them. In this case, similar code in other functions set
>> "rflags" in the
>> same way, so it shouldn't be hard to crib the assignment from there.
>> As such, I don't have a problem with deleting the code.
>>
>> rick
>> ... the commit patch ...
>> Modified:
>>   head/sys/fs/nfsclient/nfs_clcomsubs.c
>>   head/sys/fs/nfsclient/nfs_clrpcops.c
>>
>> Modified: head/sys/fs/nfsclient/nfs_clcomsubs.c
>>
>> ==
>> --- head/sys/fs/nfsclient/nfs_clcomsubs.c   Sun May 20 05:59:42 2018
>> (r333923)
>> +++ head/sys/fs/nfsclient/nfs_clcomsubs.c   Sun May 20 06:14:12 2018
>> (r333924)
>> @@ -354,7 +354,7 @@ nfsm_uiombuflist(struct uio *uiop, int siz, struct mbu
>> char *uiocp;
>> struct mbuf *mp, *mp2, *firstmp;
>> int xfer, left, mlen;
>> -   int uiosiz, clflg, rem;
>> +   int uiosiz, clflg;
>> char *tcp;
>>
>> KASSERT(uiop->uio_iovcnt == 1, ("nfsm_uiotombuf: iovcnt != 1"));
>> @@ -363,7 +363,6 @@ nfsm_uiombuflist(struct uio *uiop, int siz, struct mbu
>> clflg = 1;
>> else
>> clflg = 0;
>> -   rem = NFSM_RNDUP(siz) - siz;
>> if (clflg != 0)
>> NFSMCLGET(mp, M_WAITOK);
>> else
>>
>> Modified: head/sys/fs/nfsclient/nfs_clrpcops.c
>>
>> ==
>> --- head/sys/fs/nfsclient/nfs_clrpcops.cSun May 20 05:59:42 2018
>> (r333923)
>> +++ head/sys/fs/nfsclient/nfs_clrpcops.cSun May 20 06:14:12 2018
>> (r333924)
>> @@ -2845,7 +2845,7 @@ nfsrpc_readdir(vnode_t vp, struct uio *uiop,
>> nfsuint64
>> KASSERT(uiop->uio_iovcnt == 1 &&
>> (uio_uio_resid(uiop) & (DIRBLKSIZ - 1)) == 0,
>> ("nfs readdirrpc bad uio"));
>> -
>> +   ncookie.lval[0] = ncookie.lval[1] = 0;
>> /*
>>  * There is no point in reading a lot more than uio_resid, however
>>  * adding one additional DIRBLKSIZ makes sense. Since uio_resid
>> @@ -3288,6 +3288,7 @@ nfsrpc_readdirplus(vnode_t vp, struct uio *uiop,
>> nfsui
>> KASSERT(uiop->uio_iovcnt == 1 &&
>> (uio_uio_resid(uiop) & (DIRBLKSIZ - 1)) == 0,
>> ("nfs readdirplusrpc bad uio"));
>> +   ncookie.lval[0] = ncookie.lval[1] = 0;
>> timespecclear();
>> *attrflagp = 0;
>> if (eofp != NULL)
>> @@ -6943,6 +6944,7 @@ nfsrv_parseug(struct nfsrv_descript *nd, int dogrp,
>> ui
>>
>> NFSM_DISSECT(tl, uint32_t *, NFSX_UNSIGNED);
>> len = fxdr_unsigned(uint32_t, *tl);
>> +   str = NULL;
>> if (len > NFSV4_OPAQUELIMIT) {
>> error = NFSERR_BADXDR;
>> goto nfsmout;
>> @@ -7244,7 +7246,6 @@ nfsrpc_createlayout(vnode_t dvp, char *name, int
>> namel
>> struct nfsclsession *tsep;
>> nfsattrbit_t attrbits;
>> nfsv4stateid_t stateid;
>> -   uint32_t rflags;
>> struct nfsmount *nmp;
>>
>> nmp = VFSTONFS(dvp->v_mount);
>> @@ -7327,7 +7328,6 @@ nfsrpc_createlayout(vnode_t dvp, char *name, int
>> namel
>> stateid.other[0] = *tl++;
>> stateid.other[1] = *tl++;
>> stateid.other[2] = *tl;
>> -   rflags = fxdr_unsigned(u_int32_t, *(tl + 6));
>> nfsrv_getattrbits(nd, , NULL, NULL);
>> NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED);
>> deleg = fxdr_unsigned(int, *tl);
>>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333924 - head/sys/fs/nfsclient

[Matthew Macy]

gcc8

On Sun, May 20, 2018 at 15:14 Rick Macklem  wrote:

> Matt Macy wrote:
> >Author: mmacy
> >Date: Sun May 20 06:14:12 2018
> >New Revision: 333924
> >URL: https://svnweb.freebsd.org/changeset/base/333924
> >
> >Log:
> >  nfsclient: warnings cleanups
> Just wondering what compiler you are using. I haven't seen warnings for
> these?
> FYI, for the first two cases, ncookie is always set before it is used.
> For the third, "rflags" is set but not used. I sometimes leave code like
> this in
> the tree since I might need those argument flags later and might not
> remember
> how to get them. In this case, similar code in other functions set
> "rflags" in the
> same way, so it shouldn't be hard to crib the assignment from there.
> As such, I don't have a problem with deleting the code.
>
> rick
> ... the commit patch ...
> Modified:
>   head/sys/fs/nfsclient/nfs_clcomsubs.c
>   head/sys/fs/nfsclient/nfs_clrpcops.c
>
> Modified: head/sys/fs/nfsclient/nfs_clcomsubs.c
>
> ==
> --- head/sys/fs/nfsclient/nfs_clcomsubs.c   Sun May 20 05:59:42 2018
>   (r333923)
> +++ head/sys/fs/nfsclient/nfs_clcomsubs.c   Sun May 20 06:14:12 2018
>   (r333924)
> @@ -354,7 +354,7 @@ nfsm_uiombuflist(struct uio *uiop, int siz, struct mbu
> char *uiocp;
> struct mbuf *mp, *mp2, *firstmp;
> int xfer, left, mlen;
> -   int uiosiz, clflg, rem;
> +   int uiosiz, clflg;
> char *tcp;
>
> KASSERT(uiop->uio_iovcnt == 1, ("nfsm_uiotombuf: iovcnt != 1"));
> @@ -363,7 +363,6 @@ nfsm_uiombuflist(struct uio *uiop, int siz, struct mbu
> clflg = 1;
> else
> clflg = 0;
> -   rem = NFSM_RNDUP(siz) - siz;
> if (clflg != 0)
> NFSMCLGET(mp, M_WAITOK);
> else
>
> Modified: head/sys/fs/nfsclient/nfs_clrpcops.c
>
> ==
> --- head/sys/fs/nfsclient/nfs_clrpcops.cSun May 20 05:59:42 2018
>   (r333923)
> +++ head/sys/fs/nfsclient/nfs_clrpcops.cSun May 20 06:14:12 2018
>   (r333924)
> @@ -2845,7 +2845,7 @@ nfsrpc_readdir(vnode_t vp, struct uio *uiop,
> nfsuint64
> KASSERT(uiop->uio_iovcnt == 1 &&
> (uio_uio_resid(uiop) & (DIRBLKSIZ - 1)) == 0,
> ("nfs readdirrpc bad uio"));
> -
> +   ncookie.lval[0] = ncookie.lval[1] = 0;
> /*
>  * There is no point in reading a lot more than uio_resid, however
>  * adding one additional DIRBLKSIZ makes sense. Since uio_resid
> @@ -3288,6 +3288,7 @@ nfsrpc_readdirplus(vnode_t vp, struct uio *uiop,
> nfsui
> KASSERT(uiop->uio_iovcnt == 1 &&
> (uio_uio_resid(uiop) & (DIRBLKSIZ - 1)) == 0,
> ("nfs readdirplusrpc bad uio"));
> +   ncookie.lval[0] = ncookie.lval[1] = 0;
> timespecclear();
> *attrflagp = 0;
> if (eofp != NULL)
> @@ -6943,6 +6944,7 @@ nfsrv_parseug(struct nfsrv_descript *nd, int dogrp,
> ui
>
> NFSM_DISSECT(tl, uint32_t *, NFSX_UNSIGNED);
> len = fxdr_unsigned(uint32_t, *tl);
> +   str = NULL;
> if (len > NFSV4_OPAQUELIMIT) {
> error = NFSERR_BADXDR;
> goto nfsmout;
> @@ -7244,7 +7246,6 @@ nfsrpc_createlayout(vnode_t dvp, char *name, int
> namel
> struct nfsclsession *tsep;
> nfsattrbit_t attrbits;
> nfsv4stateid_t stateid;
> -   uint32_t rflags;
> struct nfsmount *nmp;
>
> nmp = VFSTONFS(dvp->v_mount);
> @@ -7327,7 +7328,6 @@ nfsrpc_createlayout(vnode_t dvp, char *name, int
> namel
> stateid.other[0] = *tl++;
> stateid.other[1] = *tl++;
> stateid.other[2] = *tl;
> -   rflags = fxdr_unsigned(u_int32_t, *(tl + 6));
> nfsrv_getattrbits(nd, , NULL, NULL);
> NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED);
> deleg = fxdr_unsigned(int, *tl);
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333872 - head/cddl/contrib/opensolaris/tools/ctf/cvt

[Matthew Macy]

On Sat, May 19, 2018 at 4:49 AM, Ed Maste  wrote:
> On 19 May 2018 at 02:31, Matt Macy  wrote:
>> Author: mmacy
>> Date: Sat May 19 06:31:17 2018
>> New Revision: 333872
>> URL: https://svnweb.freebsd.org/changeset/base/333872
>>
>> Log:
>>   ctfconvert: silence useless enum has too many values warning
>
> I agree it's reasonable to silence this warning as it is not
> actionable for almost everyone who encounters it. It does indicate a
> real problem in our tool chain though and I added an entry to
> https://wiki.freebsd.org/DTraceTODO.

Conrad brought up the need to change the CTF ABI on IRC. This will
involve an on-disk format change which he and I believe to be
acceptable, but perhaps interoperability with other operating systems
is still considered important.
-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Sat, May 19, 2018 at 8:56 AM, Emmanuel Vadot <m...@bidouilliste.com> wrote:
> On 2018-05-19 17:39, Matthew Macy wrote:
>>
>> On Sat, May 19, 2018 at 07:17 Emmanuel Vadot <m...@bidouilliste.com>
>> wrote:
>>
>>> On 2018-05-19 15:35, Rodney W. Grimes wrote:
>>>>
>>>> [ Charset UTF-8 unsupported, converting... ]
>>>>>
>>>>> Author: mmacy
>>>>> Date: Sat May 19 02:15:40 2018
>>>>> New Revision: 333822
>>>>> URL: https://svnweb.freebsd.org/changeset/base/333822 [1]
>>>>>
>>>>>
>>>>> Log:
>>>>> fix gcc8 unused variable and set but not used variable in unix
>>>
>>>
>>>>> sockets
>>>>> add copyright from lock rewrite while here
>>>>>
>>>>> Modified:
>>>>> head/sys/kern/uipc_usrreq.c
>>>>>
>>>>> Modified: head/sys/kern/uipc_usrreq.c
>>>>>
>>>
>>
>> ==
>>>>>
>>>>> --- head/sys/kern/uipc_usrreq.c Sat May 19 00:47:24 2018
>>>
>>> (r333821)
>>>>>
>>>>> +++ head/sys/kern/uipc_usrreq.c Sat May 19 02:15:40 2018
>>>
>>> (r333822)
>>>>>
>>>>> @@ -4,7 +4,7 @@
>>>>> * Copyright (c) 1982, 1986, 1989, 1991, 1993
>>>>> * The Regents of the University of California.
>>>>> * Copyright (c) 2004-2009 Robert N. M. Watson
>>>>> - * All rights reserved.
>>>>
>>>>
>>>> Did you have permission from Robert Watson to remove this?
>>>>
>>>>> + * Copyright (c) 2018 Matthew Macy
>>>>> *
>>>>> * Redistribution and use in source and binary forms, with or
>>>
>>> without
>>>>>
>>>>> * modification, are permitted provided that the following
>>>
>>> conditions
>>>>
>>>> ...
>>>
>>>
>>> That also seems a small patch to add your name in the copyright.
>>
>>
>> Uhh... Locking rewrite was a small patch? Really?
>
>
>  Maybe I should I read the previous commits and the full log here :) sorry.

No problem. Thanks Warner for stepping up. I probably shouldn't
respond before I've had my coffee.

Just to be clear - I wasn't upset, just a bit flabbergasted. I clearly
mentioned "locking rewrite" in the message and there were clearly no
locking changes in this particular commit. The awake thing to do would
have been to send a link to the commit itself and clarify that,
although in pure line count was < 25%,  it was a material change with
substantial impact.

https://svnweb.freebsd.org/base?view=revision=333744

Cheers
-M

>
>>
>>> --
>>> Emmanuel Vadot <m...@bidouilliste.com> <m...@freebsd.org>
>>
>>
>>
>> Links:
>> --
>> [1] https://svnweb.freebsd.org/changeset/base/333822
>
>
> --
> Emmanuel Vadot <m...@bidouilliste.com> <m...@freebsd.org>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333819 - in head/sys: conf modules/blake2 modules/crypto modules/drm2/i915kms modules/ipfilter

[Matthew Macy]

As I suspected, we fix warnings on non-contrib code. These are only
used on contrib code.
-M

mmacy@anarchy [~/devel/upstream|9:46|26] find sys | xargs grep NO_WSELF_ASSIGN
sys/modules/ipfilter/Makefile:CWARNFLAGS.fil.c=
${NO_WSELF_ASSIGN} -Wno-unused
sys/modules/ipfilter/Makefile:CWARNFLAGS.ip_lookup.c=
${NO_WSELF_ASSIGN} -Wno-unused
sys/modules/ipfilter/Makefile:CWARNFLAGS.ip_proxy.c=
${NO_WSELF_ASSIGN} -Wno-unused
sys/conf/kern.mk:NO_WSELF_ASSIGN=   -Wno-self-assign
sys/conf/files: compile-with "${NORMAL_C} ${NO_WSELF_ASSIGN}
-Wno-unused -I$S/contrib/ipfilter"
sys/conf/files: compile-with "${NORMAL_C} ${NO_WSELF_ASSIGN}
-Wno-unused -I$S/contrib/ipfilter"
sys/conf/files: compile-with "${NORMAL_C} ${NO_WSELF_ASSIGN}
-Wno-unused -Wno-error -I$S/contrib/ipfilter"
mmacy@anarchy [~/devel/upstream|9:47|27] find sys | xargs grep
NO_WSHIFT_COUNT_NEGATIVE
sys/modules/ath_hal/Makefile:CWARNFLAGS.ah_regdomain.c=
${NO_WSHIFT_COUNT_NEGATIVE} ${NO_WSHIFT_COUNT_OVERFLOW}
sys/conf/kern.mk:NO_WSHIFT_COUNT_NEGATIVE=  -Wno-shift-count-negative
sys/conf/files: compile-with "${NORMAL_C} ${NO_WSHIFT_COUNT_NEGATIVE}
${NO_WSHIFT_COUNT_OVERFLOW} -I$S/dev/ath"
mmacy@anarchy [~/devel/upstream|9:47|28] find sys | xargs grep
NO_WSHIFT_COUNT_OVERFLOW
sys/modules/ath_hal/Makefile:CWARNFLAGS.ah_regdomain.c=
${NO_WSHIFT_COUNT_NEGATIVE} ${NO_WSHIFT_COUNT_OVERFLOW}
sys/conf/kern.mk:NO_WSHIFT_COUNT_OVERFLOW=  -Wno-shift-count-overflow
sys/conf/files: compile-with "${NORMAL_C} ${NO_WSHIFT_COUNT_NEGATIVE}
${NO_WSHIFT_COUNT_OVERFLOW} -I$S/dev/ath"

On Sat, May 19, 2018 at 5:30 AM, Matthew Macy <mm...@freebsd.org> wrote:
> Oops I’ll add a separate define for that
>
> On Sat, May 19, 2018 at 04:27 Ed Maste <ema...@freebsd.org> wrote:
>>
>> On 18 May 2018 at 20:04, Matt Macy <mm...@freebsd.org> wrote:
>> > Author: mmacy
>> > Date: Sat May 19 00:04:01 2018
>> > New Revision: 333819
>> > URL: https://svnweb.freebsd.org/changeset/base/333819
>> >
>> > Log:
>> >   Silence non-actionable warnings in vendor code
>> ...
>> > Modified: head/sys/conf/kern.mk
>> >
>> > ==
>> > --- head/sys/conf/kern.mk   Fri May 18 23:42:08 2018
>> > (r333818)
>> > +++ head/sys/conf/kern.mk   Sat May 19 00:04:01 2018
>> > (r333819)
>> > @@ -18,12 +18,13 @@ CWARNFLAGS?=-Wall -Wredundant-decls
>> > -Wnested-externs
>> >  # a false positive.
>> >  .if ${COMPILER_TYPE} == "clang"
>> >  NO_WCONSTANT_CONVERSION=   -Wno-error-constant-conversion
>> > -NO_WSHIFT_COUNT_NEGATIVE=  -Wno-error-shift-count-negative
>> > -NO_WSHIFT_COUNT_OVERFLOW=  -Wno-error-shift-count-overflow
>> > -NO_WSELF_ASSIGN=   -Wno-error-self-assign
>> > +NO_WSHIFT_COUNT_NEGATIVE=  -Wno-shift-count-negative
>> > +NO_WSHIFT_COUNT_OVERFLOW=  -Wno-shift-count-overflow
>> > +NO_WSELF_ASSIGN=   -Wno-self-assign
>>
>> This silences the warning across the tree, not just vendor code.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Matthew Macy]

On Sat, May 19, 2018 at 07:17 Emmanuel Vadot <m...@bidouilliste.com> wrote:

> On 2018-05-19 15:35, Rodney W. Grimes wrote:
> > [ Charset UTF-8 unsupported, converting... ]
> >> Author: mmacy
> >> Date: Sat May 19 02:15:40 2018
> >> New Revision: 333822
> >> URL: https://svnweb.freebsd.org/changeset/base/333822
> >>
> >> Log:
> >>   fix gcc8 unused variable and set but not used variable in unix
> >> sockets
> >>   add copyright from lock rewrite while here
> >>
> >> Modified:
> >>   head/sys/kern/uipc_usrreq.c
> >>
> >> Modified: head/sys/kern/uipc_usrreq.c
> >>
> ==
> >> --- head/sys/kern/uipc_usrreq.c  Sat May 19 00:47:24 2018
> (r333821)
> >> +++ head/sys/kern/uipc_usrreq.c  Sat May 19 02:15:40 2018
> (r333822)
> >> @@ -4,7 +4,7 @@
> >>   * Copyright (c) 1982, 1986, 1989, 1991, 1993
> >>   *  The Regents of the University of California.
> >>   * Copyright (c) 2004-2009 Robert N. M. Watson
> >> - * All rights reserved.
> >
> > Did you have permission from Robert Watson to remove this?
> >
> >> + * Copyright (c) 2018 Matthew Macy
> >>   *
> >>   * Redistribution and use in source and binary forms, with or without
> >>   * modification, are permitted provided that the following conditions
> > ...
>
>   That also seems a small patch to add your name in the copyright.
>

Uhh... Locking rewrite was a small patch? Really?

>
> --
> Emmanuel Vadot <m...@bidouilliste.com> <m...@freebsd.org>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333819 - in head/sys: conf modules/blake2 modules/crypto modules/drm2/i915kms modules/ipfilter

[Matthew Macy]

Oops I’ll add a separate define for that

On Sat, May 19, 2018 at 04:27 Ed Maste  wrote:

> On 18 May 2018 at 20:04, Matt Macy  wrote:
> > Author: mmacy
> > Date: Sat May 19 00:04:01 2018
> > New Revision: 333819
> > URL: https://svnweb.freebsd.org/changeset/base/333819
> >
> > Log:
> >   Silence non-actionable warnings in vendor code
> ...
> > Modified: head/sys/conf/kern.mk
> >
> ==
> > --- head/sys/conf/kern.mk   Fri May 18 23:42:08 2018
> (r333818)
> > +++ head/sys/conf/kern.mk   Sat May 19 00:04:01 2018
> (r333819)
> > @@ -18,12 +18,13 @@ CWARNFLAGS?=-Wall -Wredundant-decls
> -Wnested-externs
> >  # a false positive.
> >  .if ${COMPILER_TYPE} == "clang"
> >  NO_WCONSTANT_CONVERSION=   -Wno-error-constant-conversion
> > -NO_WSHIFT_COUNT_NEGATIVE=  -Wno-error-shift-count-negative
> > -NO_WSHIFT_COUNT_OVERFLOW=  -Wno-error-shift-count-overflow
> > -NO_WSELF_ASSIGN=   -Wno-error-self-assign
> > +NO_WSHIFT_COUNT_NEGATIVE=  -Wno-shift-count-negative
> > +NO_WSHIFT_COUNT_OVERFLOW=  -Wno-shift-count-overflow
> > +NO_WSELF_ASSIGN=   -Wno-self-assign
>
> This silences the warning across the tree, not just vendor code.
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333425 - in head/sys: cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs compat/cloudabi compat/linux compat/linuxkpi/common/include/linux dev/filemon dev/hwpmc fs

[Matthew Macy]

r333874

On Sat, May 19, 2018 at 12:16 AM, Matthew Macy <mm...@freebsd.org> wrote:
> I can do that tomorrow. But point is that something else will push it over 
> soon.
>
> On Sat, May 19, 2018 at 12:14 AM, Mateusz Guzik <mjgu...@gmail.com> wrote:
>> imo all these sysinits can and shoud be collapsed into one, which will have
>> a side effect of getting rid of the problem.
>>
>> On Sat, May 19, 2018 at 9:13 AM, Matthew Macy <mm...@freebsd.org> wrote:
>>>
>>> I guess we'll need to allocate more pages at boot. We must have been
>>> on the edge already if that pushed us over.
>>> -M
>>>
>>> On Fri, May 18, 2018 at 12:03 PM, Ilya Bakulin <i...@bakulin.de> wrote:
>>> > Hi Matt,
>>> > seems this commit has broken at least BeagleBone Black booting process.
>>> > On
>>> > all revisions after it the kernel panics with this message:
>>> > http://dl.bakulin.de/bbb_panic.txt
>>> >  My suspicion is that there are quite a few new SYSINIT objects that are
>>> > created on startup, and as a result some kind of memory reservation gets
>>> > exhausted. I don't have immediate idea how to debug this further; just
>>> > can
>>> > confirm that patching out this change allows the board to boot again.
>>> >
>>> >
>>> > On Wed, May 9, 2018 at 8:47 PM Matt Macy <mm...@freebsd.org> wrote:
>>> >>
>>> >> Author: mmacy
>>> >> Date: Wed May  9 18:47:24 2018
>>> >> New Revision: 333425
>>> >> URL: https://svnweb.freebsd.org/changeset/base/333425
>>> >>
>>> >> Log:
>>> >>   Eliminate the overhead of gratuitous repeated reinitialization of
>>> >> cap_rights
>>> >>
>>> >>   - Add macros to allow preinitialization of cap_rights_t.
>>> >>
>>> >>   - Convert most commonly used code paths to use preinitialized
>>> >> cap_rights_t.
>>> >> A 3.6% speedup in fstat was measured with this change.
>>> >>
>>> >>   Reported by:  mjg
>>> >>   Reviewed by:  oshogbo
>>> >>   Approved by:  sbruno
>>> >>   MFC after:1 month
>>> >>
>>> >> Modified:
>>> >>   head/sys/cddl/compat/opensolaris/sys/file.h
>>> >>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>>> >>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_onexit.c
>>> >>   head/sys/compat/cloudabi/cloudabi_file.c
>>> >>   head/sys/compat/linux/linux_event.c
>>> >>   head/sys/compat/linux/linux_file.c
>>> >>   head/sys/compat/linux/linux_ioctl.c
>>> >>   head/sys/compat/linux/linux_mmap.c
>>> >>   head/sys/compat/linux/linux_socket.c
>>> >>   head/sys/compat/linux/linux_stats.c
>>> >>   head/sys/compat/linuxkpi/common/include/linux/file.h
>>> >>   head/sys/dev/filemon/filemon.c
>>> >>   head/sys/dev/hwpmc/hwpmc_logging.c
>>> >>   head/sys/fs/fdescfs/fdesc_vnops.c
>>> >>   head/sys/fs/fuse/fuse_vfsops.c
>>> >>   head/sys/kern/kern_descrip.c
>>> >>   head/sys/kern/kern_event.c
>>> >>   head/sys/kern/kern_exec.c
>>> >>   head/sys/kern/kern_sendfile.c
>>> >>   head/sys/kern/kern_sig.c
>>> >>   head/sys/kern/subr_capability.c
>>> >>   head/sys/kern/sys_generic.c
>>> >>   head/sys/kern/sys_procdesc.c
>>> >>   head/sys/kern/uipc_mqueue.c
>>> >>   head/sys/kern/uipc_sem.c
>>> >>   head/sys/kern/uipc_syscalls.c
>>> >>   head/sys/kern/vfs_aio.c
>>> >>   head/sys/kern/vfs_syscalls.c
>>> >>   head/sys/netsmb/smb_dev.c
>>> >>   head/sys/sys/capsicum.h
>>> >>
>>> >> Modified: head/sys/cddl/compat/opensolaris/sys/file.h
>>> >>
>>> >>
>>> >> ==
>>> >> --- head/sys/cddl/compat/opensolaris/sys/file.h Wed May  9 18:41:04
>>> >> 2018
>>> >> (r333424)
>>> >> +++ head/sys/cddl/compat/opensolaris/sys/file.h Wed May  9 18:47:24
>>> >> 2018
>>> >> (r333425)
>>> >> @@ -52,10 +52,9 @@ static __inline void
>>> >>  releasef(int fd)
>>> >>  {
>>> >>   

Re: svn commit: r333425 - in head/sys: cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs compat/cloudabi compat/linux compat/linuxkpi/common/include/linux dev/filemon dev/hwpmc fs

[Matthew Macy]

I can do that tomorrow. But point is that something else will push it over soon.

On Sat, May 19, 2018 at 12:14 AM, Mateusz Guzik <mjgu...@gmail.com> wrote:
> imo all these sysinits can and shoud be collapsed into one, which will have
> a side effect of getting rid of the problem.
>
> On Sat, May 19, 2018 at 9:13 AM, Matthew Macy <mm...@freebsd.org> wrote:
>>
>> I guess we'll need to allocate more pages at boot. We must have been
>> on the edge already if that pushed us over.
>> -M
>>
>> On Fri, May 18, 2018 at 12:03 PM, Ilya Bakulin <i...@bakulin.de> wrote:
>> > Hi Matt,
>> > seems this commit has broken at least BeagleBone Black booting process.
>> > On
>> > all revisions after it the kernel panics with this message:
>> > http://dl.bakulin.de/bbb_panic.txt
>> >  My suspicion is that there are quite a few new SYSINIT objects that are
>> > created on startup, and as a result some kind of memory reservation gets
>> > exhausted. I don't have immediate idea how to debug this further; just
>> > can
>> > confirm that patching out this change allows the board to boot again.
>> >
>> >
>> > On Wed, May 9, 2018 at 8:47 PM Matt Macy <mm...@freebsd.org> wrote:
>> >>
>> >> Author: mmacy
>> >> Date: Wed May  9 18:47:24 2018
>> >> New Revision: 333425
>> >> URL: https://svnweb.freebsd.org/changeset/base/333425
>> >>
>> >> Log:
>> >>   Eliminate the overhead of gratuitous repeated reinitialization of
>> >> cap_rights
>> >>
>> >>   - Add macros to allow preinitialization of cap_rights_t.
>> >>
>> >>   - Convert most commonly used code paths to use preinitialized
>> >> cap_rights_t.
>> >> A 3.6% speedup in fstat was measured with this change.
>> >>
>> >>   Reported by:  mjg
>> >>   Reviewed by:  oshogbo
>> >>   Approved by:  sbruno
>> >>   MFC after:1 month
>> >>
>> >> Modified:
>> >>   head/sys/cddl/compat/opensolaris/sys/file.h
>> >>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>> >>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_onexit.c
>> >>   head/sys/compat/cloudabi/cloudabi_file.c
>> >>   head/sys/compat/linux/linux_event.c
>> >>   head/sys/compat/linux/linux_file.c
>> >>   head/sys/compat/linux/linux_ioctl.c
>> >>   head/sys/compat/linux/linux_mmap.c
>> >>   head/sys/compat/linux/linux_socket.c
>> >>   head/sys/compat/linux/linux_stats.c
>> >>   head/sys/compat/linuxkpi/common/include/linux/file.h
>> >>   head/sys/dev/filemon/filemon.c
>> >>   head/sys/dev/hwpmc/hwpmc_logging.c
>> >>   head/sys/fs/fdescfs/fdesc_vnops.c
>> >>   head/sys/fs/fuse/fuse_vfsops.c
>> >>   head/sys/kern/kern_descrip.c
>> >>   head/sys/kern/kern_event.c
>> >>   head/sys/kern/kern_exec.c
>> >>   head/sys/kern/kern_sendfile.c
>> >>   head/sys/kern/kern_sig.c
>> >>   head/sys/kern/subr_capability.c
>> >>   head/sys/kern/sys_generic.c
>> >>   head/sys/kern/sys_procdesc.c
>> >>   head/sys/kern/uipc_mqueue.c
>> >>   head/sys/kern/uipc_sem.c
>> >>   head/sys/kern/uipc_syscalls.c
>> >>   head/sys/kern/vfs_aio.c
>> >>   head/sys/kern/vfs_syscalls.c
>> >>   head/sys/netsmb/smb_dev.c
>> >>   head/sys/sys/capsicum.h
>> >>
>> >> Modified: head/sys/cddl/compat/opensolaris/sys/file.h
>> >>
>> >>
>> >> ==
>> >> --- head/sys/cddl/compat/opensolaris/sys/file.h Wed May  9 18:41:04
>> >> 2018
>> >> (r333424)
>> >> +++ head/sys/cddl/compat/opensolaris/sys/file.h Wed May  9 18:47:24
>> >> 2018
>> >> (r333425)
>> >> @@ -52,10 +52,9 @@ static __inline void
>> >>  releasef(int fd)
>> >>  {
>> >> struct file *fp;
>> >> -   cap_rights_t rights;
>> >>
>> >> /* No CAP_ rights required, as we're only releasing. */
>> >> -   if (fget(curthread, fd, cap_rights_init(), ) == 0) {
>> >> +   if (fget(curthread, fd, _no_rights, ) == 0) {
>> >> fdrop(fp, curthread);
>> >> fdrop(fp, curthread);
>> >> }
>> >>

Re: svn commit: r333425 - in head/sys: cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs compat/cloudabi compat/linux compat/linuxkpi/common/include/linux dev/filemon dev/hwpmc fs

[Matthew Macy]

I guess we'll need to allocate more pages at boot. We must have been
on the edge already if that pushed us over.
-M

On Fri, May 18, 2018 at 12:03 PM, Ilya Bakulin  wrote:
> Hi Matt,
> seems this commit has broken at least BeagleBone Black booting process. On
> all revisions after it the kernel panics with this message:
> http://dl.bakulin.de/bbb_panic.txt
>  My suspicion is that there are quite a few new SYSINIT objects that are
> created on startup, and as a result some kind of memory reservation gets
> exhausted. I don't have immediate idea how to debug this further; just can
> confirm that patching out this change allows the board to boot again.
>
>
> On Wed, May 9, 2018 at 8:47 PM Matt Macy  wrote:
>>
>> Author: mmacy
>> Date: Wed May  9 18:47:24 2018
>> New Revision: 333425
>> URL: https://svnweb.freebsd.org/changeset/base/333425
>>
>> Log:
>>   Eliminate the overhead of gratuitous repeated reinitialization of
>> cap_rights
>>
>>   - Add macros to allow preinitialization of cap_rights_t.
>>
>>   - Convert most commonly used code paths to use preinitialized
>> cap_rights_t.
>> A 3.6% speedup in fstat was measured with this change.
>>
>>   Reported by:  mjg
>>   Reviewed by:  oshogbo
>>   Approved by:  sbruno
>>   MFC after:1 month
>>
>> Modified:
>>   head/sys/cddl/compat/opensolaris/sys/file.h
>>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_onexit.c
>>   head/sys/compat/cloudabi/cloudabi_file.c
>>   head/sys/compat/linux/linux_event.c
>>   head/sys/compat/linux/linux_file.c
>>   head/sys/compat/linux/linux_ioctl.c
>>   head/sys/compat/linux/linux_mmap.c
>>   head/sys/compat/linux/linux_socket.c
>>   head/sys/compat/linux/linux_stats.c
>>   head/sys/compat/linuxkpi/common/include/linux/file.h
>>   head/sys/dev/filemon/filemon.c
>>   head/sys/dev/hwpmc/hwpmc_logging.c
>>   head/sys/fs/fdescfs/fdesc_vnops.c
>>   head/sys/fs/fuse/fuse_vfsops.c
>>   head/sys/kern/kern_descrip.c
>>   head/sys/kern/kern_event.c
>>   head/sys/kern/kern_exec.c
>>   head/sys/kern/kern_sendfile.c
>>   head/sys/kern/kern_sig.c
>>   head/sys/kern/subr_capability.c
>>   head/sys/kern/sys_generic.c
>>   head/sys/kern/sys_procdesc.c
>>   head/sys/kern/uipc_mqueue.c
>>   head/sys/kern/uipc_sem.c
>>   head/sys/kern/uipc_syscalls.c
>>   head/sys/kern/vfs_aio.c
>>   head/sys/kern/vfs_syscalls.c
>>   head/sys/netsmb/smb_dev.c
>>   head/sys/sys/capsicum.h
>>
>> Modified: head/sys/cddl/compat/opensolaris/sys/file.h
>>
>> ==
>> --- head/sys/cddl/compat/opensolaris/sys/file.h Wed May  9 18:41:04 2018
>> (r333424)
>> +++ head/sys/cddl/compat/opensolaris/sys/file.h Wed May  9 18:47:24 2018
>> (r333425)
>> @@ -52,10 +52,9 @@ static __inline void
>>  releasef(int fd)
>>  {
>> struct file *fp;
>> -   cap_rights_t rights;
>>
>> /* No CAP_ rights required, as we're only releasing. */
>> -   if (fget(curthread, fd, cap_rights_init(), ) == 0) {
>> +   if (fget(curthread, fd, _no_rights, ) == 0) {
>> fdrop(fp, curthread);
>> fdrop(fp, curthread);
>> }
>>
>> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>>
>> ==
>> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>> Wed May  9 18:41:04 2018(r333424)
>> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>> Wed May  9 18:47:24 2018(r333425)
>> @@ -4446,7 +4446,6 @@ zfs_ioc_recv(zfs_cmd_t *zc)
>> char *origin = NULL;
>> char *tosnap;
>> char tofs[ZFS_MAX_DATASET_NAME_LEN];
>> -   cap_rights_t rights;
>> boolean_t first_recvd_props = B_FALSE;
>>
>> if (dataset_namecheck(zc->zc_value, NULL, NULL) != 0 ||
>> @@ -4467,7 +4466,7 @@ zfs_ioc_recv(zfs_cmd_t *zc)
>>  #ifdef illumos
>> fp = getf(fd);
>>  #else
>> -   fget_read(curthread, fd, cap_rights_init(, CAP_PREAD),
>> );
>> +   fget_read(curthread, fd, _pread_rights, );
>>  #endif
>> if (fp == NULL) {
>> nvlist_free(props);
>> @@ -4744,13 +4743,11 @@ zfs_ioc_send(zfs_cmd_t *zc)
>> dsl_pool_rele(dp, FTAG);
>> } else {
>> file_t *fp;
>> -   cap_rights_t rights;
>>
>>  #ifdef illumos
>> fp = getf(zc->zc_cookie);
>>  #else
>> -   fget_write(curthread, zc->zc_cookie,
>> -   cap_rights_init(, CAP_WRITE), );
>> +   fget_write(curthread, zc->zc_cookie, _write_rights,
>> );
>>  #endif
>> if (fp == NULL)
>> return (SET_ERROR(EBADF));
>> @@ -5387,15 +5384,13 @@ static int
>>  zfs_ioc_diff(zfs_cmd_t *zc)
>>  {
>> file_t *fp;
>> -   cap_rights_t rights;
>> offset_t off;
>> int error;
>>
>>  #ifdef 

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

[Matthew Macy]

Sorry, will do

On Fri, May 18, 2018 at 15:29 John Baldwin  wrote:

> On Thursday, May 17, 2018 07:30:57 PM Matt Macy wrote:
> > Author: mmacy
> > Date: Thu May 17 19:30:57 2018
> > New Revision: 333757
> > URL: https://svnweb.freebsd.org/changeset/base/333757
> >
> > Log:
> >   epoch(9): missed add from r333755
> >
> >   Reported by:flo
> >   Approved by:sbruno
> >
> > Modified:
> >   head/sys/sys/proc.h
> >
> > Modified: head/sys/sys/proc.h
> >
> ==
> > --- head/sys/sys/proc.h   Thu May 17 19:10:13 2018(r333756)
> > +++ head/sys/sys/proc.h   Thu May 17 19:30:57 2018(r333757)
> > @@ -322,6 +322,7 @@ struct thread {
> >   u_char  td_pri_class;   /* (t) Scheduling class. */
> >   u_char  td_user_pri;/* (t) User pri from estcpu and
> nice. */
> >   u_char  td_base_user_pri; /* (t) Base user pri */
> > + u_char  td_pre_epoch_prio;
> >   uintptr_t   td_rb_list; /* (k) Robust list head. */
> >   uintptr_t   td_rbp_list;/* (k) Robust priv list head. */
> >   uintptr_t   td_rb_inact;/* (k) Current in-action mutex
> loc. */
>
> Can you annotate the locking for this field for future readers?
>
> --
> John Baldwin
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333775 - in head/sys: kern net sys tests/epoch

[Matthew Macy]

>   - epoch_enter_critical() - can be called inside a different epoch,
> starts a section that will acquire any MTX_DEF mutexes or do
> anything that might sleep.

Should read will _not_ acquire ...

>   - epoch_exit_critical() - corresponding exit call
>   - epoch_wait_critical() - wait variant that is guaranteed that any
> threads in a section are running.
>   - epoch_global_critical - an epoch_wait_critical safe epoch instance
>
>   Requested by:   markj
>   Approved by:  sbruno
>
> Modified:
>   head/sys/kern/subr_epoch.c
>   head/sys/net/if.c
>   head/sys/sys/epoch.h
>   head/sys/tests/epoch/epoch_test.c
>
> Modified: head/sys/kern/subr_epoch.c
> ==
> --- head/sys/kern/subr_epoch.c  Thu May 17 23:59:56 2018(r333774)
> +++ head/sys/kern/subr_epoch.c  Fri May 18 01:52:51 2018(r333775)
> @@ -83,6 +83,12 @@ SYSCTL_COUNTER_U64(_kern_epoch_stats, OID_AUTO, nconte
>  static counter_u64_t switch_count;
>  SYSCTL_COUNTER_U64(_kern_epoch_stats, OID_AUTO, switches, CTLFLAG_RW,
>_count, "# of times a thread 
> voluntarily context switched in epoch_wait");
> +static counter_u64_t epoch_call_count;
> +SYSCTL_COUNTER_U64(_kern_epoch_stats, OID_AUTO, epoch_calls, CTLFLAG_RW,
> +  _call_count, "# of times a callback 
> was deferred");
> +static counter_u64_t epoch_call_task_count;
> +SYSCTL_COUNTER_U64(_kern_epoch_stats, OID_AUTO, epoch_call_tasks, CTLFLAG_RW,
> +  _call_task_count, "# of times a 
> callback task was run");
>
>  TAILQ_HEAD(threadlist, thread);
>
> @@ -103,9 +109,8 @@ struct epoch_pcpu_state {
>  struct epoch {
> struct ck_epoch e_epoch __aligned(EPOCH_ALIGN);
> struct epoch_pcpu_state *e_pcpu_dom[MAXMEMDOM] __aligned(EPOCH_ALIGN);
> -   counter_u64_t e_frees;
> -   uint64_t e_free_last;
> int e_idx;
> +   int e_flags;
> struct epoch_pcpu_state *e_pcpu[0];
>  };
>
> @@ -119,7 +124,7 @@ static __read_mostly int domoffsets[MAXMEMDOM];
>  static __read_mostly int inited;
>  static __read_mostly int epoch_count;
>  __read_mostly epoch_t global_epoch;
> -static __read_mostly epoch_t private_epoch;
> +__read_mostly epoch_t global_epoch_critical;
>
>  static void epoch_call_task(void *context __unused);
>
> @@ -161,8 +166,8 @@ epoch_init(void *arg __unused)
> taskqgroup_attach_cpu(qgroup_softirq, DPCPU_ID_PTR(cpu, 
> epoch_cb_task), NULL, cpu, -1, "epoch call task");
> }
> inited = 1;
> -   global_epoch = epoch_alloc();
> -   private_epoch = epoch_alloc();
> +   global_epoch = epoch_alloc(0);
> +   global_epoch_critical = epoch_alloc(EPOCH_CRITICAL);
>  }
>  SYSINIT(epoch, SI_SUB_TASKQ + 1, SI_ORDER_FIRST, epoch_init, NULL);
>
> @@ -206,7 +211,7 @@ epoch_init_legacy(epoch_t epoch)
>  }
>
>  epoch_t
> -epoch_alloc(void)
> +epoch_alloc(int flags)
>  {
> epoch_t epoch;
>
> @@ -215,12 +220,12 @@ epoch_alloc(void)
> epoch = malloc(sizeof(struct epoch) + mp_ncpus*sizeof(void*),
>M_EPOCH, M_ZERO|M_WAITOK);
> ck_epoch_init(>e_epoch);
> -   epoch->e_frees = counter_u64_alloc(M_WAITOK);
> if (usedomains)
> epoch_init_numa(epoch);
> else
> epoch_init_legacy(epoch);
> MPASS(epoch_count < MAX_EPOCHS-2);
> +   epoch->e_flags = flags;
> epoch->e_idx = epoch_count;
> allepochs[epoch_count++] = epoch;
> return (epoch);
> @@ -240,11 +245,7 @@ epoch_free(epoch_t epoch)
> }
>  #endif
> allepochs[epoch->e_idx] = NULL;
> -   epoch_wait(private_epoch);
> -   /*
> -* Execute any lingering callbacks
> -*/
> -   counter_u64_free(epoch->e_frees);
> +   epoch_wait_critical(global_epoch_critical);
> if (usedomains)
> for (domain = 0; domain < vm_ndomains; domain++)
> free_domain(epoch->e_pcpu_dom[domain], M_EPOCH);
> @@ -289,14 +290,21 @@ epoch_enter_internal(epoch_t epoch, struct thread *td)
>  }
>
>
> -static void
> -epoch_enter_private(ck_epoch_section_t *section)
> +void
> +epoch_enter_critical(epoch_t epoch)
>  {
> -   struct epoch_pcpu_state *eps;
> +   ck_epoch_record_t *record;
> +   ck_epoch_section_t *section;
> +   struct thread *td;
>
> -   MPASS(curthread->td_critnest);
> -   eps = private_epoch->e_pcpu[curcpu];
> -   ck_epoch_begin(>eps_record.er_record, section);
> +   section = NULL;
> +   td = curthread;
> +   critical_enter();
> +   if (__predict_true(td->td_epochnest++ == 0))
> +   section = (ck_epoch_section_t*)>td_epoch_section;
> +
> +   record = >e_pcpu[curcpu]->eps_record.er_record;
> +   ck_epoch_begin(record, section);
>  }
>
>  void
> @@ -321,14 +329,21 @@ epoch_exit_internal(epoch_t epoch, struct thread *td)
>

Re: svn commit: r333765 - head/sys/conf

[Matthew Macy]

It's not breaking the build there, but I'd be fine with removing it
more generally.

-M

On Thu, May 17, 2018 at 2:08 PM, Nathan Whitehorn
 wrote:
>
>
> On 05/17/18 14:04, Matt Macy wrote:
>>
>> Author: mmacy
>> Date: Thu May 17 21:04:19 2018
>> New Revision: 333765
>> URL: https://svnweb.freebsd.org/changeset/base/333765
>>
>> Log:
>>powerpc: fix LINT build
>>   netmap currently doesn't build, take it out of LINT to prevent
>>hiding regressions in universe
>>   Reviewed by:  jhibbits
>>Approved by: sbruno
>>
>> Modified:
>>head/sys/conf/makeLINT.mk
>>
>> Modified: head/sys/conf/makeLINT.mk
>>
>> ==
>> --- head/sys/conf/makeLINT.mk   Thu May 17 21:03:36 2018(r333764)
>> +++ head/sys/conf/makeLINT.mk   Thu May 17 21:04:19 2018(r333765)
>> @@ -53,6 +53,7 @@ LINT: ${NOTES} ${MAKELINT_SED}
>>   .if ${TARGET} == "powerpc"
>> # cat is available, not sure if cp is?
>> cat ${.TARGET} > ${.TARGET}64
>> +   echo "nodevice netmap"  >> ${.TARGET}
>> echo "machine   ${TARGET} powerpc" >> ${.TARGET}
>> echo "machine   ${TARGET} powerpc64" >> ${.TARGET}64
>>   .endif
>>
>
> The build error is that it is broken on all 32-bit architectures because of
> some integer/pointer type confusion, not just (32-bit) PowerPC. So, if we
> aren't fixing it, it at least it should also be removed from ARM, i386,
> mips, etc.
> -Nathan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333745 - in head/sys/contrib/ck: include src

[Matthew Macy]

On Thu, May 17, 2018 at 12:32 PM, Pedro Giffuni <p...@freebsd.org> wrote:
>
>
> On 17/05/2018 14:27, Emmanuel Vadot wrote:
>>
>> On Thu, 17 May 2018 14:20:05 -0500
>> Pedro Giffuni <p...@freebsd.org> wrote:
>>
>>> On 17/05/2018 14:12, Matthew Macy wrote:
>>>>
>>>> How do I avoid problems while allowing timely updates?
>>>>
>>>> -M
>>>>
>>>> On Thu, May 17, 2018 at 11:38 AM, Emmanuel Vadot <m...@bidouilliste.com>
>>>> wrote:
>>>>>
>>>>>Hi Matt,
>>>>>
>>>>> On Thu, 17 May 2018 18:14:10 + (UTC)
>>>>> Matt Macy <mm...@freebsd.org> wrote:
>>>>>
>>>>>> Author: mmacy
>>>>>> Date: Thu May 17 18:14:10 2018
>>>>>> New Revision: 333745
>>>>>> URL: https://svnweb.freebsd.org/changeset/base/333745
>>>>>>
>>>>>> Log:
>>>>>> ck: add support for executing callbacks outside of main poll loop
>>>>>>
>>>>>> Pull in change from upstream
>>>>>> deca119d14bfffd440770eb67cbdbeaf7b57eb7b
>>>>>>
>>>>>> |ck_epoch: introduce ck_epoch_deferred
>>>>>> |
>>>>>> |Allow for deferral to occur outside epoch poll critical loop
>>>>>> (which may access per-CPU structures).
>>>>>> |
>>>>>>
>>>>>> Approved by:sbruno
>>>>>>
>>>>>> Modified:
>>>>>> head/sys/contrib/ck/include/ck_epoch.h
>>>>>> head/sys/contrib/ck/src/ck_epoch.c
>>>>>>
>>>>>CK was imported in vendor-sys/ck, commiting directly into head will
>>>>> cause some problems in the future.
>>>
>>> Actually ... committing to head is fine: some things just have to be
>>> fixed.
>>
>>   This is not true, it can cause a lot of problems with futures updates
>> if the change isn't in the vendor repo.
>
> OK ... I meant generally you can. The catch here is that the change was
> already upstream so it should have been done in the vendor area and then
> merged. It's best to fix this properly now.
>
> Sorry for misguiding.
>

I'm happy to comply - just point me at the fine manual.


-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333745 - in head/sys/contrib/ck: include src

[Matthew Macy]

How do I avoid problems while allowing timely updates?

-M

On Thu, May 17, 2018 at 11:38 AM, Emmanuel Vadot  wrote:
>
>  Hi Matt,
>
> On Thu, 17 May 2018 18:14:10 + (UTC)
> Matt Macy  wrote:
>
>> Author: mmacy
>> Date: Thu May 17 18:14:10 2018
>> New Revision: 333745
>> URL: https://svnweb.freebsd.org/changeset/base/333745
>>
>> Log:
>>   ck: add support for executing callbacks outside of main poll loop
>>
>>   Pull in change from upstream deca119d14bfffd440770eb67cbdbeaf7b57eb7b
>>
>>   |ck_epoch: introduce ck_epoch_deferred
>>   |
>>   |Allow for deferral to occur outside epoch poll critical loop (which 
>> may access per-CPU structures).
>>   |
>>
>>   Approved by:sbruno
>>
>> Modified:
>>   head/sys/contrib/ck/include/ck_epoch.h
>>   head/sys/contrib/ck/src/ck_epoch.c
>>
>
>  CK was imported in vendor-sys/ck, commiting directly into head will
> cause some problems in the future.
>
> --
> Emmanuel Vadot  
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333345 - head/sys/dev/e1000

[Matthew Macy]

See r333631

On Tue, May 15, 2018 at 09:18 Mark Johnston  wrote:

> On Tue, May 08, 2018 at 01:39:45AM +, Matt Macy wrote:
> > Author: mmacy
> > Date: Tue May  8 01:39:45 2018
> > New Revision: 45
> > URL: https://svnweb.freebsd.org/changeset/base/45
> >
> > Log:
> >   Sleep rather than spin in e1000 when doing long running config
> operations.
> >
> >   With r333218 it is now possible for drivers to use an sx lock and thus
> sleep while
> >   waiting on long running operations rather than DELAY().
> >
> >   Reported by:gallatin
> >   Reviewed by:sbruno
> >   Approved by:sbruno
> >   MFC after:  1 month
> >   Sponsored by:   Limelight Networks
> >   Differential Revision:  https://reviews.freebsd.org/D14984
>
> This commit causes igb to fail to attach to one of the ports on an I210:
>
> igb0@pci0:5:0:0:class=0x02 card=0x153315d9 chip=0x15338086
> rev=0x03 hdr=0x00
> vendor = 'Intel Corporation'
> device = 'I210 Gigabit Network Connection'
> class  = network
> subclass   = ethernet
> igb1@pci0:6:0:0:class=0x02 card=0x153315d9 chip=0x15338086
> rev=0x03 hdr=0x00
> vendor = 'Intel Corporation'
> device = 'I210 Gigabit Network Connection'
> class  = network
> subclass   = ethernet
>
> In the dmesg I see:
>
> igb0:  port 0xd000-0xd01f
> mem 0xf830-0xf837,0xf838-0xf8383fff irq 18 at device 0.0
> numa-domain 0 on pci6
> igb0: attach_pre capping queues at 4
> igb0: Setup of Shared code failed, error -2
> igb0: IFDI_ATTACH_PRE failed 6
> device_attach: igb0 attach returned 6
> pcib7:  irq 19 at device 28.3 numa-domain 0 on pci1
> pci7:  numa-domain 0 on pcib7
> igb0:  port 0xc000-0xc01f
> mem 0xf820-0xf827,0xf828-0xf8283fff irq 19 at device 0.0
> numa-domain 0 on pci7
> igb0: attach_pre capping queues at 4
> igb0: using 1024 tx descriptors and 1024 rx descriptors
> igb0: msix_init qsets capped at 4
> igb0: pxm cpus: 8 queue msgs: 4 admincnt: 1
> igb0: using 4 rx queues 4 tx queues
> igb0: Using MSIX interrupts with 5 vectors
> igb0: allocated for 4 tx_queues
> igb0: allocated for 4 rx_queues
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333509 - in head/sys: dev/hwpmc kern sys

[Matthew Macy]

Fixed in 333575.

On Sat, May 12, 2018 at 00:35 Peter Holm  wrote:

> On Sat, May 12, 2018 at 01:26:34AM +, Matt Macy wrote:
> > Author: mmacy
> > Date: Sat May 12 01:26:34 2018
> > New Revision: 333509
> > URL: https://svnweb.freebsd.org/changeset/base/333509
> >
> > Log:
> >   hwpmc(9): Make pmclog buffer pcpu and update constants
> >
> >   On non-trivial SMP systems the contention on the pmc_owner mutex leads
> >   to a substantial number of samples captured being from the pmc process
> >   itself. This change a) makes buffers larger to avoid contention on the
> >   global list b) makes the working sample buffer per cpu.
> >
> >   Run pmcstat in the background (default event rate of 64k):
> >   pmcstat -S UNHALTED_CORE_CYCLES -O /dev/null sleep 600 &
> >
> >   Before:
> >   make -j96 buildkernel -s >&/dev/null 3336.68s user 24684.10s system
> 7442% cpu 6:16.50 total
> >
> >   After:
> >   make -j96 buildkernel -s >&/dev/null 2697.82s user 1347.35s system
> 6058% cpu 1:06.77 total
> >
> >   For more realistic overhead measurement set the sample rate for ~2khz
> >   on a 2.1Ghz processor:
> >   pmcstat -n 105 -S UNHALTED_CORE_CYCLES -O /dev/null sleep 6000 &
> >
> >   Collecting 10 samples of `make -j96 buildkernel` from each:
> >
> >   x before
> >   + after
> >
> >   real time:
> >   N   Min   MaxMedian   Avg
> Stddev
> >   x  10  76.4127.6284.84588.577
>  15.100031
> >   +  10 59.71 60.7960.13560.179
> 0.29957192
> >   Difference at 95.0% confidence
> >   -28.398 +/- 10.0344
> >   -32.0602% +/- 7.69825%
> >   (Student's t, pooled s = 10.6794)
> >
> >   system time:
> >   N   Min   MaxMedian   Avg
> Stddev
> >   x  10   2277.96   6948.53   2949.47  3341.492
>  1385.2677
> >   +  101038.7   1081.06  1070.555  1064.017
> 15.85404
> >   Difference at 95.0% confidence
> >   -2277.47 +/- 920.425
> >   -68.1574% +/- 8.77623%
> >   (Student's t, pooled s = 979.596)
> >
> >   x no pmc
> >   + pmc running
> >   real time:
> >
> >   HEAD:
> >   N   Min   MaxMedian   Avg
> Stddev
> >   x  10 58.38 59.15 58.8658.847
> 0.22504567
> >   +  10  76.4127.6284.84588.577
>  15.100031
> >   Difference at 95.0% confidence
> >   29.73 +/- 10.0335
> >   50.5208% +/- 17.0525%
> >   (Student's t, pooled s = 10.6785)
> >
> >   patched:
> >   N   Min   MaxMedian   Avg
> Stddev
> >   x  10 58.38 59.15 58.8658.847
> 0.22504567
> >   +  10 59.71 60.7960.13560.179
> 0.29957192
> >   Difference at 95.0% confidence
> >   1.332 +/- 0.248939
> >   2.2635% +/- 0.426506%
> >   (Student's t, pooled s = 0.264942)
> >
> >   system time:
> >
> >   HEAD:
> >   N   Min   MaxMedian   Avg
> Stddev
> >   x  10   1010.15   1073.31  1025.465  1031.524
>  18.135705
> >   +  10   2277.96   6948.53   2949.47  3341.492
>  1385.2677
> >   Difference at 95.0% confidence
> >   2309.97 +/- 920.443
> >   223.937% +/- 89.3039%
> >   (Student's t, pooled s = 979.616)
> >
> >   patched:
> >   N   Min   MaxMedian   Avg
> Stddev
> >   x  10   1010.15   1073.31  1025.465  1031.524
>  18.135705
> >   +  101038.7   1081.06  1070.555  1064.017
> 15.85404
> >   Difference at 95.0% confidence
> >   32.493 +/- 16.0042
> >   3.15% +/- 1.5794%
> >   (Student's t, pooled s = 17.0331)
> >
> >   Reviewed by:jeff@
> >   Approved by:sbruno@
> >   Differential Revision:  https://reviews.freebsd.org/D15155
> >
> > Modified:
> >   head/sys/dev/hwpmc/hwpmc_amd.c
> >   head/sys/dev/hwpmc/hwpmc_core.c
> >   head/sys/dev/hwpmc/hwpmc_e500.c
> >   head/sys/dev/hwpmc/hwpmc_intel.c
> >   head/sys/dev/hwpmc/hwpmc_logging.c
> >   head/sys/dev/hwpmc/hwpmc_mod.c
> >   head/sys/dev/hwpmc/hwpmc_mpc7xxx.c
> >   head/sys/dev/hwpmc/hwpmc_piv.c
> >   head/sys/dev/hwpmc/hwpmc_ppc970.c
> >   head/sys/dev/hwpmc/hwpmc_ppro.c
> >   head/sys/dev/hwpmc/hwpmc_soft.c
> >   head/sys/kern/kern_pmc.c
> >   head/sys/sys/pmc.h
> >   head/sys/sys/pmckern.h
> >
> > Modified: head/sys/dev/hwpmc/hwpmc_amd.c
> >
> ==
> > --- head/sys/dev/hwpmc/hwpmc_amd.cFri May 11 22:16:23 2018
> (r333508)
>
> Got this during boot:
>
> ada1: Serial Number 15150F3BC143
> ada1: 600.000MB/s transfers (SATA 3.x, UDMA6, PIO 8192bytes)
> ada1: Command Queueing enabled
> ada1: 976762MB (2000409264 512 byte sectors)
>
>
> Fatal trap 12: page fault while in kernel mode
> cpuid 

Re: svn commit: r333466 - in head: contrib/bmake sys/conf sys/kern sys/modules/epoch_test sys/sys sys/tests/epoch

[Matthew Macy]

It fixes something that is completely broken in bmake that everyone in
the know patches on their local systems. It was included by accident.
I immediately backed it out in the subsequent commit.

On Fri, May 11, 2018 at 1:13 PM, Bryan Drewery  wrote:
> On 5/10/2018 10:55 AM, Matt Macy wrote:
>> Author: mmacy
>> Date: Thu May 10 17:55:24 2018
>> New Revision: 333466
>> URL: https://svnweb.freebsd.org/changeset/base/333466
>>
>> Log:
>>   Add simple preempt safe epoch API
>>
>>   Read locking is over used in the kernel to guarantee liveness. This API 
>> makes
>>   it easy to provide livenes guarantees without atomics.
>>
>>   Includes epoch_test kernel module to stress test the API.
>>
>>   Documentation will follow initial use case.
>>
>>   Test case and improvements to preemption handling in response to discussion
>>   with mjg@
>>
>>   Reviewed by:imp@, shurd@
>>   Approved by:sbruno@
>>
>> Added:
>>   head/sys/kern/subr_epoch.c   (contents, props changed)
>>   head/sys/modules/epoch_test/
>>   head/sys/modules/epoch_test/Makefile   (contents, props changed)
>>   head/sys/sys/epoch.h   (contents, props changed)
>>   head/sys/tests/epoch/
>>   head/sys/tests/epoch/epoch_test.c   (contents, props changed)
>> Modified:
>>   head/contrib/bmake/job.c
>>   head/sys/conf/files
>>   head/sys/conf/kern.pre.mk
>>   head/sys/kern/kern_malloc.c
>>   head/sys/kern/kern_synch.c
>>   head/sys/kern/subr_trap.c
>>   head/sys/kern/subr_turnstile.c
>>   head/sys/sys/proc.h
>>   head/sys/sys/turnstile.h
>>
>> Modified: head/contrib/bmake/job.c
>> ==
>> --- head/contrib/bmake/job.c  Thu May 10 17:22:04 2018(r333465)
>> +++ head/contrib/bmake/job.c  Thu May 10 17:55:24 2018(r333466)
>> @@ -2121,13 +2121,15 @@ Job_CatchOutput(void)
>>  {
>>  int nready;
>>  Job *job;
>> -int i;
>> +int i, pollToken;
>>
>>  (void)fflush(stdout);
>>
>> + pollToken = 0;
>> +
>>  /* The first fd in the list is the job token pipe */
>>  do {
>> - nready = poll(fds + 1 - wantToken, nfds - 1 + wantToken, POLL_MSEC);
>> + nready = poll(fds + 1 - pollToken, nfds - 1 + pollToken, POLL_MSEC);
>>  } while (nready < 0 && errno == EINTR);
>>
>>  if (nready < 0)
>
> What's up with this?
>
> --
> Regards,
> Bryan Drewery
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333466 - in head: contrib/bmake sys/conf sys/kern sys/modules/epoch_test sys/sys sys/tests/epoch

[Matthew Macy]

Thanks

On Fri, May 11, 2018 at 11:47 Mark Johnston  wrote:

> On Thu, May 10, 2018 at 05:55:25PM +, Matt Macy wrote:
> > Author: mmacy
> > Date: Thu May 10 17:55:24 2018
> > New Revision: 333466
> > URL: https://svnweb.freebsd.org/changeset/base/333466
> >
> > Log:
> >   Add simple preempt safe epoch API
> >
> >   Read locking is over used in the kernel to guarantee liveness. This
> API makes
> >   it easy to provide livenes guarantees without atomics.
> >
> >   Includes epoch_test kernel module to stress test the API.
> >
> >   Documentation will follow initial use case.
> >
> >   Test case and improvements to preemption handling in response to
> discussion
> >   with mjg@
> >
> >   Reviewed by:imp@, shurd@
> >   Approved by:sbruno@
> >
> > Added:
> >   head/sys/kern/subr_epoch.c   (contents, props changed)
> >   head/sys/modules/epoch_test/
> >   head/sys/modules/epoch_test/Makefile   (contents, props changed)
> >   head/sys/sys/epoch.h   (contents, props changed)
> >   head/sys/tests/epoch/
> >   head/sys/tests/epoch/epoch_test.c   (contents, props changed)
> > Modified:
> >   head/contrib/bmake/job.c
> >   head/sys/conf/files
> >   head/sys/conf/kern.pre.mk
> >   head/sys/kern/kern_malloc.c
> >   head/sys/kern/kern_synch.c
> >   head/sys/kern/subr_trap.c
> >   head/sys/kern/subr_turnstile.c
> >   head/sys/sys/proc.h
> >   head/sys/sys/turnstile.h
> >
> > Modified: head/sys/sys/proc.h
> >
> ==
> > --- head/sys/sys/proc.h   Thu May 10 17:22:04 2018(r333465)
> > +++ head/sys/sys/proc.h   Thu May 10 17:55:24 2018(r333466)
> > @@ -243,6 +243,7 @@ struct thread {
> >
> >  /* Cleared during fork1() */
> >  #define  td_startzero td_flags
> > + u_char  td_epochnest; /* (k) Private thread epoch nest counter */
>
> It looks like td_epochnest was inteded to be in the zeroed section, but
> td_startzero still points to td_flags.
>
> >   int td_flags;   /* (t) TDF_* flags. */
> >   int td_inhibitors;  /* (t) Why can not run. */
> >   int td_pflags;  /* (k) Private thread (TDP_*)
> flags. */
> > @@ -355,6 +356,7 @@ struct thread {
> >   int td_lastcpu; /* (t) Last cpu we were on. */
> >   int td_oncpu;   /* (t) Which cpu we are on. */
> >   void*td_lkpi_task;  /* LinuxKPI task struct pointer */
> > + TAILQ_ENTRY(thread) td_epochq;  /* (t) Epoch queue. */
> >  };
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"