| Message ID | 20250211005646.222452-3-kbusch@meta.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ublk zero-copy support | expand |
On 2/11/25 00:56, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > When a registered resource is about to be freed, check if it has > registered a release function, and call it if so. This is preparing for > resources that are related to an external component. > > Signed-off-by: Keith Busch <kbusch@kernel.org> > --- > io_uring/rsrc.c | 2 ++ > io_uring/rsrc.h | 3 +++ > 2 files changed, 5 insertions(+) > > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c > index 4d0e1c06c8bc6..30f08cf13ef60 100644 > --- a/io_uring/rsrc.c > +++ b/io_uring/rsrc.c > @@ -455,6 +455,8 @@ void io_free_rsrc_node(struct io_ring_ctx *ctx, struct io_rsrc_node *node) > case IORING_RSRC_BUFFER: > if (node->buf) > io_buffer_unmap(ctx, node); > + if (node->release) > + node->release(node->priv); > break; > default: > WARN_ON_ONCE(1); > diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h > index abd0d5d42c3e1..a3826ab84e666 100644 > --- a/io_uring/rsrc.h > +++ b/io_uring/rsrc.h > @@ -24,6 +24,9 @@ struct io_rsrc_node { > unsigned long file_ptr; > struct io_mapped_ubuf *buf; > }; > + > + void (*release)(void *); > + void *priv; Nodes are more generic than buffers. Unless we want to reuse it for files, and I very much doubt that, it should move into io_mapped_ubuf. > }; > > struct io_mapped_ubuf {
On Thu, Feb 13, 2025 at 01:31:07AM +0000, Pavel Begunkov wrote: > On 2/11/25 00:56, Keith Busch wrote: > > @@ -24,6 +24,9 @@ struct io_rsrc_node { > > unsigned long file_ptr; > > struct io_mapped_ubuf *buf; > > }; > > + > > + void (*release)(void *); > > + void *priv; > > Nodes are more generic than buffers. Unless we want to reuse it > for files, and I very much doubt that, it should move into > io_mapped_ubuf. Good point. Cloning is another reason it should be in the imu. If we want to save space, the "KBUFFER" type doesn't need ubuf or folio_shift, so we could even union the new fields to prevent imu from getting any bigger. The downside would be we can't use "release" to distinguish kernel vs user buffers, which you suggested in that patch's thread. Which option do you prefer?
On 2/13/25 01:58, Keith Busch wrote: > On Thu, Feb 13, 2025 at 01:31:07AM +0000, Pavel Begunkov wrote: >> On 2/11/25 00:56, Keith Busch wrote: >>> @@ -24,6 +24,9 @@ struct io_rsrc_node { >>> unsigned long file_ptr; >>> struct io_mapped_ubuf *buf; >>> }; >>> + >>> + void (*release)(void *); >>> + void *priv; >> >> Nodes are more generic than buffers. Unless we want to reuse it >> for files, and I very much doubt that, it should move into >> io_mapped_ubuf. > > Good point. Cloning is another reason it should be in the imu. > > If we want to save space, the "KBUFFER" type doesn't need ubuf or > folio_shift, so we could even union the new fields to prevent imu from > getting any bigger. The downside would be we can't use "release" to > distinguish kernel vs user buffers, which you suggested in that patch's > thread. Which option do you prefer? I think we can just grow the struct for now, it's not a big problem, and follow up on that separately. Should make it easier for the series as well.
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c index 4d0e1c06c8bc6..30f08cf13ef60 100644 --- a/io_uring/rsrc.c +++ b/io_uring/rsrc.c @@ -455,6 +455,8 @@ void io_free_rsrc_node(struct io_ring_ctx *ctx, struct io_rsrc_node *node) case IORING_RSRC_BUFFER: if (node->buf) io_buffer_unmap(ctx, node); + if (node->release) + node->release(node->priv); break; default: WARN_ON_ONCE(1); diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h index abd0d5d42c3e1..a3826ab84e666 100644 --- a/io_uring/rsrc.h +++ b/io_uring/rsrc.h @@ -24,6 +24,9 @@ struct io_rsrc_node { unsigned long file_ptr; struct io_mapped_ubuf *buf; }; + + void (*release)(void *); + void *priv; }; struct io_mapped_ubuf {