diff mbox series

[net-next,4/5] virtio_ring: introduce virtqueue_get_dma_premapped()

Message ID 20240116075924.42798-5-xuanzhuo@linux.alibaba.com (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series virtio-net: sq support premapped mode | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/apply fail Patch does not apply to net-next

Commit Message

Xuan Zhuo Jan. 16, 2024, 7:59 a.m. UTC
Introduce helper virtqueue_get_dma_premapped(), then the driver
can know whether dma unmap is needed.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio/main.c       | 22 +++++++++-------------
 drivers/net/virtio/virtio_net.h |  3 ---
 drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
 include/linux/virtio.h          |  1 +
 4 files changed, 32 insertions(+), 16 deletions(-)

Comments

Jason Wang Jan. 25, 2024, 3:39 a.m. UTC | #1
On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> Introduce helper virtqueue_get_dma_premapped(), then the driver
> can know whether dma unmap is needed.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  drivers/net/virtio/main.c       | 22 +++++++++-------------
>  drivers/net/virtio/virtio_net.h |  3 ---
>  drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
>  include/linux/virtio.h          |  1 +
>  4 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> index 186b2cf5d8fc..4fbf612da235 100644
> --- a/drivers/net/virtio/main.c
> +++ b/drivers/net/virtio/main.c
> @@ -483,7 +483,7 @@ static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
>         void *buf;
>
>         buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> -       if (buf && rq->do_dma)
> +       if (buf && virtqueue_get_dma_premapped(rq->vq))
>                 virtnet_rq_unmap(rq, buf, *len);
>
>         return buf;
> @@ -496,7 +496,7 @@ static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
>         u32 offset;
>         void *head;
>
> -       if (!rq->do_dma) {
> +       if (!virtqueue_get_dma_premapped(rq->vq)) {
>                 sg_init_one(rq->sg, buf, len);
>                 return;
>         }
> @@ -526,7 +526,7 @@ static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)
>
>         head = page_address(alloc_frag->page);
>
> -       if (rq->do_dma) {
> +       if (virtqueue_get_dma_premapped(rq->vq)) {
>                 dma = head;
>
>                 /* new pages */
> @@ -580,12 +580,8 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
>         if (!vi->mergeable_rx_bufs && vi->big_packets)
>                 return;
>
> -       for (i = 0; i < vi->max_queue_pairs; i++) {
> -               if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> -                       continue;
> -
> -               vi->rq[i].do_dma = true;
> -       }
> +       for (i = 0; i < vi->max_queue_pairs; i++)
> +               virtqueue_set_dma_premapped(vi->rq[i].vq);
>  }
>
>  static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)
> @@ -1643,7 +1639,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,
>
>         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
>         if (err < 0) {
> -               if (rq->do_dma)
> +               if (virtqueue_get_dma_premapped(rq->vq))
>                         virtnet_rq_unmap(rq, buf, 0);
>                 put_page(virt_to_head_page(buf));
>         }
> @@ -1758,7 +1754,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
>         ctx = mergeable_len_to_ctx(len + room, headroom);
>         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
>         if (err < 0) {
> -               if (rq->do_dma)
> +               if (virtqueue_get_dma_premapped(rq->vq))
>                         virtnet_rq_unmap(rq, buf, 0);
>                 put_page(virt_to_head_page(buf));
>         }
> @@ -4007,7 +4003,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
>         int i;
>         for (i = 0; i < vi->max_queue_pairs; i++)
>                 if (vi->rq[i].alloc_frag.page) {
> -                       if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> +                       if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
>                                 virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
>                         put_page(vi->rq[i].alloc_frag.page);
>                 }
> @@ -4035,7 +4031,7 @@ static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
>         rq = &vi->rq[i];
>
>         while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> -               if (rq->do_dma)
> +               if (virtqueue_get_dma_premapped(rq->vq))
>                         virtnet_rq_unmap(rq, buf, 0);
>
>                 virtnet_rq_free_buf(vi, rq, buf);
> diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> index b28a4d0a3150..066a2b9d2b3c 100644
> --- a/drivers/net/virtio/virtio_net.h
> +++ b/drivers/net/virtio/virtio_net.h
> @@ -115,9 +115,6 @@ struct virtnet_rq {
>
>         /* Record the last dma info to free after new pages is allocated. */
>         struct virtnet_rq_dma *last_dma;
> -
> -       /* Do dma by self */
> -       bool do_dma;
>  };
>
>  struct virtnet_info {
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 2c5089d3b510..9092bcdebb53 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -2905,6 +2905,28 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);
>
> +/**
> + * virtqueue_get_dma_premapped - get the vring premapped mode
> + * @_vq: the struct virtqueue we're talking about.
> + *
> + * Get the premapped mode of the vq.
> + *
> + * Returns bool for the vq premapped mode.
> + */
> +bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
> +{
> +       struct vring_virtqueue *vq = to_vvq(_vq);
> +       bool premapped;
> +
> +       START_USE(vq);
> +       premapped = vq->premapped;
> +       END_USE(vq);

Why do we need to protect premapped like this? Is the user allowed to
change it on the fly?

Thanks
Xuan Zhuo Jan. 25, 2024, 5:57 a.m. UTC | #2
On Thu, 25 Jan 2024 11:39:03 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > Introduce helper virtqueue_get_dma_premapped(), then the driver
> > can know whether dma unmap is needed.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  drivers/net/virtio/main.c       | 22 +++++++++-------------
> >  drivers/net/virtio/virtio_net.h |  3 ---
> >  drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
> >  include/linux/virtio.h          |  1 +
> >  4 files changed, 32 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > index 186b2cf5d8fc..4fbf612da235 100644
> > --- a/drivers/net/virtio/main.c
> > +++ b/drivers/net/virtio/main.c
> > @@ -483,7 +483,7 @@ static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
> >         void *buf;
> >
> >         buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > -       if (buf && rq->do_dma)
> > +       if (buf && virtqueue_get_dma_premapped(rq->vq))
> >                 virtnet_rq_unmap(rq, buf, *len);
> >
> >         return buf;
> > @@ -496,7 +496,7 @@ static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
> >         u32 offset;
> >         void *head;
> >
> > -       if (!rq->do_dma) {
> > +       if (!virtqueue_get_dma_premapped(rq->vq)) {
> >                 sg_init_one(rq->sg, buf, len);
> >                 return;
> >         }
> > @@ -526,7 +526,7 @@ static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)
> >
> >         head = page_address(alloc_frag->page);
> >
> > -       if (rq->do_dma) {
> > +       if (virtqueue_get_dma_premapped(rq->vq)) {
> >                 dma = head;
> >
> >                 /* new pages */
> > @@ -580,12 +580,8 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
> >         if (!vi->mergeable_rx_bufs && vi->big_packets)
> >                 return;
> >
> > -       for (i = 0; i < vi->max_queue_pairs; i++) {
> > -               if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> > -                       continue;
> > -
> > -               vi->rq[i].do_dma = true;
> > -       }
> > +       for (i = 0; i < vi->max_queue_pairs; i++)
> > +               virtqueue_set_dma_premapped(vi->rq[i].vq);
> >  }
> >
> >  static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)
> > @@ -1643,7 +1639,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,
> >
> >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> >         if (err < 0) {
> > -               if (rq->do_dma)
> > +               if (virtqueue_get_dma_premapped(rq->vq))
> >                         virtnet_rq_unmap(rq, buf, 0);
> >                 put_page(virt_to_head_page(buf));
> >         }
> > @@ -1758,7 +1754,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> >         ctx = mergeable_len_to_ctx(len + room, headroom);
> >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> >         if (err < 0) {
> > -               if (rq->do_dma)
> > +               if (virtqueue_get_dma_premapped(rq->vq))
> >                         virtnet_rq_unmap(rq, buf, 0);
> >                 put_page(virt_to_head_page(buf));
> >         }
> > @@ -4007,7 +4003,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
> >         int i;
> >         for (i = 0; i < vi->max_queue_pairs; i++)
> >                 if (vi->rq[i].alloc_frag.page) {
> > -                       if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> > +                       if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
> >                                 virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
> >                         put_page(vi->rq[i].alloc_frag.page);
> >                 }
> > @@ -4035,7 +4031,7 @@ static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
> >         rq = &vi->rq[i];
> >
> >         while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> > -               if (rq->do_dma)
> > +               if (virtqueue_get_dma_premapped(rq->vq))
> >                         virtnet_rq_unmap(rq, buf, 0);
> >
> >                 virtnet_rq_free_buf(vi, rq, buf);
> > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > index b28a4d0a3150..066a2b9d2b3c 100644
> > --- a/drivers/net/virtio/virtio_net.h
> > +++ b/drivers/net/virtio/virtio_net.h
> > @@ -115,9 +115,6 @@ struct virtnet_rq {
> >
> >         /* Record the last dma info to free after new pages is allocated. */
> >         struct virtnet_rq_dma *last_dma;
> > -
> > -       /* Do dma by self */
> > -       bool do_dma;
> >  };
> >
> >  struct virtnet_info {
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 2c5089d3b510..9092bcdebb53 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -2905,6 +2905,28 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> >  }
> >  EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);
> >
> > +/**
> > + * virtqueue_get_dma_premapped - get the vring premapped mode
> > + * @_vq: the struct virtqueue we're talking about.
> > + *
> > + * Get the premapped mode of the vq.
> > + *
> > + * Returns bool for the vq premapped mode.
> > + */
> > +bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
> > +{
> > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > +       bool premapped;
> > +
> > +       START_USE(vq);
> > +       premapped = vq->premapped;
> > +       END_USE(vq);
>
> Why do we need to protect premapped like this? Is the user allowed to
> change it on the fly?


Just protect before accessing vq.

Thanks.
>
> Thanks
>
Jason Wang Jan. 29, 2024, 3:07 a.m. UTC | #3
On Thu, Jan 25, 2024 at 1:58 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Thu, 25 Jan 2024 11:39:03 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > Introduce helper virtqueue_get_dma_premapped(), then the driver
> > > can know whether dma unmap is needed.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > ---
> > >  drivers/net/virtio/main.c       | 22 +++++++++-------------
> > >  drivers/net/virtio/virtio_net.h |  3 ---
> > >  drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
> > >  include/linux/virtio.h          |  1 +
> > >  4 files changed, 32 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > > index 186b2cf5d8fc..4fbf612da235 100644
> > > --- a/drivers/net/virtio/main.c
> > > +++ b/drivers/net/virtio/main.c
> > > @@ -483,7 +483,7 @@ static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
> > >         void *buf;
> > >
> > >         buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > > -       if (buf && rq->do_dma)
> > > +       if (buf && virtqueue_get_dma_premapped(rq->vq))
> > >                 virtnet_rq_unmap(rq, buf, *len);
> > >
> > >         return buf;
> > > @@ -496,7 +496,7 @@ static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
> > >         u32 offset;
> > >         void *head;
> > >
> > > -       if (!rq->do_dma) {
> > > +       if (!virtqueue_get_dma_premapped(rq->vq)) {
> > >                 sg_init_one(rq->sg, buf, len);
> > >                 return;
> > >         }
> > > @@ -526,7 +526,7 @@ static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)
> > >
> > >         head = page_address(alloc_frag->page);
> > >
> > > -       if (rq->do_dma) {
> > > +       if (virtqueue_get_dma_premapped(rq->vq)) {
> > >                 dma = head;
> > >
> > >                 /* new pages */
> > > @@ -580,12 +580,8 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
> > >         if (!vi->mergeable_rx_bufs && vi->big_packets)
> > >                 return;
> > >
> > > -       for (i = 0; i < vi->max_queue_pairs; i++) {
> > > -               if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> > > -                       continue;
> > > -
> > > -               vi->rq[i].do_dma = true;
> > > -       }
> > > +       for (i = 0; i < vi->max_queue_pairs; i++)
> > > +               virtqueue_set_dma_premapped(vi->rq[i].vq);
> > >  }
> > >
> > >  static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)
> > > @@ -1643,7 +1639,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,
> > >
> > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > >         if (err < 0) {
> > > -               if (rq->do_dma)
> > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > >                         virtnet_rq_unmap(rq, buf, 0);
> > >                 put_page(virt_to_head_page(buf));
> > >         }
> > > @@ -1758,7 +1754,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> > >         ctx = mergeable_len_to_ctx(len + room, headroom);
> > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > >         if (err < 0) {
> > > -               if (rq->do_dma)
> > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > >                         virtnet_rq_unmap(rq, buf, 0);
> > >                 put_page(virt_to_head_page(buf));
> > >         }
> > > @@ -4007,7 +4003,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
> > >         int i;
> > >         for (i = 0; i < vi->max_queue_pairs; i++)
> > >                 if (vi->rq[i].alloc_frag.page) {
> > > -                       if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> > > +                       if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
> > >                                 virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
> > >                         put_page(vi->rq[i].alloc_frag.page);
> > >                 }
> > > @@ -4035,7 +4031,7 @@ static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
> > >         rq = &vi->rq[i];
> > >
> > >         while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> > > -               if (rq->do_dma)
> > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > >                         virtnet_rq_unmap(rq, buf, 0);
> > >
> > >                 virtnet_rq_free_buf(vi, rq, buf);
> > > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > > index b28a4d0a3150..066a2b9d2b3c 100644
> > > --- a/drivers/net/virtio/virtio_net.h
> > > +++ b/drivers/net/virtio/virtio_net.h
> > > @@ -115,9 +115,6 @@ struct virtnet_rq {
> > >
> > >         /* Record the last dma info to free after new pages is allocated. */
> > >         struct virtnet_rq_dma *last_dma;
> > > -
> > > -       /* Do dma by self */
> > > -       bool do_dma;
> > >  };
> > >
> > >  struct virtnet_info {
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index 2c5089d3b510..9092bcdebb53 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -2905,6 +2905,28 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> > >  }
> > >  EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);
> > >
> > > +/**
> > > + * virtqueue_get_dma_premapped - get the vring premapped mode
> > > + * @_vq: the struct virtqueue we're talking about.
> > > + *
> > > + * Get the premapped mode of the vq.
> > > + *
> > > + * Returns bool for the vq premapped mode.
> > > + */
> > > +bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
> > > +{
> > > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > > +       bool premapped;
> > > +
> > > +       START_USE(vq);
> > > +       premapped = vq->premapped;
> > > +       END_USE(vq);
> >
> > Why do we need to protect premapped like this? Is the user allowed to
> > change it on the fly?
>
>
> Just protect before accessing vq.

I meant how did that differ from other booleans? E.g use_dma_api, do_unmap etc.

Thanks

>
> Thanks.
> >
> > Thanks
> >
>
Xuan Zhuo Jan. 29, 2024, 3:30 a.m. UTC | #4
On Mon, 29 Jan 2024 11:07:50 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Thu, Jan 25, 2024 at 1:58 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Thu, 25 Jan 2024 11:39:03 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > >
> > > > Introduce helper virtqueue_get_dma_premapped(), then the driver
> > > > can know whether dma unmap is needed.
> > > >
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > ---
> > > >  drivers/net/virtio/main.c       | 22 +++++++++-------------
> > > >  drivers/net/virtio/virtio_net.h |  3 ---
> > > >  drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
> > > >  include/linux/virtio.h          |  1 +
> > > >  4 files changed, 32 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > > > index 186b2cf5d8fc..4fbf612da235 100644
> > > > --- a/drivers/net/virtio/main.c
> > > > +++ b/drivers/net/virtio/main.c
> > > > @@ -483,7 +483,7 @@ static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
> > > >         void *buf;
> > > >
> > > >         buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > > > -       if (buf && rq->do_dma)
> > > > +       if (buf && virtqueue_get_dma_premapped(rq->vq))
> > > >                 virtnet_rq_unmap(rq, buf, *len);
> > > >
> > > >         return buf;
> > > > @@ -496,7 +496,7 @@ static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
> > > >         u32 offset;
> > > >         void *head;
> > > >
> > > > -       if (!rq->do_dma) {
> > > > +       if (!virtqueue_get_dma_premapped(rq->vq)) {
> > > >                 sg_init_one(rq->sg, buf, len);
> > > >                 return;
> > > >         }
> > > > @@ -526,7 +526,7 @@ static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)
> > > >
> > > >         head = page_address(alloc_frag->page);
> > > >
> > > > -       if (rq->do_dma) {
> > > > +       if (virtqueue_get_dma_premapped(rq->vq)) {
> > > >                 dma = head;
> > > >
> > > >                 /* new pages */
> > > > @@ -580,12 +580,8 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
> > > >         if (!vi->mergeable_rx_bufs && vi->big_packets)
> > > >                 return;
> > > >
> > > > -       for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > -               if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> > > > -                       continue;
> > > > -
> > > > -               vi->rq[i].do_dma = true;
> > > > -       }
> > > > +       for (i = 0; i < vi->max_queue_pairs; i++)
> > > > +               virtqueue_set_dma_premapped(vi->rq[i].vq);
> > > >  }
> > > >
> > > >  static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)
> > > > @@ -1643,7 +1639,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,
> > > >
> > > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > > >         if (err < 0) {
> > > > -               if (rq->do_dma)
> > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > >                 put_page(virt_to_head_page(buf));
> > > >         }
> > > > @@ -1758,7 +1754,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> > > >         ctx = mergeable_len_to_ctx(len + room, headroom);
> > > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > > >         if (err < 0) {
> > > > -               if (rq->do_dma)
> > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > >                 put_page(virt_to_head_page(buf));
> > > >         }
> > > > @@ -4007,7 +4003,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
> > > >         int i;
> > > >         for (i = 0; i < vi->max_queue_pairs; i++)
> > > >                 if (vi->rq[i].alloc_frag.page) {
> > > > -                       if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> > > > +                       if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
> > > >                                 virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
> > > >                         put_page(vi->rq[i].alloc_frag.page);
> > > >                 }
> > > > @@ -4035,7 +4031,7 @@ static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
> > > >         rq = &vi->rq[i];
> > > >
> > > >         while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> > > > -               if (rq->do_dma)
> > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > >
> > > >                 virtnet_rq_free_buf(vi, rq, buf);
> > > > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > > > index b28a4d0a3150..066a2b9d2b3c 100644
> > > > --- a/drivers/net/virtio/virtio_net.h
> > > > +++ b/drivers/net/virtio/virtio_net.h
> > > > @@ -115,9 +115,6 @@ struct virtnet_rq {
> > > >
> > > >         /* Record the last dma info to free after new pages is allocated. */
> > > >         struct virtnet_rq_dma *last_dma;
> > > > -
> > > > -       /* Do dma by self */
> > > > -       bool do_dma;
> > > >  };
> > > >
> > > >  struct virtnet_info {
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index 2c5089d3b510..9092bcdebb53 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -2905,6 +2905,28 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);
> > > >
> > > > +/**
> > > > + * virtqueue_get_dma_premapped - get the vring premapped mode
> > > > + * @_vq: the struct virtqueue we're talking about.
> > > > + *
> > > > + * Get the premapped mode of the vq.
> > > > + *
> > > > + * Returns bool for the vq premapped mode.
> > > > + */
> > > > +bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
> > > > +{
> > > > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > > > +       bool premapped;
> > > > +
> > > > +       START_USE(vq);
> > > > +       premapped = vq->premapped;
> > > > +       END_USE(vq);
> > >
> > > Why do we need to protect premapped like this? Is the user allowed to
> > > change it on the fly?
> >
> >
> > Just protect before accessing vq.
>
> I meant how did that differ from other booleans? E.g use_dma_api, do_unmap etc.

Sorry, maybe I misunderstanded you.

Do you mean, should we put "premapped" to the struct virtqueue?
Then the user can read/write by the struct virtqueue directly?

If that, the reason is that when set premapped, we must check
use_dma_api.


int virtqueue_set_dma_premapped(struct virtqueue *_vq)
{
	struct vring_virtqueue *vq = to_vvq(_vq);
	u32 num;

	START_USE(vq);

	num = vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;

	if (num != vq->vq.num_free) {
		END_USE(vq);
		return -EINVAL;
	}

	if (!vq->use_dma_api) {
		END_USE(vq);
		return -EINVAL;
	}

	vq->buffer_is_premapped = true;

	END_USE(vq);

	return 0;
}

Thanks.

>
> Thanks
>
> >
> > Thanks.
> > >
> > > Thanks
> > >
> >
>
Jason Wang Jan. 30, 2024, 2:54 a.m. UTC | #5
On Mon, Jan 29, 2024 at 11:33 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Mon, 29 Jan 2024 11:07:50 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Thu, Jan 25, 2024 at 1:58 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > On Thu, 25 Jan 2024 11:39:03 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > >
> > > > > Introduce helper virtqueue_get_dma_premapped(), then the driver
> > > > > can know whether dma unmap is needed.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > ---
> > > > >  drivers/net/virtio/main.c       | 22 +++++++++-------------
> > > > >  drivers/net/virtio/virtio_net.h |  3 ---
> > > > >  drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
> > > > >  include/linux/virtio.h          |  1 +
> > > > >  4 files changed, 32 insertions(+), 16 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > > > > index 186b2cf5d8fc..4fbf612da235 100644
> > > > > --- a/drivers/net/virtio/main.c
> > > > > +++ b/drivers/net/virtio/main.c
> > > > > @@ -483,7 +483,7 @@ static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
> > > > >         void *buf;
> > > > >
> > > > >         buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > > > > -       if (buf && rq->do_dma)
> > > > > +       if (buf && virtqueue_get_dma_premapped(rq->vq))
> > > > >                 virtnet_rq_unmap(rq, buf, *len);
> > > > >
> > > > >         return buf;
> > > > > @@ -496,7 +496,7 @@ static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
> > > > >         u32 offset;
> > > > >         void *head;
> > > > >
> > > > > -       if (!rq->do_dma) {
> > > > > +       if (!virtqueue_get_dma_premapped(rq->vq)) {
> > > > >                 sg_init_one(rq->sg, buf, len);
> > > > >                 return;
> > > > >         }
> > > > > @@ -526,7 +526,7 @@ static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)
> > > > >
> > > > >         head = page_address(alloc_frag->page);
> > > > >
> > > > > -       if (rq->do_dma) {
> > > > > +       if (virtqueue_get_dma_premapped(rq->vq)) {
> > > > >                 dma = head;
> > > > >
> > > > >                 /* new pages */
> > > > > @@ -580,12 +580,8 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
> > > > >         if (!vi->mergeable_rx_bufs && vi->big_packets)
> > > > >                 return;
> > > > >
> > > > > -       for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > > -               if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> > > > > -                       continue;
> > > > > -
> > > > > -               vi->rq[i].do_dma = true;
> > > > > -       }
> > > > > +       for (i = 0; i < vi->max_queue_pairs; i++)
> > > > > +               virtqueue_set_dma_premapped(vi->rq[i].vq);
> > > > >  }
> > > > >
> > > > >  static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)
> > > > > @@ -1643,7 +1639,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,
> > > > >
> > > > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > > > >         if (err < 0) {
> > > > > -               if (rq->do_dma)
> > > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > > >                 put_page(virt_to_head_page(buf));
> > > > >         }
> > > > > @@ -1758,7 +1754,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> > > > >         ctx = mergeable_len_to_ctx(len + room, headroom);
> > > > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > > > >         if (err < 0) {
> > > > > -               if (rq->do_dma)
> > > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > > >                 put_page(virt_to_head_page(buf));
> > > > >         }
> > > > > @@ -4007,7 +4003,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
> > > > >         int i;
> > > > >         for (i = 0; i < vi->max_queue_pairs; i++)
> > > > >                 if (vi->rq[i].alloc_frag.page) {
> > > > > -                       if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> > > > > +                       if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
> > > > >                                 virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
> > > > >                         put_page(vi->rq[i].alloc_frag.page);
> > > > >                 }
> > > > > @@ -4035,7 +4031,7 @@ static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
> > > > >         rq = &vi->rq[i];
> > > > >
> > > > >         while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> > > > > -               if (rq->do_dma)
> > > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > > >
> > > > >                 virtnet_rq_free_buf(vi, rq, buf);
> > > > > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > > > > index b28a4d0a3150..066a2b9d2b3c 100644
> > > > > --- a/drivers/net/virtio/virtio_net.h
> > > > > +++ b/drivers/net/virtio/virtio_net.h
> > > > > @@ -115,9 +115,6 @@ struct virtnet_rq {
> > > > >
> > > > >         /* Record the last dma info to free after new pages is allocated. */
> > > > >         struct virtnet_rq_dma *last_dma;
> > > > > -
> > > > > -       /* Do dma by self */
> > > > > -       bool do_dma;
> > > > >  };
> > > > >
> > > > >  struct virtnet_info {
> > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > > index 2c5089d3b510..9092bcdebb53 100644
> > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > @@ -2905,6 +2905,28 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> > > > >  }
> > > > >  EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);
> > > > >
> > > > > +/**
> > > > > + * virtqueue_get_dma_premapped - get the vring premapped mode
> > > > > + * @_vq: the struct virtqueue we're talking about.
> > > > > + *
> > > > > + * Get the premapped mode of the vq.
> > > > > + *
> > > > > + * Returns bool for the vq premapped mode.
> > > > > + */
> > > > > +bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
> > > > > +{
> > > > > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > > > > +       bool premapped;
> > > > > +
> > > > > +       START_USE(vq);
> > > > > +       premapped = vq->premapped;
> > > > > +       END_USE(vq);
> > > >
> > > > Why do we need to protect premapped like this? Is the user allowed to
> > > > change it on the fly?
> > >
> > >
> > > Just protect before accessing vq.
> >
> > I meant how did that differ from other booleans? E.g use_dma_api, do_unmap etc.
>
> Sorry, maybe I misunderstanded you.
>
> Do you mean, should we put "premapped" to the struct virtqueue?
> Then the user can read/write by the struct virtqueue directly?
>
> If that, the reason is that when set premapped, we must check
> use_dma_api.

I may not be very clear.

I meant why we should protect premapped with START_USE()/END_USER() here.

If it is set once during init_vqs we should not need that.

Thanks
Xuan Zhuo Jan. 30, 2024, 3:13 a.m. UTC | #6
On Tue, 30 Jan 2024 10:54:25 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Mon, Jan 29, 2024 at 11:33 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Mon, 29 Jan 2024 11:07:50 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > On Thu, Jan 25, 2024 at 1:58 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > >
> > > > On Thu, 25 Jan 2024 11:39:03 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > > On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > >
> > > > > > Introduce helper virtqueue_get_dma_premapped(), then the driver
> > > > > > can know whether dma unmap is needed.
> > > > > >
> > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > > ---
> > > > > >  drivers/net/virtio/main.c       | 22 +++++++++-------------
> > > > > >  drivers/net/virtio/virtio_net.h |  3 ---
> > > > > >  drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
> > > > > >  include/linux/virtio.h          |  1 +
> > > > > >  4 files changed, 32 insertions(+), 16 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > > > > > index 186b2cf5d8fc..4fbf612da235 100644
> > > > > > --- a/drivers/net/virtio/main.c
> > > > > > +++ b/drivers/net/virtio/main.c
> > > > > > @@ -483,7 +483,7 @@ static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
> > > > > >         void *buf;
> > > > > >
> > > > > >         buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > > > > > -       if (buf && rq->do_dma)
> > > > > > +       if (buf && virtqueue_get_dma_premapped(rq->vq))
> > > > > >                 virtnet_rq_unmap(rq, buf, *len);
> > > > > >
> > > > > >         return buf;
> > > > > > @@ -496,7 +496,7 @@ static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
> > > > > >         u32 offset;
> > > > > >         void *head;
> > > > > >
> > > > > > -       if (!rq->do_dma) {
> > > > > > +       if (!virtqueue_get_dma_premapped(rq->vq)) {
> > > > > >                 sg_init_one(rq->sg, buf, len);
> > > > > >                 return;
> > > > > >         }
> > > > > > @@ -526,7 +526,7 @@ static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)
> > > > > >
> > > > > >         head = page_address(alloc_frag->page);
> > > > > >
> > > > > > -       if (rq->do_dma) {
> > > > > > +       if (virtqueue_get_dma_premapped(rq->vq)) {
> > > > > >                 dma = head;
> > > > > >
> > > > > >                 /* new pages */
> > > > > > @@ -580,12 +580,8 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
> > > > > >         if (!vi->mergeable_rx_bufs && vi->big_packets)
> > > > > >                 return;
> > > > > >
> > > > > > -       for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > > > -               if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> > > > > > -                       continue;
> > > > > > -
> > > > > > -               vi->rq[i].do_dma = true;
> > > > > > -       }
> > > > > > +       for (i = 0; i < vi->max_queue_pairs; i++)
> > > > > > +               virtqueue_set_dma_premapped(vi->rq[i].vq);
> > > > > >  }
> > > > > >
> > > > > >  static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)
> > > > > > @@ -1643,7 +1639,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,
> > > > > >
> > > > > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > > > > >         if (err < 0) {
> > > > > > -               if (rq->do_dma)
> > > > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > > > >                 put_page(virt_to_head_page(buf));
> > > > > >         }
> > > > > > @@ -1758,7 +1754,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> > > > > >         ctx = mergeable_len_to_ctx(len + room, headroom);
> > > > > >         err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > > > > >         if (err < 0) {
> > > > > > -               if (rq->do_dma)
> > > > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > > > >                 put_page(virt_to_head_page(buf));
> > > > > >         }
> > > > > > @@ -4007,7 +4003,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
> > > > > >         int i;
> > > > > >         for (i = 0; i < vi->max_queue_pairs; i++)
> > > > > >                 if (vi->rq[i].alloc_frag.page) {
> > > > > > -                       if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> > > > > > +                       if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
> > > > > >                                 virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
> > > > > >                         put_page(vi->rq[i].alloc_frag.page);
> > > > > >                 }
> > > > > > @@ -4035,7 +4031,7 @@ static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
> > > > > >         rq = &vi->rq[i];
> > > > > >
> > > > > >         while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> > > > > > -               if (rq->do_dma)
> > > > > > +               if (virtqueue_get_dma_premapped(rq->vq))
> > > > > >                         virtnet_rq_unmap(rq, buf, 0);
> > > > > >
> > > > > >                 virtnet_rq_free_buf(vi, rq, buf);
> > > > > > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > > > > > index b28a4d0a3150..066a2b9d2b3c 100644
> > > > > > --- a/drivers/net/virtio/virtio_net.h
> > > > > > +++ b/drivers/net/virtio/virtio_net.h
> > > > > > @@ -115,9 +115,6 @@ struct virtnet_rq {
> > > > > >
> > > > > >         /* Record the last dma info to free after new pages is allocated. */
> > > > > >         struct virtnet_rq_dma *last_dma;
> > > > > > -
> > > > > > -       /* Do dma by self */
> > > > > > -       bool do_dma;
> > > > > >  };
> > > > > >
> > > > > >  struct virtnet_info {
> > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > > > index 2c5089d3b510..9092bcdebb53 100644
> > > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > > @@ -2905,6 +2905,28 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> > > > > >  }
> > > > > >  EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);
> > > > > >
> > > > > > +/**
> > > > > > + * virtqueue_get_dma_premapped - get the vring premapped mode
> > > > > > + * @_vq: the struct virtqueue we're talking about.
> > > > > > + *
> > > > > > + * Get the premapped mode of the vq.
> > > > > > + *
> > > > > > + * Returns bool for the vq premapped mode.
> > > > > > + */
> > > > > > +bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
> > > > > > +{
> > > > > > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > > > > > +       bool premapped;
> > > > > > +
> > > > > > +       START_USE(vq);
> > > > > > +       premapped = vq->premapped;
> > > > > > +       END_USE(vq);
> > > > >
> > > > > Why do we need to protect premapped like this? Is the user allowed to
> > > > > change it on the fly?
> > > >
> > > >
> > > > Just protect before accessing vq.
> > >
> > > I meant how did that differ from other booleans? E.g use_dma_api, do_unmap etc.
> >
> > Sorry, maybe I misunderstanded you.
> >
> > Do you mean, should we put "premapped" to the struct virtqueue?
> > Then the user can read/write by the struct virtqueue directly?
> >
> > If that, the reason is that when set premapped, we must check
> > use_dma_api.
>
> I may not be very clear.
>
> I meant why we should protect premapped with START_USE()/END_USER() here.
>
> If it is set once during init_vqs we should not need that.


OK. I see.

Thanks.


>
> Thanks
>
diff mbox series

Patch

diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
index 186b2cf5d8fc..4fbf612da235 100644
--- a/drivers/net/virtio/main.c
+++ b/drivers/net/virtio/main.c
@@ -483,7 +483,7 @@  static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
 	void *buf;
 
 	buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
-	if (buf && rq->do_dma)
+	if (buf && virtqueue_get_dma_premapped(rq->vq))
 		virtnet_rq_unmap(rq, buf, *len);
 
 	return buf;
@@ -496,7 +496,7 @@  static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
 	u32 offset;
 	void *head;
 
-	if (!rq->do_dma) {
+	if (!virtqueue_get_dma_premapped(rq->vq)) {
 		sg_init_one(rq->sg, buf, len);
 		return;
 	}
@@ -526,7 +526,7 @@  static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)
 
 	head = page_address(alloc_frag->page);
 
-	if (rq->do_dma) {
+	if (virtqueue_get_dma_premapped(rq->vq)) {
 		dma = head;
 
 		/* new pages */
@@ -580,12 +580,8 @@  static void virtnet_rq_set_premapped(struct virtnet_info *vi)
 	if (!vi->mergeable_rx_bufs && vi->big_packets)
 		return;
 
-	for (i = 0; i < vi->max_queue_pairs; i++) {
-		if (virtqueue_set_dma_premapped(vi->rq[i].vq))
-			continue;
-
-		vi->rq[i].do_dma = true;
-	}
+	for (i = 0; i < vi->max_queue_pairs; i++)
+		virtqueue_set_dma_premapped(vi->rq[i].vq);
 }
 
 static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)
@@ -1643,7 +1639,7 @@  static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,
 
 	err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
 	if (err < 0) {
-		if (rq->do_dma)
+		if (virtqueue_get_dma_premapped(rq->vq))
 			virtnet_rq_unmap(rq, buf, 0);
 		put_page(virt_to_head_page(buf));
 	}
@@ -1758,7 +1754,7 @@  static int add_recvbuf_mergeable(struct virtnet_info *vi,
 	ctx = mergeable_len_to_ctx(len + room, headroom);
 	err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
 	if (err < 0) {
-		if (rq->do_dma)
+		if (virtqueue_get_dma_premapped(rq->vq))
 			virtnet_rq_unmap(rq, buf, 0);
 		put_page(virt_to_head_page(buf));
 	}
@@ -4007,7 +4003,7 @@  static void free_receive_page_frags(struct virtnet_info *vi)
 	int i;
 	for (i = 0; i < vi->max_queue_pairs; i++)
 		if (vi->rq[i].alloc_frag.page) {
-			if (vi->rq[i].do_dma && vi->rq[i].last_dma)
+			if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
 				virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
 			put_page(vi->rq[i].alloc_frag.page);
 		}
@@ -4035,7 +4031,7 @@  static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
 	rq = &vi->rq[i];
 
 	while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
-		if (rq->do_dma)
+		if (virtqueue_get_dma_premapped(rq->vq))
 			virtnet_rq_unmap(rq, buf, 0);
 
 		virtnet_rq_free_buf(vi, rq, buf);
diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
index b28a4d0a3150..066a2b9d2b3c 100644
--- a/drivers/net/virtio/virtio_net.h
+++ b/drivers/net/virtio/virtio_net.h
@@ -115,9 +115,6 @@  struct virtnet_rq {
 
 	/* Record the last dma info to free after new pages is allocated. */
 	struct virtnet_rq_dma *last_dma;
-
-	/* Do dma by self */
-	bool do_dma;
 };
 
 struct virtnet_info {
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 2c5089d3b510..9092bcdebb53 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2905,6 +2905,28 @@  int virtqueue_set_dma_premapped(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);
 
+/**
+ * virtqueue_get_dma_premapped - get the vring premapped mode
+ * @_vq: the struct virtqueue we're talking about.
+ *
+ * Get the premapped mode of the vq.
+ *
+ * Returns bool for the vq premapped mode.
+ */
+bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	bool premapped;
+
+	START_USE(vq);
+	premapped = vq->premapped;
+	END_USE(vq);
+
+	return premapped;
+
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_dma_premapped);
+
 /**
  * virtqueue_reset - detach and recycle all unused buffers
  * @_vq: the struct virtqueue we're talking about.
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 2596f0e7e395..3e9a2bb75af6 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -98,6 +98,7 @@  bool virtqueue_enable_cb(struct virtqueue *vq);
 unsigned virtqueue_enable_cb_prepare(struct virtqueue *vq);
 
 int virtqueue_set_dma_premapped(struct virtqueue *_vq);
+bool virtqueue_get_dma_premapped(struct virtqueue *_vq);
 
 bool virtqueue_poll(struct virtqueue *vq, unsigned);