Le 07/11/2022 à 06:23, Arseniy Krasnov a écrit :
On 06.11.2022 22:50, Christophe JAILLET wrote:
Le 06/11/2022 à 20:36, Arseniy Krasnov a écrit :
To support zerocopy receive, packet's buffer allocation is changed: for
buffers which could be mapped to user's vma we can't use 'kmalloc()'(as
kernel restricts to map slab pages to user's vma) and raw buddy
allocator now called. But, for tx packets(such packets won't be mapped
to user), previous 'kmalloc()' way is used, but with special flag in
packet's structure which allows to distinguish between 'kmalloc()' and
raw pages buffers.
Signed-off-by: Arseniy Krasnov <AVKrasnov@xxxxxxxxxxxxxx>
---
drivers/vhost/vsock.c | 1 +
include/linux/virtio_vsock.h | 1 +
net/vmw_vsock/virtio_transport.c | 8 ++++++--
net/vmw_vsock/virtio_transport_common.c | 10 +++++++++-
4 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 5703775af129..65475d128a1d 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -399,6 +399,7 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq,
return NULL;
}
+ pkt->slab_buf = true;
pkt->buf_len = pkt->len;
nbytes = copy_from_iter(pkt->buf, pkt->len, &iov_iter);
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 35d7eedb5e8e..d02cb7aa922f 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -50,6 +50,7 @@ struct virtio_vsock_pkt {
u32 off;
bool reply;
bool tap_delivered;
+ bool slab_buf;
};
struct virtio_vsock_pkt_info {
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index ad64f403536a..19909c1e9ba3 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -255,16 +255,20 @@ static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
vq = vsock->vqs[VSOCK_VQ_RX];
do {
+ struct page *buf_page;
+
pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
if (!pkt)
break;
- pkt->buf = kmalloc(buf_len, GFP_KERNEL);
- if (!pkt->buf) {
+ buf_page = alloc_page(GFP_KERNEL);
+
+ if (!buf_page) {
virtio_transport_free_pkt(pkt);
break;
}
+ pkt->buf = page_to_virt(buf_page);
pkt->buf_len = buf_len;
pkt->len = buf_len;
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index a9980e9b9304..37e8dbfe2f5d 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -69,6 +69,7 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
if (!pkt->buf)
goto out_pkt;
+ pkt->slab_buf = true;
pkt->buf_len = len;
err = memcpy_from_msg(pkt->buf, info->msg, len);
@@ -1339,7 +1340,14 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
void virtio_transport_free_pkt(struct virtio_vsock_pkt *pkt)
{
- kvfree(pkt->buf);
+ if (pkt->buf_len) {
+ if (pkt->slab_buf)
+ kvfree(pkt->buf);
Hi,
kfree()? (according to virtio_transport_alloc_pkt() in -next) or something else need to be changed.
Hello Cristophe,
I think, 'kvfree()' is still needed here, because buffer for packet could be allocated by 'kvmalloc()'
in drivers/vhost/vsock.c. Correct me if i'm wrong.
Agreed.
+ else
+ free_pages((unsigned long)pkt->buf,
+ get_order(pkt->buf_len));
In virtio_vsock_rx_fill(), only alloc_page() is used.
Should this be alloc_pages(.., get_order(buf_len)) or free_page() (without an 's') here?
This function frees packets which were allocated in vhost path also. In vhost, for zerocopy
packets alloc_pageS() is used.
Ok. Seen in patch 5/11.
But wouldn't it be cleaner and future-proof to also have alloc_pageS()
in virtio_vsock_rx_fill(), even if get_order(buf->len) is kwown to be 0?
What, if for some reason a PAGE_SIZE was < 4kb for a given arch, or
VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE increased?
CJ
Thank You, Arseniy
Just my 2c,
CJ
+ }
+
kfree(pkt);
}
EXPORT_SYMBOL_GPL(virtio_transport_free_pkt);
