Date: Sun, 18 Nov 2018 18:38:49 -0700 From: Warner Losh <imp@bsdimp.com> To: "Rodney W. Grimes" <rgrimes@freebsd.org> Cc: Allan Jude <allanjude@freebsd.org>, Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r340450 - head/sys/sys Message-ID: <CANCZdfoGKKcL70ESKow=hfNABpO=5%2BQtUYcNmpt4gReRkeUvrA@mail.gmail.com> In-Reply-To: <201811190104.wAJ14CaE059062@pdx.rh.CN85.dnsmgr.net> References: <CANCZdfqr=R-MCpDEGWDqGYJbcQ46Hqw7PMrVinAsYTRPjLjJPA@mail.gmail.com> <201811190104.wAJ14CaE059062@pdx.rh.CN85.dnsmgr.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Nov 18, 2018 at 6:04 PM Rodney W. Grimes < freebsd@pdx.rh.cn85.dnsmgr.net> wrote: > [ Charset UTF-8 unsupported, converting... ] > > I don't see how this could possibly have changed things. The changes will > > change the least significant bit by one for fractional results. I've > looked > > at all the functions that use it and get called by the mlx driver and > have > > trouble seeing where it could be relevant... > > > > Can you do an experiment to let me know which of the three functions I > > changed breaks things? > > Alan well have to answer on that. I keep stairing at this > code change and I wonder.. has clang done something odd > with the - 1? What I read you trying to do is to remove > a LSB from a left shifted by 32 value, but what has clang > decided to do? Has it somehow gotten this wrong? Is there > some promottion of the type of the bare 1 causing an issue? > I'll talk to Allan to see if he can test that. the bare 1 should be handled properly because of C's promotion rules. 1ull << 32 is an unsigned long long. What I really wanted was "~(uint32_t)0" but that construct has bit me in the past. > Could you look at the assmbler output for your expression > and see that it does infact do what you expected it to do? > I haven't, but that's kinda hard since these are inline functions... I have some of this hardware at work, but don't think I have any machines with it assigned to me. If I can't work it out by the end of Tuesday, I'll back it out until I can get to the bottom of it (since I'll be out most of Wed->Sun). Warner > Thanks, > Rod > Scratching my head as much as anyone on this. > > > > > Warner > > > > On Sun, Nov 18, 2018 at 12:56 PM Allan Jude <allanjude@freebsd.org> > wrote: > > > > > On 2018-11-15 11:02, Warner Losh wrote: > > > > Author: imp > > > > Date: Thu Nov 15 16:02:13 2018 > > > > New Revision: 340450 > > > > URL: https://svnweb.freebsd.org/changeset/base/340450 > > > > > > > > Log: > > > > When converting ns,us,ms to sbt, return the ceil() of the result > > > > rather than the floor(). Returning the floor means that > > > > sbttoX(Xtosbt(y)) != y for almost all values of y. In practice, > this > > > > results in a difference of at most 1 in the lsb of the sbintime_t. > > > > This difference is meaningless for all current users of these > > > > functions, but is important for the newly introduced sysctl > conversion > > > > routines which implicitly rely on the transformation being > idempotent. > > > > > > > > Sponsored by: Netflix, Inc > > > > > > > > > > This seems to break attaching for my mlxen(4), with or without r340451 > > > > > > I don't understand why at this point. > > > > > > Nov 18 19:28:12 CA-HML3-09 kernel: mlx4_core0: <mlx4_core> mem > > > 0xfbe00000-0xfbefffff,0xfa800000-0xfaffffff irq 48 at device 0.0 > > > numa-domain 1 on pci11 > > > Nov 18 19:28:12 CA-HML3-09 kernel: mlx4_core: Initializing mlx4_core > > > Nov 18 19:29:12 CA-HML3-09 kernel: mlx4_core0: command 0x4 timed out > (go > > > bit not cleared) > > > Nov 18 19:29:12 CA-HML3-09 kernel: mlx4_core0: device is going to be > reset > > > Nov 18 19:29:12 CA-HML3-09 kernel: mlx4_core0: device was reset > > > successfully > > > Nov 18 19:29:13 CA-HML3-09 kernel: mlx4_core0: QUERY_FW command failed, > > > aborting > > > Nov 18 19:29:13 CA-HML3-09 kernel: mlx4_core0: Failed to init fw, > aborting. > > > Nov 18 19:29:13 CA-HML3-09 kernel: device_attach: mlx4_core0 attach > > > returned 5 > > > > > > It works fine under r340449: > > > > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_core0: <mlx4_core> mem > > > 0xfbe00000-0xfbefffff,0xfa800000-0xfaffffff irq 48 at device 0.0 > > > numa-domain 1 on pci11 > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_core: Mellanox ConnectX core > > > driver v3.4.1 (October 2017) > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_core: Initializing mlx4_core > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_core0: Unable to determine PCI > > > device chain minimum BW > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_en mlx4_core0: Activating > port:1 > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlxen0: Ethernet address: > > > 00:02:c9:4d:6a:e8 > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_en: mlx4_core0: Port 1: Using > 32 > > > TX rings > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlxen0: link state changed to DOWN > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_en: mlx4_core0: Port 1: Using > 16 > > > RX rings > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_en: mlxen0: Using 32 TX rings > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_en: mlxen0: Using 16 RX rings > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_en: mlxen0: Initializing port > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlx4_en: mlxen0: Link Down > > > Nov 18 19:46:07 CA-HML3-09 kernel: mlxen0: link state changed to UP > > > > > > > > > > > > > Modified: > > > > head/sys/sys/time.h > > > > > > > > Modified: head/sys/sys/time.h > > > > > > > > ============================================================================== > > > > --- head/sys/sys/time.h Thu Nov 15 08:43:17 2018 > (r340449) > > > > +++ head/sys/sys/time.h Thu Nov 15 16:02:13 2018 > (r340450) > > > > @@ -161,6 +161,10 @@ sbttobt(sbintime_t _sbt) > > > > * Decimal<->sbt conversions. Multiplying or dividing by SBT_1NS > > > results in > > > > * large roundoff errors which sbttons() and nstosbt() avoid. > > > Millisecond and > > > > * microsecond functions are also provided for completeness. > > > > + * > > > > + * These functions return the smallest sbt larger or equal to the > > > number of > > > > + * seconds requested so that sbttoX(Xtosbt(y)) == y. The 1 << 32 - 1 > > > term added > > > > + * transforms the >> 32 from floor() to ceil(). > > > > */ > > > > static __inline int64_t > > > > sbttons(sbintime_t _sbt) > > > > @@ -173,7 +177,7 @@ static __inline sbintime_t > > > > nstosbt(int64_t _ns) > > > > { > > > > > > > > - return ((_ns * (((uint64_t)1 << 63) / 500000000)) >> 32); > > > > + return ((_ns * (((uint64_t)1 << 63) / 500000000) + (1ull << > 32) - > > > 1) >> 32); > > > > } > > > > > > > > static __inline int64_t > > > > @@ -187,7 +191,7 @@ static __inline sbintime_t > > > > ustosbt(int64_t _us) > > > > { > > > > > > > > - return ((_us * (((uint64_t)1 << 63) / 500000)) >> 32); > > > > + return ((_us * (((uint64_t)1 << 63) / 500000) + (1ull << 32) - > 1) > > > >> 32); > > > > } > > > > > > > > static __inline int64_t > > > > @@ -201,7 +205,7 @@ static __inline sbintime_t > > > > mstosbt(int64_t _ms) > > > > { > > > > > > > > - return ((_ms * (((uint64_t)1 << 63) / 500)) >> 32); > > > > + return ((_ms * (((uint64_t)1 << 63) / 500) + (1ull << 32) - 1) > >> > > > 32); > > > > } > > > > > > > > /*- > > > > > > > > > > > > > -- > > > Allan Jude > > > > > > > > -- > Rod Grimes > rgrimes@freebsd.org >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfoGKKcL70ESKow=hfNABpO=5%2BQtUYcNmpt4gReRkeUvrA>