Messages in this thread |  | | Subject | Re: [PATCH v2 7/8] vsock/virtio: increase RX buffer size to 64 KiB | From | Jason Wang <> | Date | Tue, 14 May 2019 11:38:05 +0800 |
| |
On 2019/5/14 上午1:51, Stefano Garzarella wrote: > On Mon, May 13, 2019 at 06:01:52PM +0800, Jason Wang wrote: >> On 2019/5/10 下午8:58, Stefano Garzarella wrote: >>> In order to increase host -> guest throughput with large packets, >>> we can use 64 KiB RX buffers. >>> >>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >>> --- >>> include/linux/virtio_vsock.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h >>> index 84b72026d327..5a9d25be72df 100644 >>> --- a/include/linux/virtio_vsock.h >>> +++ b/include/linux/virtio_vsock.h >>> @@ -10,7 +10,7 @@ >>> #define VIRTIO_VSOCK_DEFAULT_MIN_BUF_SIZE 128 >>> #define VIRTIO_VSOCK_DEFAULT_BUF_SIZE (1024 * 256) >>> #define VIRTIO_VSOCK_DEFAULT_MAX_BUF_SIZE (1024 * 256) >>> -#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4) >>> +#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 64) >>> #define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL >>> #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64) >> >> We probably don't want such high order allocation. It's better to switch to >> use order 0 pages in this case. See add_recvbuf_big() for virtio-net. If we >> get datapath unified, we will get more stuffs set. > IIUC, you are suggesting to allocate only pages and put them in a > scatterlist, then add them to the virtqueue. > > Is it correct?
Yes since you are using:
pkt->buf = kmalloc(buf_len, GFP_KERNEL); if (!pkt->buf) { virtio_transport_free_pkt(pkt); break; }
This is likely to fail when the memory is fragmented which is kind of fragile.
> > The issue that I have here, is that the virtio-vsock guest driver, see > virtio_vsock_rx_fill(), allocates a struct virtio_vsock_pkt that > contains the room for the header, then allocates the buffer for the payload. > At this point it fills the scatterlist with the &virtio_vsock_pkt.hdr and the > buffer for the payload.
This part should be fine since what is needed is just adding more pages to sg[] and call virtuqeueu_add_sg().
> > Changing this will require several modifications, and if we get datapath > unified, I'm not sure it's worth it. > Of course, if we leave the datapaths separated, I'd like to do that later. > > What do you think?
For the driver it self, it should not be hard. But I think you mean the issue of e.g virtio_vsock_pkt itself which doesn't support sg. For short time, maybe we can use kvec instead.
Thanks
> > Thanks, > Stefano
|  |