Hi,
On 10/14/16 11:35 AM, Slawa Olhovchenkov wrote:> On Thu, Oct 13, 2016 at 06:14:29PM +0200, Julien Charbon wrote:
>> On 10/13/16 5:17 PM, Slawa Olhovchenkov wrote:
>>> On Thu, Oct 13, 2016 at 05:06:00PM +0200, Julien Charbon wrote:
>>>
>>>>>> will give you that trace in the core, and without
INVARIANT then it is
>>>>>> better to use dtrace:
>>>>>>
>>>>>> $ cat tcp-twstart-dropped.d
>>>>>> fbt::tcp_twstart:entry
>>>>>> /args[0]->t_inpcb->inp_flags & 0x04000000/
>>>>>> {
>>>>>> stack();
>>>>>> printf("INP_DROPPED in tcp_twstart: %x",
args[0]->t_inpcb->inp_flags);
>>>>>> }
>>>>>
>>>>> Same code may be insert there too, IMHO.
>>>>
>>>> Hmm, I don't think so:
>>>>
>>>> - If you have INVARIANT, the kernel will panic in
tcp_twstart() or
>>>> tcp_detach() and you will have everything you need to debug.
>>>> - If you don't, dtrace is the right tool to use in all
cases anyway.
>>>
>>> dtrace don't executed in may case w/ diagnostic "dtrace:
processing
>>> aborted: Abort due to systemic unresponsiveness". This is for
>>> tcp_close. May be tcp_twstart will be more successuful, may be not.
>>
>> It does and will.
>>
>>> Also, using dtrace too complex in production (need complex startup
>>> under screen and capture output) and for many peoples.
>>> kdb_backtrace() have too less administrative overhead.
>>
>> I still think it is overkill. The main goal of this change is to fix
a
>> quite tricky and old TCP stack locking issue. Let's try to do that
>> first, it is complex enough by itself.
>>
>> Once the fix is validated and pushed, feel free to propose your own
>> patch/review to add kdb_backtrace(), log(), etc.. to get other devs
>> point of view.
>>
>> I don't remember who said: "Never ever optimize error
cases"...
>
> This is not optimeze error cases, this is error recovery and
> diagnostic of error cases in other subsystems.
Sure, I guess this quote is more geared toward: "Always spend 50x more
time on improving the main path than the error path".
> Currently FreeBSD internals too complex for just always trust on
> correct of other subsystem or do panic on any incosystency.
>
> INVARIANTS too expensive now (20Gbit drops to 8Gbits).
I do agree. I am not expert enough to see all the side effects of
calling kdb_backtrace() from the TCP stack, might be way too slow,
tricky in interruption context, etc. You can see that kdb_backtrace()
is rarely called in the kernel source. That's why it is better if you
propose a review on adding this line to get comments from other devs on
just this question.
> PS: I am applay patch. Wait till monday.
>
> Thanks very match for this hard work!
No problem, thanks for your time. But it is not over yet: We have to
wait for final test.
--
Julien
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20161014/12c2ec02/attachment.sig>