On Wed, Nov 06, 2024 at 08:15:13AM -0700, Jens Axboe wrote:
> On 11/6/24 5:26 AM, Ming Lei wrote:
> > Prepare for supporting kernel buffer in case of io group, in which group
> > leader leases kernel buffer to io_uring, and consumed by io_uring OPs.
> >
> > So reuse io_mapped_buf for group kernel buffer, and unfortunately
> > io_import_fixed() can't be reused since userspace fixed buffer is
> > virt-contiguous, but it isn't true for kernel buffer.
> >
> > Also kernel buffer lifetime is bound with group leader request, it isn't
> > necessary to use rsrc_node for tracking its lifetime, especially it needs
> > extra allocation of rsrc_node for each IO.
>
> While it isn't strictly necessary, I do think it'd clean up the io_kiocb
> parts and hopefully unify the assign and put path more. So I'd strongly
> suggest you do use an io_rsrc_node, even if it does just map the
> io_mapped_buf for this.
Can you share your idea about how to unify buffer? I am also interested
in this area, so I may take it into account in this patch.
Will you plan to use io_rsrc_node for all buffer type(include buffer
select)?
>
> > +struct io_mapped_buf {
> > + u64 start;
> > + unsigned int len;
> > + unsigned int nr_bvecs;
> > +
> > + /* kbuf hasn't refs and accounting, its lifetime is bound with req */
> > + union {
> > + struct {
> > + refcount_t refs;
> > + unsigned int acct_pages;
> > + };
> > + /* pbvec is only for kbuf */
> > + const struct bio_vec *pbvec;
> > + };
> > + unsigned int folio_shift:6;
> > + unsigned int dir:1; /* ITER_DEST or ITER_SOURCE */
> > + unsigned int kbuf:1; /* kernel buffer or not */
> > + /* offset in the 1st bvec, for kbuf only */
> > + unsigned int offset;
> > + struct bio_vec bvec[] __counted_by(nr_bvecs);
> > +};
>
> And then I'd get rid of this union, and have it follow the normal rules
> for an io_mapped_buf in that the refs are valid. Yes it'll take 8b more,
> but honestly I think unifying these bits and keeping it consistent is a
> LOT more important than saving a bit of space.
>
> This is imho the last piece missing to make this conform more nicely
> with how resource nodes are generally handled and used.
OK.
thanks,
Ming
