On Wed, Dec 6, 2023 at 3:46 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, 5 Dec 2023 15:12:03 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> > On Mon, Dec 4, 2023 at 3:23 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
> > >
> > > introduce virtqueue_get_buf_ctx_dma() to collect the dma info when
> > > get buf from virtio core for premapped mode.
> > >
> > > If the virtio queue is premapped mode, the virtio-net send buf may
> > > have many desc. Every desc dma address need to be unmap. So here we
> > > introduce a new helper to collect the dma address of the buffer from
> > > the virtio core.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/virtio/virtio_ring.c | 157 +++++++++++++++++++++++------------
> > > include/linux/virtio.h | 16 ++++
> > > 2 files changed, 121 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index 51d8f3299c10..103d181f05bb 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -362,6 +362,26 @@ static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> > > return vq->dma_dev;
> > > }
> > >
> > > +static void collect_dma_info(const struct vring_virtqueue *vq,
> >
> > Not a native speaker, but it looks to me like "get_dma_info" is better here.
>
> get_dma_info?
>
> This function collect the dma info to the dma array.
> Not to the dirver. So do you also think "get_dma_info" is better?
Not sure, it's not a must, you know I'm not a native speaker.
>
> >
> > > + struct virtio_dma_head *dma,
> > > + dma_addr_t addr, unsigned int length)
> > > +{
> > > + if (WARN_ON_ONCE(!dma))
> > > + return;
> >
> > So all the callers did:
> >
> > if (!vq->do_unmap)
> > collect_dma_info()
> >
> > It's a strong hint that we should move the vq->do_unmap inside
> > collect_dma_info()?
>
> No,
> vq->do_unmap affects the logic of caller.
>
> Such as:
>
> if (!vq->do_unmap) {
> collect_dma_info(vq, dma, le64_to_cpu(desc->addr), le32_to_cpu(desc->len));
> return;
> }
>
> So we should move it to this function.
I'm not sure I get here, but we can let collect_dma_info() return
boolean anyhow.
Thanks
