On 11/13/24 6:43 AM, Pavel Begunkov wrote: > On 11/12/24 00:53, Ming Lei wrote: >> On Thu, Nov 07, 2024 at 03:25:59PM -0700, Jens Axboe wrote: >>> On 11/7/24 3:25 PM, Jens Axboe wrote: > ... >> Hi Jens, >> >> Any comment on the rest of the series? > > Ming, it's dragging on because it's over complicated. I very much want > it to get to some conclusion, get it merged and move on, and I strongly > believe Jens shares the sentiment on getting the thing done. > > Please, take the patches attached, adjust them to your needs and put > ublk on top. Or tell if there is a strong reason why it doesn't work. > The implementation is very simple and doesn't need almost anything > from io_uring, it's low risk and we can merge in no time. Indeed, nobody would love to get this moving forward more than me! Pavel, can you setup a branch with the required patches? Should be your two and then the buf update and mapping bits I did earlier. I can do it too. Ming, would you mind if we try and setup a base that we can do this on more trivially? I had merged the sqe grouping, but the more I think about it, the less I like the added complexity, and the limitations we had to put in there because relationships weren't fully understandable. With the #1 goal of getting leased/borrowed buffers working asap, here's what I suggest: 1) Pavel/I provide a base for doing the bare minimum, which is having an ephemeral buffer that zc can use. 2) You do the ublk bits on top Yes this won't have grouping, so buf update will have to be done separately. The downside here is that it'll add a (tiny) bit of overhead as there's an extra sqe involved, but I don't really think that's an issue, not even a minor one. The main objective here is NOT copying the data, which will dwarf any other tiny extra overhead added. This avoids introducing sqe grouping as a concept as a requirement for zero copy ublk, which I did mention earlier is part of the complication here. I'm a BIG fan of keeping things simple initially, particularly when it adds major dependency complexity to the core code. The goal here is doing the simple buffer leasing in such a way that it's trivially understandable, and doesn't depend on grouping. With that, we can easily get this done for the 6.14 kernel and finally ship it. I wish 6.13 was a week more away because then I think we could get it in for 6.13, but we only really have a few days at this point, so it's a bit late. Unfortunately! Ming, what do you think? Let's get this sorted so we can move on to actually being able to use zc ublk, which is the goal we all share here. > If you can't cache the allocation in ublk, io_uring can add a cache. > If ublk needs more space and cannot embed the structure, we can add > a "private" pointer into io_mapped_ubuf. If it needs to check the IO > direction, we can add that as well (though I have doubts you really need > it, read-only might makes sense, write-only not so much). We'll also > merge Jens' patch allowing to remove a buffer with a request. Right, we can always improve the little things as we go forward and make it more efficient. I do think it's diminishing returns at that point, but that doesn't mean we should not do it and make it better. But first, let's get the concept working. That's just violent agreement btw, I think we all share that objective :-) -- Jens Axboe
