From: Jason Wang <jasowang@redhat.com>
To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Jesper Dangaard Brouer <hawk@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
virtualization@lists.linux-foundation.org, bpf@vger.kernel.org
Subject: Re: [PATCH net-next v3 06/27] virtio_ring: introduce virtqueue_get_buf_ctx_dma()
Date: Mon, 22 Jan 2024 14:54:39 +0800 [thread overview]
Message-ID: <CACGkMEsYs3zKVNxzDMtAHZKAUEFppxBvWb0LMGDWVMwQqvX83Q@mail.gmail.com> (raw)
In-Reply-To: <1705903444.5368986-1-xuanzhuo@linux.alibaba.com>
On Mon, Jan 22, 2024 at 2:12 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Mon, 22 Jan 2024 12:18:51 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Tue, Jan 16, 2024 at 3:47 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > On Thu, 11 Jan 2024 16:34:09 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > On Fri, Dec 29, 2023 at 3:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> 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.
> > > > >
> > > > > Because the BAD_RING is called (that may set vq->broken), so
> > > > > the relative "const" of vq is removed.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > ---
> > > > > drivers/virtio/virtio_ring.c | 174 +++++++++++++++++++++++++----------
> > > > > include/linux/virtio.h | 16 ++++
> > > > > 2 files changed, 142 insertions(+), 48 deletions(-)
> > > > >
> > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > > index 51d8f3299c10..1374b3fd447c 100644
> > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > @@ -362,6 +362,45 @@ static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> > > > > return vq->dma_dev;
> > > > > }
> > > > >
> > > > > +/*
> > > > > + * use_dma_api premapped -> do_unmap
> > > > > + * 1. false false false
> > > > > + * 2. true false true
> > > > > + * 3. true true false
> > > > > + *
> > > > > + * Only #3, we should return the DMA info to the driver.
> > > >
> > > > Btw, I guess you meant "#3 is false" here?
> > > >
> > > > And could we reduce the size of these 3 * 3 matrices? It's usually a
> > > > hint that the code is not optmized.
> > >
> > > On the process of doing dma map, we force the (use_dma_api, premapped).
> > >
> > > if premapped:
> > > virtio core skip dma map
> > > else:
> > > if use_dma_api:
> > > do dma map
> > > else:
> > > work with the physical address.
> > >
> > > Here we force the (premapped, do_unmap).
> > >
> > > do_unmap is an optimization. We just check this to know should we do dma unmap
> > > or not.
> > >
> > > Now, we introduced an new case, when the virtio core skip dma unmap,
> > > we may need to return the dma info to the driver. That just occur when
> > > the (premapped, do_unmap) is (true, false). Because that the (premmaped,
> > > do_unmap) may be (false, false).
> > >
> > > For the matrices, I just want to show where the do_unmap comes from.
> > > That is a optimization, we use this many places, not to check (use_dma_api,
> > > premapped) on the process of doing unmap. And only for the case #3, we should
> > > return the dma info to drivers.
> >
> > Ok, it tries to ease the life of the readers.
> >
> > I wonder if something like
> >
> > bool virtqueue_needs_unmap() can help, it can judge based on the value
> > of use_dma_api and premapped.
>
>
> I think not too much.
>
> Because do_unmap is for this.
>
>
>
> +static bool vring_need_unmap(struct vring_virtqueue *vq,
> + struct virtio_dma_head *dma,
> + dma_addr_t addr, unsigned int length)
> +{
> + if (vq->do_unmap)
> + return true;
>
> Before this, we is to judge whether we should do unmap or not.
> After this, we is to judge whehter we should return dma info to driver or not.
>
> If you want to simplify this function, I will say no.
>
> If you want to replace "do_unmap" with virtqueue_needs_unmap(), I will say ok.
That's my point.
> But I think we donot need to do that.
Just a suggestion, and you can move the comment above there.
Thanks
>
> +
> + if (!vq->premapped)
> + return false;
> +
> + if (!dma)
> + return false;
> +
> + if (unlikely(dma->next >= dma->num)) {
> + BAD_RING(vq, "premapped vq: collect dma overflow: %pad %u\n",
> + &addr, length);
> + return false;
> + }
> +
> + dma->items[dma->next].addr = addr;
> + dma->items[dma->next].length = length;
> +
> + ++dma->next;
> +
> + return false;
> +}
>
>
> Thanks.
>
>
> >
> > Thanks
> >
> > >
> > > Thanks.
> > >
> > > >
> > > > Thanks
> > > >
> > > >
> > >
> >
>
next prev parent reply other threads:[~2024-01-22 6:54 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-29 7:30 [PATCH net-next v3 00/27] virtio-net: support AF_XDP zero copy Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 01/27] virtio_net: rename free_old_xmit_skbs to free_old_xmit Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 02/27] virtio_net: unify the code for recycling the xmit ptr Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 03/27] virtio_net: independent directory Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 04/27] virtio_net: move core structures to virtio_net.h Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 05/27] virtio_net: add prefix virtnet to all struct inside virtio_net.h Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 06/27] virtio_ring: introduce virtqueue_get_buf_ctx_dma() Xuan Zhuo
2024-01-11 8:34 ` Jason Wang
2024-01-16 7:32 ` Xuan Zhuo
2024-01-22 4:18 ` Jason Wang
2024-01-22 6:04 ` Xuan Zhuo
2024-01-22 6:54 ` Jason Wang [this message]
2023-12-29 7:30 ` [PATCH net-next v3 07/27] virtio_ring: virtqueue_disable_and_recycle let the callback detach bufs Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 08/27] virtio_ring: introduce virtqueue_detach_unused_buf_dma() Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 09/27] virtio_ring: introduce virtqueue_get_dma_premapped() Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 10/27] virtio_net: sq support premapped mode Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 11/27] virtio_net: separate virtnet_rx_resize() Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 12/27] virtio_net: separate virtnet_tx_resize() Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 13/27] virtio_net: xsk: bind/unbind xsk Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 14/27] virtio_net: xsk: prevent disable tx napi Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 15/27] virtio_net: move some api to header Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 16/27] virtio_net: xsk: tx: support xmit xsk buffer Xuan Zhuo
2023-12-30 0:28 ` kernel test robot
2023-12-29 7:30 ` [PATCH net-next v3 17/27] virtio_net: xsk: tx: support wakeup Xuan Zhuo
2023-12-29 7:30 ` [PATCH net-next v3 18/27] virtio_net: xsk: tx: handle the transmitted xsk buffer Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 19/27] virtio_net: xsk: tx: free the unused " Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 20/27] virtio_net: separate receive_mergeable Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 21/27] virtio_net: separate receive_buf Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 22/27] virtio_net: xsk: rx: support fill with xsk buffer Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 23/27] virtio_net: xsk: rx: support recv merge mode Xuan Zhuo
2023-12-29 21:03 ` kernel test robot
2023-12-30 1:01 ` kernel test robot
2023-12-29 7:31 ` [PATCH net-next v3 24/27] virtio_net: xsk: rx: support recv small mode Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 25/27] virtio_net: xsk: rx: free the unused xsk buffer Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 26/27] virtio_net: update tx timeout record Xuan Zhuo
2023-12-29 7:31 ` [PATCH net-next v3 27/27] virtio_net: xdp_features add NETDEV_XDP_ACT_XSK_ZEROCOPY Xuan Zhuo
2024-01-11 3:27 ` [PATCH net-next v3 00/27] virtio-net: support AF_XDP zero copy Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACGkMEsYs3zKVNxzDMtAHZKAUEFppxBvWb0LMGDWVMwQqvX83Q@mail.gmail.com \
--to=jasowang@redhat.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).