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.


Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to