--- On 2024-10-10 15:35 Pavel Begunkov wrote: >> Two questions: >> >> 1. I agree with you, we shouldn't walk a potentially very >> long list under spinlock. but i can't find any other way >> to get all the timeout > If only it's just under the spin, but with disabled irqs... >> information than to walk the timeout_list. Do you have any >> good ideas? > In the long run it'd be great to replace the spinlock > with a mutex, i.e. just ->uring_lock, but that would might be > a bit involving as need to move handling to the task context. Yes, it makes more sense to replace spin_lock, but that would require other related logic to be modified, and I don't think it's wise to do that for the sake of a piece of debugging information. >> 2. I also agree seq_printf heavier, if we use >> seq_put_decimal_ull and seq_puts to concatenate strings, >> I haven't tested whether it's more efficient or not, but >> the code is certainly not as readable as the former. It's >> also possible that I don't fully understand what you mean >> and want to hear your opinion. > 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. --- Ruyi Zhang
