On Sat, 2022-03-26 at 13:06 -0700, Jakub Kicinski wrote: > On Sat, 26 Mar 2022 13:47:24 -0600 Jens Axboe wrote: > > > Which constants are you referring to? Only odd one I see is > > NAPI_TIMEOUT, other ones are using the sysctl bits. If we're > > missing something here, do speak up and we'll make sure it's > > consistent with the regular NAPI. > > SO_BUSY_POLL_BUDGET, 8 is quite low for many practical uses. > I'd also like to have a conversation about continuing to use > the socket as a proxy for NAPI_ID, NAPI_ID is exposed to user > space now. io_uring being a new interface I wonder if it's not > better to let the user specify the request parameters directly. > > My napi busy poll integration is strongly inspired from epoll code as its persistent context is much closer to the io_uring situation than what select/poll code is doing. For instance, BUSY_POLL_BUDGET constant is taken straight from epoll code. I am a little bit surprised about your questioning. If BUSY_POLL_BUDGET is quite low for many practical uses, how is it ok to use for epoll code? If 8 is not a good default value, may I suggest that you change the define value? TBH, I didn't find the documentation about the busy poll budget parameter so I did assume that existing code was doing the right thing... For your other suggestion, I do not think that it is a good idea to let user specify the request napi ids to busy poll because it would make the io_uring interface more complex without being even sure that this is something that people want or need. select/poll implementation examine each and every sockets on every call and it can afford to do it since it is rebuilding the polling set every time through sock_poll(). epoll code does not want to do that as it would defeat its purpose and it relies on the busy poll global setting. Also, epoll code makes a pretty bold assumption that its users desiring busy polling will be willing to create an epoll set per receive queue and presumably run each set in a dedicated thread. In: https://legacy.netdevconf.info/2.1/slides/apr6/dumazet-BUSY-POLLING-Netdev-2.1.pdf Linux-4.12 changes: epoll() support was added by Sridhar Samudrala and Alexander Duyck, with the assumption that an application using epoll() and busy polling would first make sure that it would classify sockets based on their receive queue (NAPI ID), and use at least one epoll fd per receive queue. To me, this is a very big burden placed on the shoulders of their users as not every application design can accomodate this requirement. For instance, I have an Intel igb nic with 8 receive queues. I am not running my app on a 24 cores machine where I can easily allocate 8 threads just for the networking I/O. I sincerely feel that epoll busy poll integration has been specifically tailored for the patch authors needs without all the usability concerns that you appear to have for the io_uring implementation. I went beyond what epoll offers by allowing the busy polling of several receive queues from a single ring. When I did mention my interest in a napi busy poll to the io_uring list but I did not know how to proceed due to several unknown issues, Jens did encourage to give it a shot and in that context, my design goal has been to keep the initial implementation reasonable and simple. One concession that could be done to address your concern, it is that the socket receive queues added to the list of queues busy polled could be further narrowed by using sk_can_busy_loop() instead of just checking net_busy_loop_on(). Would that be a satisfactory compromise to you and your team?
