public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Pavel Begunkov <[email protected]>,
	Ruyi Zhang <[email protected]>
Cc: [email protected], [email protected],
	[email protected]
Subject: Re: [PATCH v2 RESEND] io_uring/fdinfo: add timeout_list to fdinfo
Date: Wed, 30 Oct 2024 07:26:10 -0600	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 10/29/24 7:29 PM, Pavel Begunkov wrote:
> On 10/25/24 00:25, Jens Axboe wrote:
>> On 10/24/24 12:10 PM, Pavel Begunkov wrote:
>>> On 10/24/24 18:31, Jens Axboe wrote:
>>>> On Sat, Oct 12, 2024 at 3:30?AM Ruyi Zhang <[email protected]> wrote:
>>> ...
>>>>>> I don't think there is any difference, it'd be a matter of
>>>>>> doubling the number of in flight timeouts to achieve same
>>>>>> timings. Tell me, do you really have a good case where you
>>>>>> need that (pretty verbose)? Why not drgn / bpftrace it out
>>>>>> of the kernel instead?
>>>>>
>>>>>    Of course, this information is available through existing tools.
>>>>>    But I think that most of the io_uring metadata has been exported
>>>>>    from the fdinfo file, and the purpose of adding the timeout
>>>>>    information is the same as before, easier to use. This way,
>>>>>    I don't have to write additional scripts to get all kinds of data.
>>>>>
>>>>>    And as far as I know, the io_uring_show_fdinfo function is
>>>>>    only called once when the user is viewing the
>>>>>    /proc/xxx/fdinfo/x file once. I don't think we normally need to
>>>>>    look at this file as often, and only look at it when the program
>>>>>    is abnormal, and the timeout_list is very long in the extreme case,
>>>>>    so I think the performance impact of adding this code is limited.
>>>>
>>>> I do think it's useful, sometimes the only thing you have to poke at
>>>> after-the-fact is the fdinfo information. At the same time, would it be
>>>
>>> If you have an fd to print fdinfo, you can just well run drgn
>>> or any other debugging tool. We keep pushing more debugging code
>>> that can be extracted with bpf and other tools, and not only
>>> it bloats the code, but potentially cripples the entire kernel.
>>
>> While that is certainly true, it's also a much harder barrier to entry.
>> If you're already setup with eg drgn, then yeah fdinfo is useless as you
>> can grab much more info out by just using drgn.
> 
> drgn is simple, not that harder than patching fdinfo, we can add
> liburing/scripts, and push it there so that don't need rewriting
> it each time.

It's not that drgn it's hard to use, it's not, but that people aren't
necessarily aware of it. Once you've used it, yeah it's trivial. But for
the cases where you are stuck in prod and you haven't used anything like
that, it's a bit of a stretch to get there. Once it's part of your usual
arsenal of tools, not an issue at all.

Adding something to liburing/scripts/ would indeed be awesome.

>> I'm fine punting this to "needs more advanced debugging than fdinfo".
>> It's just important we get closure on these patches, so they don't
>> linger forever in no man's land.
> 
> The only option I see is to dump first ~5 and stop there, but
> I still think the tooling option is better.

Let's just not do it at all, I think a partial dump is likely to be
potentially useless. And you can't cat it again and expect something
different if things are stuck.

-- 
Jens Axboe

      reply	other threads:[~2024-10-30 13:26 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20240925085815epcas5p16fa977581284a81dae7b67da8bc96a85@epcas5p1.samsung.com>
2024-09-25  8:58 ` [PATCH v2 RESEND] io_uring/fdinfo: add timeout_list to fdinfo Ruyi Zhang
2024-09-25 11:58   ` Pavel Begunkov
     [not found]     ` <CGME20241010092012epcas5p2bc333a1f880209003523e71d97ba3298@epcas5p2.samsung.com>
2024-10-10  9:20       ` Ruyi Zhang
2024-10-10 15:35         ` Pavel Begunkov
     [not found]           ` <CGME20241012091032epcas5p2dec0e3db5a72854f4566b251791b84ad@epcas5p2.samsung.com>
2024-10-12  9:10             ` Ruyi Zhang
2024-10-24 17:31               ` Jens Axboe
2024-10-24 18:10                 ` Pavel Begunkov
2024-10-24 23:25                   ` Jens Axboe
2024-10-30  1:29                     ` Pavel Begunkov
2024-10-30 13:26                       ` Jens Axboe [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox