Date: Tue, 20 Jan 2015 15:59:48 -0800 From: "K. Macy" <kmacy@freebsd.org> To: Sean Bruno <sbruno@freebsd.org> Cc: Hans Petter Selasky <hps@selasky.org>, Adrian Chadd <adrian@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, Jason Wolfe <nitroboost@gmail.com>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Gleb Smirnoff <glebius@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com> Subject: Re: svn commit: r277213 - in head: share/man/man9 sys/kern sys/ofed/include/linux sys/sys Message-ID: <CAHM0Q_PtJ7JHFTiu9_dmi_Ce=rmu1j72z2OYQ2CD3%2BEbcoEGsA@mail.gmail.com> In-Reply-To: <54BEEA7F.1070301@ignoranthack.me> References: <201501151532.t0FFWV2Y037455@svn.freebsd.org> <CAJ-Vmok0GXZoojyi=jE=b5D-d338APztaf3Pw0_AAQ-173XSWw@mail.gmail.com> <54BDD9E1.6090505@selasky.org> <20150120075126.GA42409@kib.kiev.ua> <20150120211137.GY15484@FreeBSD.org> <54BED6FB.8060401@selasky.org> <54BEE62D.2060703@ignoranthack.me> <CAHM0Q_MDJN_8sTvTDXfqA7UtJVO3Y8S8%2BNRCs_=6Nj4dkTzjOA@mail.gmail.com> <54BEE8E6.3080009@ignoranthack.me> <CAHM0Q_N_53BM-6RvXu8UpjfDzQHEn5oXZo1Nn8RO0cuOUhe8tg@mail.gmail.com> <54BEEA7F.1070301@ignoranthack.me>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jan 20, 2015 at 3:53 PM, Sean Bruno <sbruno@ignoranthack.me> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> 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
> sean
>
>> On Tue, Jan 20, 2015 at 3:46 PM, Sean Bruno
>> <sbruno@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@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@freebsd.org mailing list
>>>>>> http://lists.freebsd.org/mailman/listinfo/svn-src-head To
>>>>>> unsubscribe, send any mail to
>>>>>> "svn-src-head-unsubscribe@freebsd.org"
>>>>>
>>>>>
>>
>>
>>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQF8BAEBCgBmBQJUvup4XxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
> ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRCQUFENDYzMkU3MTIxREU4RDIwOTk3REQx
> MjAxRUZDQTFFNzI3RTY0AAoJEBIB78oecn5kNdgH/iFU6kSAcuPJDwiNONLULu9c
> Nu9kJzQUgCpROor2CnrsjN0WILWTAfmWaQK1b7ClUIwKPmgzvX09DZdsdfQnIMFR
> aimC3uKW0rJeMDlQNbN/J2mI5+QOyREUozxb53jetDj9t+OmC08idne0AuYuMgwY
> NZoKt2llhJScjjEu2EJV9W3FoDCmg/ITA83IbzmmoZHHtXACdDVh0vwmNb1UPluA
> 0hcJv4rQrY5Khq08a4tonQxTB96Sgk/c+0zPpvqSctKEEmf6TiREbwr68SOM4dkY
> zLxpD/9IeExFTShJSGW+slKAuzCobr+iG8a9tDLV+STtoAX9i5jyLQLnwDcX7AY=
> =F1sR
> -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHM0Q_PtJ7JHFTiu9_dmi_Ce=rmu1j72z2OYQ2CD3%2BEbcoEGsA>