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
signature.asc
Description: OpenPGP digital signature
