svn commit: r277213 - in head: share/man/man9 sys/kern sys/ofed/include/linux sys/sys

Wed Jan 21 00:22:48 UTC 2015

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 01/20/15 15:59, K. Macy wrote:
> On Tue, Jan 20, 2015 at 3:53 PM, Sean Bruno
> <sbruno at ignoranthack.me> wrote: On 01/20/15 15:48, K. Macy wrote:
>>>> Are any other drivers hitting this? e.g. cxgb/cxgbe?
>>>> 
>>>> -K
>>>> 
> 
> Unkown to me.  Nor am I aware of anyone else who ever hit our
> panics either.  Our environment, and the failure, was only seen in
> the Intel 10GE space (ixgbe).  This is an artifact of our use
> cases, and hasn't been expanded nor tested in our environment with
> other vendor interfaces.
> 
>> For an ill characterized problem isolated to one environment
>> this seems like a workaround that should not be part of the
>> general code base.
> 
>> -K
> 

There was never any indication in our testing that the driver was
involved in any way.

In our universe, this commit (right or wrong) resolved our panics.  I
think that there is some room for improvement based on the commentary
in this thread, but some people do indeed prefer stability over
performance.  I hope we can come to a middle ground somewhere here.

sean

> 
> sean
> 
>>>> On Tue, Jan 20, 2015 at 3:46 PM, Sean Bruno 
>>>> <sbruno at ignoranthack.me> wrote: On 01/20/15 15:40, K. Macy
>>>> wrote:
>>>>>>> I think you're working around driver locking bugs by 
>>>>>>> crippling the callout code.
>>>>>>> 
>>>>>>> -K
>>>>>>> 
>>>> 
>>>> We had zero evidence of this.  What leads you down that path?
>>>> I'm totally open to being wrong, e.g. "yeah, you slowed down
>>>> things so that you don't hit a race condition"
>>>> 
>>>> sean
>>>> 
>>>>>>> On Tue, Jan 20, 2015 at 3:35 PM, Sean Bruno 
>>>>>>> <sbruno at ignoranthack.me> wrote: On 01/20/15 14:30,
>>>>>>> Hans Petter Selasky wrote:
>>>>>>>>>> On 01/20/15 22:11, Gleb Smirnoff wrote:
>>>>>>>>>>> On Tue, Jan 20, 2015 at 09:51:26AM +0200,
>>>>>>>>>>> Konstantin Belousov wrote: K> > Like stated in
>>>>>>>>>>> the manual page, callout_reset_curcpu/on() does
>>>>>>>>>>> not work K> > with MPSAFE callouts any more! K>
>>>>>>>>>>> I.e. you 'fixed' some undeterminate bugs in
>>>>>>>>>>> callout migration by not K> doing migration at
>>>>>>>>>>> all anymore. K> K> > K> > You need to use
>>>>>>>>>>> callout_init_{mtx,rm,rw} and remove the custom 
>>>>>>>>>>> locking K> > inside the callback in the TCP
>>>>>>>>>>> stack to get it working like before! K> K> No,
>>>>>>>>>>> you need to do this, if you think that whole
>>>>>>>>>>> callout KPI must be K> rototiled.  It is up to
>>>>>>>>>>> the person who modifies the KPI, to ensure that
>>>>>>>>>>> K> existing code is not broken. K> K> As I
>>>>>>>>>>> understand, currently we are back to the 
>>>>>>>>>>> one-cpu callouts. K> Do other people consider
>>>>>>>>>>> this situation acceptable ?
>>>>>>>>>>> 
>>>>>>>>>>> I think this isn't acceptable. The commit to a 
>>>>>>>>>>> complex subsystem lacked a review from persons 
>>>>>>>>>>> involved in the system before. The commit to 
>>>>>>>>>>> subsystem broke consumers of the subsystem and
>>>>>>>>>>> this was even done not accidentially, but due
>>>>>>>>>>> to Hans not caring about it.
>>>>>>>>>>> 
>>>>>>>>>>> As for me this is enough to request a backout,
>>>>>>>>>>> and let the change back in only after proper
>>>>>>>>>>> review.
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Hi Gleb,
>>>>>>>>>> 
>>>>>>>>>> Backing out my callout API patch means we will
>>>>>>>>>> for sure re-introduce an unknown callout spinlock
>>>>>>>>>> hang, as noted to me by several people. What do
>>>>>>>>>> you think about that? dram Maybe "Jason Wolfe"
>>>>>>>>>> CC'ed can add to 10-stable w/o my patches:
>>>>>>>>>> 
>>>>>>> 
>>>>>>> Jason picked up this patch for work and it resolved
>>>>>>> our instability issues that had remained unsolved for
>>>>>>> quite some time as reported to freebsd-net:
>>>>>>> 
>>>>>>> https://lists.freebsd.org/pipermail/freebsd-net/2015-January/040895.html
>>>>>>>
>>>>>>>
>>>>>>>
>
>>>>>>> 
This had gone undiagnosed for some time (even with the gracious
>>>>>>> help of jhb in offline emails, thanks btw!).
>>>>>>> 
>>>>>>> There's some diagnostics in that email thread that may
>>>>>>> be of value to you folks for determination of the
>>>>>>> validity of changing the callout API or at least
>>>>>>> understanding why we were involved in diagnostics.
>>>>>>> 
>>>>>>> While I'd sure love to tune performance, the fact that
>>>>>>> our machines were basically going out to lunch without
>>>>>>> these changes, probably means that others were seeing
>>>>>>> it and didn't know what else to do.  As much as I enjoy
>>>>>>> a good "break out the pitch forks and torches" email
>>>>>>> thread, this increased stability for us and is allowing
>>>>>>> us to upgrade from freebsd8 to freebsd10.  Bear this in
>>>>>>> mind when you throw your voice in favor of reverting.
>>>>>>> 
>>>>>>>>>> int callout_reset_sbt_on(struct callout *c,
>>>>>>>>>> sbintime_t sbt, sbintime_t precision, void
>>>>>>>>>> (*ftn)(void *), void *arg, int cpu, int flags) {
>>>>>>>>>> sbintime_t to_sbt, pr; struct callout_cpu *cc;
>>>>>>>>>> int cancelled, direct;
>>>>>>>>>> 
>>>>>>>>>> +       cpu = timeout_cpu;   /* XXX test code XXX
>>>>>>>>>> */
>>>>>>>>>> 
>>>>>>>>>> cancelled = 0;
>>>>>>>>>> 
>>>>>>> 
>>>>>>> Jason or I would have to run this in production, which
>>>>>>> would be problematic I fear.  We never had a
>>>>>>> deterministic test case that would exhibit the reported
>>>>>>> failure.  We merely "tested in production" and saw that
>>>>>>> panics ceased.  We didn't note a dropoff in our traffic
>>>>>>> either, perhaps we are not as efficient as others in
>>>>>>> this corner case, but we were consistently seeing the
>>>>>>> spinlock hangs after a day or so of traffic.
>>>>>>> 
>>>>>>>>>> And see if he observes a callout spinlock hang or
>>>>>>>>>> not on his test setup. The patch above should
>>>>>>>>>> force all callouts to the same thread basically.
>>>>>>>>>> Then we could maybe see if single threading the
>>>>>>>>>> callouts has anything to do with solving the
>>>>>>>>>> spinlock hang.
>>>>>>>>>> 
>>>>>>>>>> The "rewritten" callout API still has all the
>>>>>>>>>> features and capabilities the old one had, when
>>>>>>>>>> used as described in "man 9 callout".
>>>>>>>>>> 
>>>>>>>>>> At the present moment I'm not technically
>>>>>>>>>> convinced a backout is correct.
>>>>>>> 
>>>>>>> Neither am I, to be honest.  Just based on *results*.
>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Gleb: I think we would see far better results
>>>>>>>>>> with high speed internet links using TCP if we
>>>>>>>>>> could extend the LRO (large receive offload) code
>>>>>>>>>> to accumulate more than 64KBytes worth of data
>>>>>>>>>> per call to the TCP stack instead of complaining
>>>>>>>>>> about some callouts ending up on the same thread!
>>>>>>>>>> Actually I have a patch for that.
>>>>>>>>>> 
>>>>>>>>>> --HPS
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>> 
>>>>>>>> _______________________________________________ 
>>>>>>>> svn-src-head at freebsd.org mailing list 
>>>>>>>> http://lists.freebsd.org/mailman/listinfo/svn-src-head
>>>>>>>> To unsubscribe, send any mail to 
>>>>>>>> "svn-src-head-unsubscribe at freebsd.org"
>>>>>>> 
>>>>>>> 
>>>> 
>>>> 
>>>> 
> 
> 
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQF8BAEBCgBmBQJUvvFSXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRCQUFENDYzMkU3MTIxREU4RDIwOTk3REQx
MjAxRUZDQTFFNzI3RTY0AAoJEBIB78oecn5k1AcIAJvIeDso4/a4YqEyXxGj4CAb
QLfESoILRDF3LpczRpIFGnUPRs9pWUTm6DIUut0ZjgLChq1kHzdi2uIFe9QB3ehX
zzw4MEtxDEqXv5zOAmP1gvcx1a2rKZJnTlfW2CHa2QAYTR06BFARu8u3NyC3tZiq
o/NGpidv+nrkcrDSHmzpVgIrlZzyB+YsGyNcvRUkewxMpos9syB2sKiXm9u/MQYQ
nHyjFw9IOjDFAiZgww0y/8QYB9efr639Hgt1BYn86t4iZzwDOajH+jeb9VXMT5s9
liauQ4NiiMBe6F/mldoJ+XrtlPZ9rdx5jbgz8zBQcB7LzoWv91q0GOVpk/Am8FQ=
=E1eL
-----END PGP SIGNATURE-----