Re: [PATCH 4/4] tun: indicate support for USO feature

2021-05-10 Thread Jason Wang


在 2021/5/11 下午12:42, Yuri Benditovich 写道:

Signed-off-by: Yuri Benditovich 
---
  drivers/net/tun.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 84f832806313..a35054f9d941 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned 
long arg)
arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
}
  
-		arg &= ~TUN_F_UFO;

+   arg &= ~(TUN_F_UFO|TUN_F_USO);



It looks to me kernel doesn't use "USO", so TUN_F_UDP_GSO_L4 is a better 
name for this and I guess we should toggle NETIF_F_UDP_GSO_l4 here?


And how about macvtap?

Thanks



}
  
  	/* This gives the user a way to test for new features in future by


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/4] virtio-net: add support of UDP segmentation (USO) on the host

2021-05-10 Thread Jason Wang


在 2021/5/11 下午12:42, Yuri Benditovich 写道:

Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.

Signed-off-by: Yuri Benditovich 
---
  include/linux/virtio_net.h | 5 +
  1 file changed, 5 insertions(+)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index b465f8f3e554..4ecf9a1ca912 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
ip_proto = IPPROTO_UDP;
thlen = sizeof(struct udphdr);
break;
+   case VIRTIO_NET_HDR_GSO_UDP_L4:
+   gso_type = SKB_GSO_UDP_L4;
+   ip_proto = IPPROTO_UDP;
+   thlen = sizeof(struct udphdr);
+   break;



This is only for rx, how about tx?

Thanks




default:
return -EINVAL;
}


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/4] virtio-net: add definitions for host USO feature

2021-05-10 Thread Jason Wang


在 2021/5/11 下午12:42, Yuri Benditovich 写道:

Define feature bit and GSO type according to the VIRTIO
specification.

Signed-off-by: Yuri Benditovich 
---
  include/uapi/linux/virtio_net.h | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 3f55a4215f11..a556ac735d7f 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -57,6 +57,7 @@
 * Steering */
  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
  
+#define VIRTIO_NET_F_HOST_USO 56	/* Host can handle USO packets */

  #define VIRTIO_NET_F_HASH_REPORT  57  /* Supports hash report */
  #define VIRTIO_NET_F_RSS60/* Supports RSS RX steering */
  #define VIRTIO_NET_F_RSC_EXT61/* extended coalescing info */
@@ -130,6 +131,7 @@ struct virtio_net_hdr_v1 {
  #define VIRTIO_NET_HDR_GSO_TCPV4  1   /* GSO frame, IPv4 TCP (TSO) */
  #define VIRTIO_NET_HDR_GSO_UDP3   /* GSO frame, IPv4 UDP 
(UFO) */
  #define VIRTIO_NET_HDR_GSO_TCPV6  4   /* GSO frame, IPv6 TCP */
+#define VIRTIO_NET_HDR_GSO_UDP_L4  5   /* GSO frame, IPv4 UDP (USO) */



This is the gso_type not the feature actually.

I wonder what's the reason for not

1) introducing a dedicated virtio-net feature bit for this 
(VIRTIO_NET_F_GUEST_GSO_UDP_L4.
2) toggle the NETIF_F_GSO_UDP_L4  feature for tuntap based on the 
negotiated feature.


Thanks



  #define VIRTIO_NET_HDR_GSO_ECN0x80/* TCP has ECN set */
__u8 gso_type;
__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] drm/ttm: use dma_alloc_pages for the page pool

2021-05-10 Thread Christoph Hellwig
Use the dma_alloc_pages allocator for the TTM pool allocator.
This allocator is a front end to the page allocator which takes the
DMA mask of the device into account, thus offering the best of both
worlds of the two existing allocator versions.  This conversion also
removes the ugly layering violation where the TTM pool assumes what
kind of virtual address dma_alloc_attrs can return.

Signed-off-by: Christoph Hellwig 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |   1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |   4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |   1 -
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   |   1 -
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |   1 -
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   |   1 -
 drivers/gpu/drm/drm_cache.c |  31 -
 drivers/gpu/drm/drm_gem_vram_helper.c   |   3 +-
 drivers/gpu/drm/nouveau/nouveau_ttm.c   |   8 +-
 drivers/gpu/drm/qxl/qxl_ttm.c   |   3 +-
 drivers/gpu/drm/radeon/radeon.h |   1 -
 drivers/gpu/drm/radeon/radeon_device.c  |   1 -
 drivers/gpu/drm/radeon/radeon_ttm.c |   4 +-
 drivers/gpu/drm/ttm/ttm_device.c|   7 +-
 drivers/gpu/drm/ttm/ttm_pool.c  | 178 
 drivers/gpu/drm/ttm/ttm_tt.c|  25 +---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |   4 +-
 include/drm/drm_cache.h |   1 -
 include/drm/ttm/ttm_device.h|   3 +-
 include/drm/ttm/ttm_pool.h  |   9 +-
 20 files changed, 41 insertions(+), 246 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index dc3a69296321b3..5f40527eeef1ff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -819,7 +819,6 @@ struct amdgpu_device {
int usec_timeout;
const struct amdgpu_asic_funcs  *asic_funcs;
boolshutdown;
-   boolneed_swiotlb;
boolaccel_working;
struct notifier_block   acpi_nb;
struct amdgpu_i2c_chan  *i2c_bus[AMDGPU_MAX_I2C_BUS];
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3bef0432cac2f7..9bf17b44cba6fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1705,9 +1705,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
/* No others user of address space so set it to 0 */
r = ttm_device_init(&adev->mman.bdev, &amdgpu_bo_driver, adev->dev,
   adev_to_drm(adev)->anon_inode->i_mapping,
-  adev_to_drm(adev)->vma_offset_manager,
-  adev->need_swiotlb,
-  dma_addressing_limited(adev->dev));
+  adev_to_drm(adev)->vma_offset_manager);
if (r) {
DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 405d6ad09022ca..2d4fa754513033 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -846,7 +846,6 @@ static int gmc_v6_0_sw_init(void *handle)
dev_warn(adev->dev, "No suitable DMA available.\n");
return r;
}
-   adev->need_swiotlb = drm_need_swiotlb(44);
 
r = gmc_v6_0_init_microcode(adev);
if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 210ada2289ec9c..a504db24f4c2a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -1025,7 +1025,6 @@ static int gmc_v7_0_sw_init(void *handle)
pr_warn("No suitable DMA available\n");
return r;
}
-   adev->need_swiotlb = drm_need_swiotlb(40);
 
r = gmc_v7_0_init_microcode(adev);
if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index e4f27b3f28fb58..42e7b1eb84b3bc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1141,7 +1141,6 @@ static int gmc_v8_0_sw_init(void *handle)
pr_warn("No suitable DMA available\n");
return r;
}
-   adev->need_swiotlb = drm_need_swiotlb(40);
 
r = gmc_v8_0_init_microcode(adev);
if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 455bb91060d0bc..f74784b3423740 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1548,7 +1548,6 @@ static int gmc_v9_0_sw_init(void *handle)
printk(KERN_WARNING "amdgpu: No suitable DMA available.\n");
return r;
}
-   adev->need_swiotlb = drm_need_swiotlb(44);
 
if (adev

RFC: use dma_alloc_noncoherent in ttm_pool_alloc_page

2021-05-10 Thread Christoph Hellwig
Hi all,

the memory allocation for the TTM pool is a big mess with two allocation
methods that both have issues, a layering violation and odd guessing of
pools in the callers.

This patch switches to the dma_alloc_noncoherent API instead fixing all
of the above issues.

Warning:  i don't have any of the relevant hardware, so this is a compile
tested request for comments only!

Diffstat:
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |1 
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |4 
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |1 
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   |1 
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |1 
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   |1 
 drivers/gpu/drm/drm_cache.c |   31 -
 drivers/gpu/drm/drm_gem_vram_helper.c   |3 
 drivers/gpu/drm/nouveau/nouveau_ttm.c   |8 -
 drivers/gpu/drm/qxl/qxl_ttm.c   |3 
 drivers/gpu/drm/radeon/radeon.h |1 
 drivers/gpu/drm/radeon/radeon_device.c  |1 
 drivers/gpu/drm/radeon/radeon_ttm.c |4 
 drivers/gpu/drm/ttm/ttm_device.c|7 -
 drivers/gpu/drm/ttm/ttm_pool.c  |  178 
 drivers/gpu/drm/ttm/ttm_tt.c|   25 
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |4 
 include/drm/drm_cache.h |1 
 include/drm/ttm/ttm_device.h|3 
 include/drm/ttm/ttm_pool.h  |9 -
 20 files changed, 41 insertions(+), 246 deletions(-)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 4/4] tun: indicate support for USO feature

2021-05-10 Thread Yuri Benditovich
Signed-off-by: Yuri Benditovich 
---
 drivers/net/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 84f832806313..a35054f9d941 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned 
long arg)
arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
}
 
-   arg &= ~TUN_F_UFO;
+   arg &= ~(TUN_F_UFO|TUN_F_USO);
}
 
/* This gives the user a way to test for new features in future by
-- 
2.26.3

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 2/4] virtio-net: add support of UDP segmentation (USO) on the host

2021-05-10 Thread Yuri Benditovich
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.

Signed-off-by: Yuri Benditovich 
---
 include/linux/virtio_net.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index b465f8f3e554..4ecf9a1ca912 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
ip_proto = IPPROTO_UDP;
thlen = sizeof(struct udphdr);
break;
+   case VIRTIO_NET_HDR_GSO_UDP_L4:
+   gso_type = SKB_GSO_UDP_L4;
+   ip_proto = IPPROTO_UDP;
+   thlen = sizeof(struct udphdr);
+   break;
default:
return -EINVAL;
}
-- 
2.26.3

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 1/4] virtio-net: add definitions for host USO feature

2021-05-10 Thread Yuri Benditovich
Define feature bit and GSO type according to the VIRTIO
specification.

Signed-off-by: Yuri Benditovich 
---
 include/uapi/linux/virtio_net.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 3f55a4215f11..a556ac735d7f 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -57,6 +57,7 @@
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
+#define VIRTIO_NET_F_HOST_USO 56   /* Host can handle USO packets */
 #define VIRTIO_NET_F_HASH_REPORT  57   /* Supports hash report */
 #define VIRTIO_NET_F_RSS 60/* Supports RSS RX steering */
 #define VIRTIO_NET_F_RSC_EXT 61/* extended coalescing info */
@@ -130,6 +131,7 @@ struct virtio_net_hdr_v1 {
 #define VIRTIO_NET_HDR_GSO_TCPV4   1   /* GSO frame, IPv4 TCP (TSO) */
 #define VIRTIO_NET_HDR_GSO_UDP 3   /* GSO frame, IPv4 UDP (UFO) */
 #define VIRTIO_NET_HDR_GSO_TCPV6   4   /* GSO frame, IPv6 TCP */
+#define VIRTIO_NET_HDR_GSO_UDP_L4  5   /* GSO frame, IPv4 UDP (USO) */
 #define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
__u8 gso_type;
__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
-- 
2.26.3

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 3/4] tun: define feature bit for USO support

2021-05-10 Thread Yuri Benditovich
User mode software can probe this bit to check whether the
USO feature is supported by TUN/TAP device.

Signed-off-by: Yuri Benditovich 
---
 include/uapi/linux/if_tun.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 454ae31b93c7..24f246920dd5 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -88,6 +88,7 @@
 #define TUN_F_TSO6 0x04/* I can handle TSO for IPv6 packets */
 #define TUN_F_TSO_ECN  0x08/* I can handle TSO with ECN bits. */
 #define TUN_F_UFO  0x10/* I can handle UFO packets */
+#define TUN_F_USO  0x20/* I can handle USO packets */
 
 /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */
 #define TUN_PKT_STRIP  0x0001
-- 
2.26.3

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 0/4] Add host USO support to TUN device

2021-05-10 Thread Yuri Benditovich
This series adds support for UDP segmentation offload feature
in TUN device according to the VIRTIO specification

Yuri Benditovich (4):
  virtio-net: add definitions for host USO feature
  virtio-net: add support of UDP segmentation (USO) on the host
  tun: define feature bit for USO support
  tun: indicate support for USO feature

 drivers/net/tun.c   | 2 +-
 include/linux/virtio_net.h  | 5 +
 include/uapi/linux/if_tun.h | 1 +
 include/uapi/linux/virtio_net.h | 2 ++
 4 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.26.3

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] virtiofs: Enable multiple request queues

2021-05-10 Thread Vivek Goyal
On Mon, May 10, 2021 at 11:15:21AM -0500, Connor Kuehl wrote:
> On 5/10/21 10:25 AM, Vivek Goyal wrote:
> > On Fri, May 07, 2021 at 03:15:27PM -0700, Connor Kuehl wrote:
> >> Distribute requests across the multiqueue complex automatically based
> >> on the IRQ affinity.
> > 
> > Hi Connor,
> > 
> > Thanks for the patch. I will look into it and also test it.
> > 
> > How did you test it? Did you modify vitiofsd to support multiqueue. Did
> > you also run some performance numbers. Does it provide better/worse
> > performance as compared to single queue.
> 
> Thanks, Vivek! I need to NACK this version of the patch for inclusion
> though since I think the way I did per-CPU state will not work for
> multiple virtio-fs mounts because it will be overwritten with each new
> mount, but for testing purposes this should be OK with just one mount.

Hi Connor,

Ok. Will wait for next version which fixes the multiple mount issue.

> 
> I need to do more benchmarking on this.

That would be nice.

> 
> I had to hack multiqueue support into virtiofsd, which runs against the
> warning in the virtiofsd source code that instructs people to *not*
> enable multiqueue due to thread-safety concerns. I didn't audit
> virtiofsd for correctness, so I also worry this has the potential of
> affecting benchmarks if there are races.

filesystem code already can handle multiple threads because on a single
queue we can have a thread pool processing requests in parallel. I am
not aware of any issues about supporting multiple queues. I think
may be fuse_virtio.c might require a little closer inspection to make
sure nothing is dependent on single queue. 

> 
> For testing, QEMU needs to be invoked with `num-request-queues` like
> this:
> 
>   -device vhost-user-fs-pci,chardev=char0,tag=myfs,num-request-queues=2 
> 
> And obviously you can choose any value >= 1 for num-request-queues.
> 
> and I also made a quick-and-dirty hack to let me pass in the number of
> total queues to virtiofsd on the command line:

Ok. May be there is some inspiration to take from virtio-blk. How do they
specific number of queues by default and how many. I thought stefan mentioned
that by default there is one queue per vcpu.

Vivek

> 
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index 58e32fc963..cf8f132efd 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -2565,9 +2565,9 @@ out1:
>  return NULL;
>  }
>  
> -int fuse_session_mount(struct fuse_session *se)
> +int fuse_session_mount(struct fuse_session *se, unsigned int num_queues)
>  {
> -return virtio_session_mount(se);
> +return virtio_session_mount(se, num_queues);
>  }
>  
>  int fuse_session_fd(struct fuse_session *se)
> diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
> index 3bf786b034..50bf86113d 100644
> --- a/tools/virtiofsd/fuse_lowlevel.h
> +++ b/tools/virtiofsd/fuse_lowlevel.h
> @@ -1842,7 +1842,7 @@ struct fuse_session *fuse_session_new(struct fuse_args 
> *args,
>   *
>   * @return 0 on success, -1 on failure.
>   **/
> -int fuse_session_mount(struct fuse_session *se);
> +int fuse_session_mount(struct fuse_session *se, unsigned int num_queues);
>  
>  /**
>   * Enter a single threaded, blocking event loop.
> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> index 3e13997406..8622c3dce6 100644
> --- a/tools/virtiofsd/fuse_virtio.c
> +++ b/tools/virtiofsd/fuse_virtio.c
> @@ -747,20 +747,6 @@ static void fv_queue_set_started(VuDev *dev, int qidx, 
> bool started)
>   started);
>  assert(qidx >= 0);
>  
> -/*
> - * Ignore additional request queues for now.  passthrough_ll.c must be
> - * audited for thread-safety issues first.  It was written with a
> - * well-behaved client in mind and may not protect against all types of
> - * races yet.
> - */
> -if (qidx > 1) {
> -fuse_log(FUSE_LOG_ERR,
> - "%s: multiple request queues not yet implemented, please 
> only "
> - "configure 1 request queue\n",
> - __func__);
> -exit(EXIT_FAILURE);
> -}
> -
>  if (started) {
>  /* Fire up a thread to watch this queue */
>  if (qidx >= vud->nqueues) {
> @@ -997,7 +983,7 @@ static int fv_create_listen_socket(struct fuse_session 
> *se)
>  return 0;
>  }
>  
> -int virtio_session_mount(struct fuse_session *se)
> +int virtio_session_mount(struct fuse_session *se, unsigned int num_queues)
>  {
>  int ret;
>  
> @@ -1048,8 +1034,8 @@ int virtio_session_mount(struct fuse_session *se)
>  se->vu_socketfd = data_sock;
>  se->virtio_dev->se = se;
>  pthread_rwlock_init(&se->virtio_dev->vu_dispatch_rwlock, NULL);
> -if (!vu_init(&se->virtio_dev->dev, 2, se->vu_socketfd, fv_panic, NULL,
> - fv_set_watch, fv_remove_watch, &fv_iface)) {
> +if (!vu_init(&se->virtio_dev->dev, num_queues, se->vu_socketfd,
> +   

Re: [PATCH] virtiofs: Enable multiple request queues

2021-05-10 Thread Connor Kuehl
On 5/10/21 10:25 AM, Vivek Goyal wrote:
> On Fri, May 07, 2021 at 03:15:27PM -0700, Connor Kuehl wrote:
>> Distribute requests across the multiqueue complex automatically based
>> on the IRQ affinity.
> 
> Hi Connor,
> 
> Thanks for the patch. I will look into it and also test it.
> 
> How did you test it? Did you modify vitiofsd to support multiqueue. Did
> you also run some performance numbers. Does it provide better/worse
> performance as compared to single queue.

Thanks, Vivek! I need to NACK this version of the patch for inclusion
though since I think the way I did per-CPU state will not work for
multiple virtio-fs mounts because it will be overwritten with each new
mount, but for testing purposes this should be OK with just one mount.

I need to do more benchmarking on this.

I had to hack multiqueue support into virtiofsd, which runs against the
warning in the virtiofsd source code that instructs people to *not*
enable multiqueue due to thread-safety concerns. I didn't audit
virtiofsd for correctness, so I also worry this has the potential of
affecting benchmarks if there are races.

For testing, QEMU needs to be invoked with `num-request-queues` like
this:

-device vhost-user-fs-pci,chardev=char0,tag=myfs,num-request-queues=2 

And obviously you can choose any value >= 1 for num-request-queues.

and I also made a quick-and-dirty hack to let me pass in the number of
total queues to virtiofsd on the command line:

diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 58e32fc963..cf8f132efd 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -2565,9 +2565,9 @@ out1:
 return NULL;
 }
 
-int fuse_session_mount(struct fuse_session *se)
+int fuse_session_mount(struct fuse_session *se, unsigned int num_queues)
 {
-return virtio_session_mount(se);
+return virtio_session_mount(se, num_queues);
 }
 
 int fuse_session_fd(struct fuse_session *se)
diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
index 3bf786b034..50bf86113d 100644
--- a/tools/virtiofsd/fuse_lowlevel.h
+++ b/tools/virtiofsd/fuse_lowlevel.h
@@ -1842,7 +1842,7 @@ struct fuse_session *fuse_session_new(struct fuse_args 
*args,
  *
  * @return 0 on success, -1 on failure.
  **/
-int fuse_session_mount(struct fuse_session *se);
+int fuse_session_mount(struct fuse_session *se, unsigned int num_queues);
 
 /**
  * Enter a single threaded, blocking event loop.
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 3e13997406..8622c3dce6 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -747,20 +747,6 @@ static void fv_queue_set_started(VuDev *dev, int qidx, 
bool started)
  started);
 assert(qidx >= 0);
 
-/*
- * Ignore additional request queues for now.  passthrough_ll.c must be
- * audited for thread-safety issues first.  It was written with a
- * well-behaved client in mind and may not protect against all types of
- * races yet.
- */
-if (qidx > 1) {
-fuse_log(FUSE_LOG_ERR,
- "%s: multiple request queues not yet implemented, please only 
"
- "configure 1 request queue\n",
- __func__);
-exit(EXIT_FAILURE);
-}
-
 if (started) {
 /* Fire up a thread to watch this queue */
 if (qidx >= vud->nqueues) {
@@ -997,7 +983,7 @@ static int fv_create_listen_socket(struct fuse_session *se)
 return 0;
 }
 
-int virtio_session_mount(struct fuse_session *se)
+int virtio_session_mount(struct fuse_session *se, unsigned int num_queues)
 {
 int ret;
 
@@ -1048,8 +1034,8 @@ int virtio_session_mount(struct fuse_session *se)
 se->vu_socketfd = data_sock;
 se->virtio_dev->se = se;
 pthread_rwlock_init(&se->virtio_dev->vu_dispatch_rwlock, NULL);
-if (!vu_init(&se->virtio_dev->dev, 2, se->vu_socketfd, fv_panic, NULL,
- fv_set_watch, fv_remove_watch, &fv_iface)) {
+if (!vu_init(&se->virtio_dev->dev, num_queues, se->vu_socketfd,
+fv_panic, NULL, fv_set_watch, fv_remove_watch, &fv_iface)) {
 fuse_log(FUSE_LOG_ERR, "%s: vu_init failed\n", __func__);
 return -1;
 }
diff --git a/tools/virtiofsd/fuse_virtio.h b/tools/virtiofsd/fuse_virtio.h
index 111684032c..a0e78b9b84 100644
--- a/tools/virtiofsd/fuse_virtio.h
+++ b/tools/virtiofsd/fuse_virtio.h
@@ -18,7 +18,7 @@
 
 struct fuse_session;
 
-int virtio_session_mount(struct fuse_session *se);
+int virtio_session_mount(struct fuse_session *se, unsigned int num_queues);
 void virtio_session_close(struct fuse_session *se);
 int virtio_loop(struct fuse_session *se);
 
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 1553d2ef45..9fd4e34980 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -161,6 +161,7 @@ struct lo_data {
 int allow_direct_io;
 int announce_submounts;
 bool use

Re: [PATCH] virtiofs: Enable multiple request queues

2021-05-10 Thread Vivek Goyal
On Fri, May 07, 2021 at 03:15:27PM -0700, Connor Kuehl wrote:
> Distribute requests across the multiqueue complex automatically based
> on the IRQ affinity.

Hi Connor,

Thanks for the patch. I will look into it and also test it.

How did you test it? Did you modify vitiofsd to support multiqueue. Did
you also run some performance numbers. Does it provide better/worse
performance as compared to single queue.

Thanks
Vivek

> 
> Suggested-by: Stefan Hajnoczi 
> Signed-off-by: Connor Kuehl 
> ---
>  fs/fuse/virtio_fs.c | 30 --
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index bcb8a02e2d8b..dcdc8b7b1ad5 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -30,6 +30,10 @@
>  static DEFINE_MUTEX(virtio_fs_mutex);
>  static LIST_HEAD(virtio_fs_instances);
>  
> +struct virtio_fs_vq;
> +
> +DEFINE_PER_CPU(struct virtio_fs_vq *, this_cpu_fsvq);
> +
>  enum {
>   VQ_HIPRIO,
>   VQ_REQUEST
> @@ -673,6 +677,7 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev,
>   struct virtqueue **vqs;
>   vq_callback_t **callbacks;
>   const char **names;
> + struct irq_affinity desc = { .pre_vectors = 1, .nr_sets = 1, };
>   unsigned int i;
>   int ret = 0;
>  
> @@ -681,6 +686,9 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev,
>   if (fs->num_request_queues == 0)
>   return -EINVAL;
>  
> + fs->num_request_queues = min_t(unsigned int, nr_cpu_ids,
> +fs->num_request_queues);
> +
>   fs->nvqs = VQ_REQUEST + fs->num_request_queues;
>   fs->vqs = kcalloc(fs->nvqs, sizeof(fs->vqs[VQ_HIPRIO]), GFP_KERNEL);
>   if (!fs->vqs)
> @@ -710,12 +718,24 @@ static int virtio_fs_setup_vqs(struct virtio_device 
> *vdev,
>   names[i] = fs->vqs[i].name;
>   }
>  
> - ret = virtio_find_vqs(vdev, fs->nvqs, vqs, callbacks, names, NULL);
> + ret = virtio_find_vqs(vdev, fs->nvqs, vqs, callbacks, names, &desc);
>   if (ret < 0)
>   goto out;
>  
> - for (i = 0; i < fs->nvqs; i++)
> + for (i = 0; i < fs->nvqs; i++) {
> + const struct cpumask *mask;
> + unsigned int cpu;
> +
>   fs->vqs[i].vq = vqs[i];
> + if (i == VQ_HIPRIO)
> + continue;
> +
> + mask = vdev->config->get_vq_affinity(vdev, i);
> + for_each_cpu(cpu, mask) {
> + struct virtio_fs_vq **cpu_vq = 
> per_cpu_ptr(&this_cpu_fsvq, cpu);
> + *cpu_vq = &fs->vqs[i];
> + }
> + }
>  
>   virtio_fs_start_all_queues(fs);
>  out:
> @@ -877,8 +897,6 @@ static int virtio_fs_probe(struct virtio_device *vdev)
>   if (ret < 0)
>   goto out;
>  
> - /* TODO vq affinity */
> -
>   ret = virtio_fs_setup_dax(vdev, fs);
>   if (ret < 0)
>   goto out_vqs;
> @@ -1225,7 +1243,6 @@ static int virtio_fs_enqueue_req(struct virtio_fs_vq 
> *fsvq,
>  static void virtio_fs_wake_pending_and_unlock(struct fuse_iqueue *fiq)
>  __releases(fiq->lock)
>  {
> - unsigned int queue_id = VQ_REQUEST; /* TODO multiqueue */
>   struct virtio_fs *fs;
>   struct fuse_req *req;
>   struct virtio_fs_vq *fsvq;
> @@ -1245,7 +1262,8 @@ __releases(fiq->lock)
>req->in.h.nodeid, req->in.h.len,
>fuse_len_args(req->args->out_numargs, req->args->out_args));
>  
> - fsvq = &fs->vqs[queue_id];
> + fsvq = this_cpu_read(this_cpu_fsvq);
> +
>   ret = virtio_fs_enqueue_req(fsvq, req, false);
>   if (ret < 0) {
>   if (ret == -ENOMEM || ret == -ENOSPC) {
> -- 
> 2.30.2
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC v2] virtio-vsock: add description for datagram type

2021-05-10 Thread Stefano Garzarella

On Fri, May 07, 2021 at 09:53:19AM -0700, Jiang Wang . wrote:

Hi guys,

I have one question about adding two new virtqueues for dgram. One new
thought is that we don't add two new virtqueues but keep using existing
virtqueues for both stream and dgram.

At the beginning when I first thought about supporting dgram, I thought
adding two new virtqueues would be easier and have better performance.
But now, after the prototype is done, I think that to keep using
existing virtqueues is also not complicated and could in fact be simpler.
The performance difference may not be very big.


I honestly didn't think it was easier to use two new queues, quite the 
opposite.




Original code has about 3 places which have assumptions about the
virtqueues are only used by the stream. But we can change those codes.
One place is to check pkt len. We can check only for stream pkts.
Another two are in tx and rx code path where if queued replies pkts are
too much, the code will stop the rx queue and resume later. We can keep
that same logic. The dgram will be affected a little bit but that should
be fine I think. Are there any other places that we should fix?


Did you take a look at Arseny's series?
I think he's already found the places where to check the type and it 
seems to me they are the places you listed.




In short, the virtqueues are in a lower level and can support multiple
flows and socket types. Use existing virtqueues also make it more
compatible with old versions.


It's not clear to me how compatibility is improved. Can you elaborate on 
this?




What do you guys think? I remember Stefano mentioned that we should add
two new virtqueues for dgram. Stefano, do you have some specific reasons
for that? Could we just keep using existing virtqueues? Thanks.



My biggest concern was about the credit mechanism for datagrams. I mean 
avoiding datagrams from crowding the queue without limits, preventing 
streams from communicating.


If you've found a way to limit datagram traffic, then maybe it's doable.

Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] vdpa_sim_blk: remove duplicate include of linux/blkdev.h

2021-05-10 Thread Stefano Garzarella

On Mon, May 10, 2021 at 10:43:03AM +0800, Wan Jiabing wrote:

In commit 7d189f617f83f ("vdpa_sim_blk: implement ramdisk behaviour")
linux/blkdev.h was included here causing the duplicate include.
Remove the later duplicate include.

Signed-off-by: Wan Jiabing 
---
drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c 
b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
index 5bfe1c281645..a790903f243e 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
@@ -15,7 +15,6 @@
#include 
#include 
#include 
-#include 
#include 

#include "vdpa_sim.h"
--
2.20.1



Ooops...

Thanks for fixing this!

Reviewed-by: Stefano Garzarella 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization