On 2/7/24 02:15, Jens Axboe wrote:
On 2/6/24 5:57 PM, Pavel Begunkov wrote:
On 2/6/24 16:22, Jens Axboe wrote:
This adds a flag to avoid dipping dereferencing file and then f_op
to figure out if the file has a poll handler defined or not. We
generally call this at least twice for networked workloads.
Sends are not using poll every time. For recv, we touch it
in io_arm_poll_handler(), which is done only once, and so
ammortised to 0 for multishots.
Correct
Looking at the patch, the second time we might care about is
in io_ring_buffer_select(), but I'd argue that it shouldn't
be there in the first place. It's fragile, and I don't see
why selected buffers would care specifically about polling
but not asking more generally "can it go true async"? For
reads you might want to also test FMODE_BUF_RASYNC.
That is indeed the second case that is hit, and I don't think we can
easily get around that which is the reason for the hint.
Also note that when called from recv we already know that
it's pollable, it might be much easier to pass it in as an
argument.
I did think about that, but I don't see a clean way to do it. We could
potentially do it as an issue flag, but that seems kind of ugly to me.
Open to suggestions!
I'd argue passing it as an argument is much much cleaner
and more robust design wise, those leaked abstractions are
always fragile and unreliable. And now there is an argument
that it's even faster because for recv you can just pass "true".
IOW, I'd prefer here potentially a slightly uglier but safer
code.
Surely it'd have been be great to move this "eject buffer"
thing out of the selection func and let the caller decide,
but I haven't stared at the code for long enough to say
anything concrete.
--
Pavel Begunkov
