On 4/24/23 3:55?PM, Jens Axboe wrote: > On 4/24/23 3:37?PM, Linus Torvalds wrote: >> On Mon, Apr 24, 2023 at 2:22?PM Jens Axboe <axboe@xxxxxxxxx> wrote: >>> >>> If we don't ever wait for IO with the pipe lock held, then we can skip >>> the conditional locking. But with splice, that's not at all the case! We >>> most certainly wait for IO there with the pipe lock held. >> >> I think that then needs to just be fixed. > > I took another look at this, and the main issue is in fact splice > confirming buffers. So I do think that we can make this work by simply > having the non-block nature of it being passed down the ->confirm() > callback as that's the one that'll be waiting for IO. If we have that, > then we can disregard the pipe locking as we won't be holding it over > IO. > > Which is what part of this series does, notably patch 1. > > Only other oddity is pipe_to_sendpage(), which we can probably sanely > ignore. > > IOW, would you be fine with a v2 of this pull request where patch 2 > drops the conditional locking and just passes it to ->confirm()? That's > certainly sane, and just makes the ultimate page locking conditional to > avoid waiting on IO. I'd really hate to still be missing out on pipe > performance with io_uring. I guess that would still have blocking if you have someone doing splice in a blocking fashion, and someone else trying to do RWF_NOWAIT reads or writes to the pipe... The very thing the conditional pipe locking would sort out. Only fool proof alternative would seem to be having splice use a specific pipe lock rather then pipe->mutex. And honestly pipes and splice are so tied together than I'm not sure that doing separate locking would be feasible. -- Jens Axboe
