mbox series

[v3,0/3] napi tracking strategy

Message ID cover.1726589775.git.olivier@trillion01.com (mailing list archive)
Headers show
Series napi tracking strategy | expand

Message

Olivier Langlois Sept. 18, 2024, 12:59 p.m. UTC
the actual napi tracking strategy is inducing a non-negligeable overhead.
Everytime a multishot poll is triggered or any poll armed, if the napi is
enabled on the ring a lookup is performed to either add a new napi id into
the napi_list or its timeout value is updated.

For many scenarios, this is overkill as the napi id list will be pretty
much static most of the time. To address this common scenario, a new
abstraction has been created following the common Linux kernel idiom of
creating an abstract interface with a struct filled with function pointers.

the patch serie consist of very minor fixes followed by the core of the changes
to implement the new feature.

v3 changes:
- address minor comments in patch #3
- replace the double for loop hash macro with the single loop list macro to iterate the napi elements in patch #2
- add __cold attribute to common_tracking_show_fdinfo() and napi_show_fdinfo()

v2 changes:
- extract small changes from the core changes to ease minor fixes backport
- totally remove the io_napi_tracking_ops interface

Olivier Langlois (3):
  io_uring/napi: protect concurrent io_napi_entry timeout accesses
  io_uring/napi: fix io_napi_entry RCU accesses
  io_uring/napi: add static napi tracking strategy

 include/linux/io_uring_types.h |   2 +-
 include/uapi/linux/io_uring.h  |  32 +++++++-
 io_uring/fdinfo.c              |  41 ++++++++++
 io_uring/napi.c                | 140 +++++++++++++++++++++++++--------
 io_uring/napi.h                |  15 +++-
 5 files changed, 192 insertions(+), 38 deletions(-)

Comments

Jens Axboe Sept. 19, 2024, 2:36 a.m. UTC | #1
On 9/18/24 6:59 AM, Olivier Langlois wrote:
> the actual napi tracking strategy is inducing a non-negligeable overhead.
> Everytime a multishot poll is triggered or any poll armed, if the napi is
> enabled on the ring a lookup is performed to either add a new napi id into
> the napi_list or its timeout value is updated.
> 
> For many scenarios, this is overkill as the napi id list will be pretty
> much static most of the time. To address this common scenario, a new
> abstraction has been created following the common Linux kernel idiom of
> creating an abstract interface with a struct filled with function pointers.

This paragraph seems to be a remnant of the v1 implementation?
Olivier Langlois Sept. 21, 2024, 1:32 p.m. UTC | #2
On Wed, 2024-09-18 at 20:36 -0600, Jens Axboe wrote:
> On 9/18/24 6:59 AM, Olivier Langlois wrote:
> > the actual napi tracking strategy is inducing a non-negligeable
> > overhead.
> > Everytime a multishot poll is triggered or any poll armed, if the
> > napi is
> > enabled on the ring a lookup is performed to either add a new napi
> > id into
> > the napi_list or its timeout value is updated.
> > 
> > For many scenarios, this is overkill as the napi id list will be
> > pretty
> > much static most of the time. To address this common scenario, a
> > new
> > abstraction has been created following the common Linux kernel
> > idiom of
> > creating an abstract interface with a struct filled with function
> > pointers.
> 
> This paragraph seems to be a remnant of the v1 implementation?
> 
you are right. At least the last sentence is. Is the cover letter
injected somewhere in any way once the patch is accepted? Would that
detail alone justify the creation of a v4?

I am taking note of the detail and I will correct this if there is a
need for a v4.
Jens Axboe Sept. 21, 2024, 1:46 p.m. UTC | #3
On 9/21/24 7:32 AM, Olivier Langlois wrote:
> On Wed, 2024-09-18 at 20:36 -0600, Jens Axboe wrote:
>> On 9/18/24 6:59 AM, Olivier Langlois wrote:
>>> the actual napi tracking strategy is inducing a non-negligeable
>>> overhead.
>>> Everytime a multishot poll is triggered or any poll armed, if the
>>> napi is
>>> enabled on the ring a lookup is performed to either add a new napi
>>> id into
>>> the napi_list or its timeout value is updated.
>>>
>>> For many scenarios, this is overkill as the napi id list will be
>>> pretty
>>> much static most of the time. To address this common scenario, a
>>> new
>>> abstraction has been created following the common Linux kernel
>>> idiom of
>>> creating an abstract interface with a struct filled with function
>>> pointers.
>>
>> This paragraph seems to be a remnant of the v1 implementation?
>>
> you are right. At least the last sentence is. Is the cover letter
> injected somewhere in any way once the patch is accepted? Would that
> detail alone justify the creation of a v4?
> 
> I am taking note of the detail and I will correct this if there is a
> need for a v4.

Right just make a note of it if you need to send a v4 for other reasons,
cover letter doesn't go anywhere in the git log - but is, however,
linked off the git commits in terms of the Link in there. But since we
now have this discussion in there as well clarifying it, that is fine.

Doing conferences in Europe until end of next week, I'll see if I can do
some thorough looking at this version during that, or worst case end of
that.