-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
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
>
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQF8BAEBCgBmBQJUvuYrXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRCQUFENDYzMkU3MTIxREU4RDIwOTk3REQx
MjAxRUZDQTFFNzI3RTY0AAoJEBIB78oecn5kJTMIAMfh6ghV/AwQauY+a44q1hjJ
WC7E3u69FK0opgSYg71kk6HckbyB+sTWND6HdXnpyrcMLXUt74zlB8c48wbUUV5+
EwKNYzGNNnDNhoc0WtPMect8e9Y1kBRvSGfHBdVrqATXfPOyZEa+i4lQAXpiFKIt
nndqVrAH7bJM6143YDpnIg7vaR+8IQnC2ztSP4ogJzh03DZ7zVsg4BsoCPg50eVZ
kr46cXcE+SP/TsQBsVNVwRJD5NFie6QJdLoTnwkd0XfQGJMIWivNgUcE4tIxqPIf
nGQ0xMJCotpNLuPtzYzCIurSaDHdHmL6bjkhjTtBmWsMNfdGH8TKoih79GDxkTg=
=Y3rd
-----END PGP SIGNATURE-----
_______________________________________________
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-unsubscr...@freebsd.org"
