From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
To: virtualization@lists.linux.dev
Cc: "Richard Weinberger" <richard@nod.at>,
"Anton Ivanov" <anton.ivanov@cambridgegreys.com>,
"Johannes Berg" <johannes@sipsolutions.net>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Xuan Zhuo" <xuanzhuo@linux.alibaba.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Hans de Goede" <hdegoede@redhat.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Vadim Pasternak" <vadimp@nvidia.com>,
"Bjorn Andersson" <andersson@kernel.org>,
"Mathieu Poirier" <mathieu.poirier@linaro.org>,
"Cornelia Huck" <cohuck@redhat.com>,
"Halil Pasic" <pasic@linux.ibm.com>,
"Eric Farman" <farman@linux.ibm.com>,
"Heiko Carstens" <hca@linux.ibm.com>,
"Vasily Gorbik" <gor@linux.ibm.com>,
"Alexander Gordeev" <agordeev@linux.ibm.com>,
"Christian Borntraeger" <borntraeger@linux.ibm.com>,
"Sven Schnelle" <svens@linux.ibm.com>,
"Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Jesper Dangaard Brouer" <hawk@kernel.org>,
"John Fastabend" <john.fastabend@gmail.com>,
"Benjamin Berg" <benjamin.berg@intel.com>,
"Yang Li" <yang.lee@linux.alibaba.com>,
linux-um@lists.infradead.org, netdev@vger.kernel.org,
platform-driver-x86@vger.kernel.org,
linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org,
kvm@vger.kernel.org, bpf@vger.kernel.org
Subject: [PATCH 02/14] virtio_ring: packed: remove double check of the unmap ops
Date: Tue, 30 Jan 2024 11:05:52 +0800 [thread overview]
Message-ID: <20240130030604.108463-3-xuanzhuo@linux.alibaba.com> (raw)
In-Reply-To: <20240130030604.108463-1-xuanzhuo@linux.alibaba.com>
In the functions vring_unmap_extra_packed and vring_unmap_desc_packed,
multiple checks are made whether unmap is performed and whether it is
INDIRECT.
These two functions are usually called in a loop, and we should put the
check outside the loop.
And we unmap the descs with VRING_DESC_F_INDIRECT on the same path with
other descs, that make the thing more complex. If we distinguish the
descs with VRING_DESC_F_INDIRECT before unmap, thing will be clearer.
1. only one desc of the desc table is used, we do not need the loop
2. the called unmap api is difference from the other desc
3. the vq->premapped is not needed to check
4. the vq->indirect is not needed to check
5. the state->indir_desc must not be null
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
drivers/virtio/virtio_ring.c | 76 ++++++++++++++++++------------------
1 file changed, 39 insertions(+), 37 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 4677831e6c26..7280a1706cca 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1220,6 +1220,7 @@ static u16 packed_last_used(u16 last_used_idx)
return last_used_idx & ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR));
}
+/* caller must check vring_need_unmap_buffer() */
static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
const struct vring_desc_extra *extra)
{
@@ -1227,33 +1228,18 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
flags = extra->flags;
- if (flags & VRING_DESC_F_INDIRECT) {
- if (!vq->use_dma_api)
- return;
-
- dma_unmap_single(vring_dma_dev(vq),
- extra->addr, extra->len,
- (flags & VRING_DESC_F_WRITE) ?
- DMA_FROM_DEVICE : DMA_TO_DEVICE);
- } else {
- if (!vring_need_unmap_buffer(vq))
- return;
-
- dma_unmap_page(vring_dma_dev(vq),
- extra->addr, extra->len,
- (flags & VRING_DESC_F_WRITE) ?
- DMA_FROM_DEVICE : DMA_TO_DEVICE);
- }
+ dma_unmap_page(vring_dma_dev(vq),
+ extra->addr, extra->len,
+ (flags & VRING_DESC_F_WRITE) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE);
}
+/* caller must check vring_need_unmap_buffer() */
static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
const struct vring_packed_desc *desc)
{
u16 flags;
- if (!vring_need_unmap_buffer(vq))
- return;
-
flags = le16_to_cpu(desc->flags);
dma_unmap_page(vring_dma_dev(vq),
@@ -1329,7 +1315,7 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
total_sg * sizeof(struct vring_packed_desc),
DMA_TO_DEVICE);
if (vring_mapping_error(vq, addr)) {
- if (vq->premapped)
+ if (!vring_need_unmap_buffer(vq))
goto free_desc;
goto unmap_release;
@@ -1344,10 +1330,11 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
vq->packed.desc_extra[id].addr = addr;
vq->packed.desc_extra[id].len = total_sg *
sizeof(struct vring_packed_desc);
- vq->packed.desc_extra[id].flags = VRING_DESC_F_INDIRECT |
- vq->packed.avail_used_flags;
}
+ vq->packed.desc_extra[id].flags = VRING_DESC_F_INDIRECT |
+ vq->packed.avail_used_flags;
+
/*
* A driver MUST NOT make the first descriptor in the list
* available before all subsequent descriptors comprising
@@ -1388,6 +1375,8 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
unmap_release:
err_idx = i;
+ WARN_ON(!vring_need_unmap_buffer(vq));
+
for (i = 0; i < err_idx; i++)
vring_unmap_desc_packed(vq, &desc[i]);
@@ -1484,9 +1473,10 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
if (unlikely(vring_need_unmap_buffer(vq))) {
vq->packed.desc_extra[curr].addr = addr;
vq->packed.desc_extra[curr].len = sg->length;
- vq->packed.desc_extra[curr].flags =
- le16_to_cpu(flags);
}
+
+ vq->packed.desc_extra[curr].flags = le16_to_cpu(flags);
+
prev = curr;
curr = vq->packed.desc_extra[curr].next;
@@ -1536,6 +1526,8 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
vq->packed.avail_used_flags = avail_used_flags;
+ WARN_ON(!vring_need_unmap_buffer(vq));
+
for (n = 0; n < total_sg; n++) {
if (i == err_idx)
break;
@@ -1605,7 +1597,9 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
struct vring_desc_state_packed *state = NULL;
struct vring_packed_desc *desc;
unsigned int i, curr;
+ u16 flags;
+ flags = vq->packed.desc_extra[id].flags;
state = &vq->packed.desc_state[id];
/* Clear data ptr. */
@@ -1615,22 +1609,32 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
vq->free_head = id;
vq->vq.num_free += state->num;
- if (unlikely(vring_need_unmap_buffer(vq))) {
- curr = id;
- for (i = 0; i < state->num; i++) {
- vring_unmap_extra_packed(vq,
- &vq->packed.desc_extra[curr]);
- curr = vq->packed.desc_extra[curr].next;
+ if (!(flags & VRING_DESC_F_INDIRECT)) {
+ if (vring_need_unmap_buffer(vq)) {
+ curr = id;
+ for (i = 0; i < state->num; i++) {
+ vring_unmap_extra_packed(vq,
+ &vq->packed.desc_extra[curr]);
+ curr = vq->packed.desc_extra[curr].next;
+ }
}
- }
- if (vq->indirect) {
+ if (ctx)
+ *ctx = state->indir_desc;
+ } else {
+ const struct vring_desc_extra *extra;
u32 len;
+ if (vq->use_dma_api) {
+ extra = &vq->packed.desc_extra[id];
+ dma_unmap_single(vring_dma_dev(vq),
+ extra->addr, extra->len,
+ (flags & VRING_DESC_F_WRITE) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ }
+
/* Free the indirect table, if any, now that it's unmapped. */
desc = state->indir_desc;
- if (!desc)
- return;
if (vring_need_unmap_buffer(vq)) {
len = vq->packed.desc_extra[id].len;
@@ -1640,8 +1644,6 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
}
kfree(desc);
state->indir_desc = NULL;
- } else if (ctx) {
- *ctx = state->indir_desc;
}
}
--
2.32.0.3.g01195cf9f
next prev parent reply other threads:[~2024-01-30 3:06 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 3:05 [PATCH vhost 00/14] virtio remove dma info for premapped mode Xuan Zhuo
2024-01-30 3:05 ` [PATCH 01/14] virtio_ring: introduce vring_need_unmap_buffer Xuan Zhuo
2024-01-30 3:05 ` Xuan Zhuo [this message]
2024-01-30 3:05 ` [PATCH 03/14] virtio_ring: packed: structure the indirect desc table Xuan Zhuo
2024-01-30 3:05 ` [PATCH 04/14] virtio_ring: split: remove double check of the unmap ops Xuan Zhuo
2024-01-30 3:05 ` [PATCH 05/14] virtio_ring: split: structure the indirect desc table Xuan Zhuo
2024-01-30 3:05 ` [PATCH 06/14] virtio_ring: no store dma info when unmap is not needed Xuan Zhuo
2024-01-30 3:05 ` [PATCH 07/14] virtio_ring: introduce dma map api for page Xuan Zhuo
2024-01-30 3:05 ` [PATCH 08/14] virtio: find_vqs introduce premapped parameter Xuan Zhuo
2024-01-30 3:05 ` [PATCH 09/14] virtio_ring: export premapped to driver by struct virtqueue Xuan Zhuo
2024-01-30 3:06 ` [PATCH 10/14] virtio_net: set premapped mode by find_vqs() Xuan Zhuo
2024-01-30 3:06 ` [PATCH 11/14] virtio_ring: remove api of setting vq premapped Xuan Zhuo
2024-01-30 3:06 ` [PATCH 12/14] virtio_net: unify the code for recycling the xmit ptr Xuan Zhuo
2024-01-30 3:06 ` [PATCH 13/14] virtio_net: rename free_old_xmit_skbs to free_old_xmit Xuan Zhuo
2024-01-30 3:06 ` [PATCH 14/14] virtio_net: sq support premapped mode Xuan Zhuo
2024-01-30 11:43 ` [PATCH vhost 00/14] virtio remove dma info for " Xuan Zhuo
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=20240130030604.108463-3-xuanzhuo@linux.alibaba.com \
--to=xuanzhuo@linux.alibaba.com \
--cc=agordeev@linux.ibm.com \
--cc=andersson@kernel.org \
--cc=anton.ivanov@cambridgegreys.com \
--cc=ast@kernel.org \
--cc=benjamin.berg@intel.com \
--cc=borntraeger@linux.ibm.com \
--cc=bpf@vger.kernel.org \
--cc=cohuck@redhat.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=farman@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hawk@kernel.org \
--cc=hca@linux.ibm.com \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jasowang@redhat.com \
--cc=johannes@sipsolutions.net \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-um@lists.infradead.org \
--cc=mathieu.poirier@linaro.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pasic@linux.ibm.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=richard@nod.at \
--cc=svens@linux.ibm.com \
--cc=vadimp@nvidia.com \
--cc=virtualization@lists.linux.dev \
--cc=yang.lee@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 read-only IMAP folder(s) and NNTP newsgroup(s).