| Message ID | 20240424091533.86949-1-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | refactor the params of find_vqs() | expand |
Hi, Michael I hope this in your for_linus branch to merge to Linux 6.9. https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=linux-next And some commits from me in your branch are changed after you picked them. And there are merged by net-next. virtio_net: remove the misleading comment virtio_net: rx remove premapped failover code virtio_net: enable premapped by default virtio_net: big mode support premapped virtio_net: replace private by pp struct inside page virtio_ring: enable premapped mode whatever use_dma_api virtio_ring: introduce dma map api for page Thanks. On Wed, 24 Apr 2024 17:15:27 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote: > This pathset is splited from the > > http://lore.kernel.org/all/20240229072044.77388-1-xuanzhuo@linux.alibaba.com > > That may needs some cycles to discuss. But that notifies too many people. > > But just the four commits need to notify so many people. > And four commits are independent. So I split that patch set, > let us review these first. > > The patch set try to refactor the params of find_vqs(). > Then we can just change the structure, when introducing new > features. > > Thanks. > > v8: > 1. rebase the vhost branch > > v7: > 1. fix two bugs. @Jason > > v6: > 1. virtio_balloon: a single variable for both purposes. > 2. if names[i] is null, return error > > v5: > 1. virtio_balloon: follow David Hildenbrand's suggest > http://lore.kernel.org/all/3620be9c-e288-4ff2-a7be-1fcf806e6e6e@redhat.com > 2. fix bug of the reference of "cfg_idx" > http://lore.kernel.org/all/202403222227.Sdp23Lcb-lkp@intel.com > > v4: > 1. remove support for names array entries being null > 2. remove cfg_idx from virtio_vq_config > > v3: > 1. fix the bug: "assignment of read-only location '*cfg.names'" > > v2: > 1. add kerneldoc for "struct vq_transport_config" @ilpo.jarvinen > > v1: > 1. fix some comments from ilpo.jarvinen@linux.intel.com > > > > > > > > > > Xuan Zhuo (6): > virtio_balloon: remove the dependence where names[] is null > virtio: remove support for names array entries being null. > virtio: find_vqs: pass struct instead of multi parameters > virtio: vring_create_virtqueue: pass struct instead of multi > parameters > virtio: vring_new_virtqueue(): pass struct instead of multi parameters > virtio_ring: simplify the parameters of the funcs related to > vring_create/new_virtqueue() > > arch/um/drivers/virtio_uml.c | 36 +++-- > drivers/platform/mellanox/mlxbf-tmfifo.c | 23 +-- > drivers/remoteproc/remoteproc_virtio.c | 37 +++-- > drivers/s390/virtio/virtio_ccw.c | 38 ++--- > drivers/virtio/virtio_balloon.c | 48 +++--- > drivers/virtio/virtio_mmio.c | 36 +++-- > drivers/virtio/virtio_pci_common.c | 69 ++++----- > drivers/virtio/virtio_pci_common.h | 9 +- > drivers/virtio/virtio_pci_legacy.c | 16 +- > drivers/virtio/virtio_pci_modern.c | 37 +++-- > drivers/virtio/virtio_ring.c | 177 ++++++++--------------- > drivers/virtio/virtio_vdpa.c | 51 +++---- > include/linux/virtio_config.h | 76 +++++++--- > include/linux/virtio_ring.h | 93 +++++++----- > tools/virtio/virtio_test.c | 4 +- > tools/virtio/vringh_test.c | 28 ++-- > 16 files changed, 384 insertions(+), 394 deletions(-) > > -- > 2.32.0.3.g01195cf9f >
On Wed, Apr 24, 2024 at 05:15:27PM +0800, Xuan Zhuo wrote: > This pathset is splited from the > > http://lore.kernel.org/all/20240229072044.77388-1-xuanzhuo@linux.alibaba.com > > That may needs some cycles to discuss. But that notifies too many people. > > But just the four commits need to notify so many people. > And four commits are independent. So I split that patch set, > let us review these first. > > The patch set try to refactor the params of find_vqs(). > Then we can just change the structure, when introducing new > features. > > Thanks. It's nice but I'd like to see something that uses this before I bother merging. IIUC premapped is dropped - are we going to use this in practice? > v8: > 1. rebase the vhost branch > > v7: > 1. fix two bugs. @Jason > > v6: > 1. virtio_balloon: a single variable for both purposes. > 2. if names[i] is null, return error > > v5: > 1. virtio_balloon: follow David Hildenbrand's suggest > http://lore.kernel.org/all/3620be9c-e288-4ff2-a7be-1fcf806e6e6e@redhat.com > 2. fix bug of the reference of "cfg_idx" > http://lore.kernel.org/all/202403222227.Sdp23Lcb-lkp@intel.com > > v4: > 1. remove support for names array entries being null > 2. remove cfg_idx from virtio_vq_config > > v3: > 1. fix the bug: "assignment of read-only location '*cfg.names'" > > v2: > 1. add kerneldoc for "struct vq_transport_config" @ilpo.jarvinen > > v1: > 1. fix some comments from ilpo.jarvinen@linux.intel.com > > > > > > > > > > Xuan Zhuo (6): > virtio_balloon: remove the dependence where names[] is null > virtio: remove support for names array entries being null. > virtio: find_vqs: pass struct instead of multi parameters > virtio: vring_create_virtqueue: pass struct instead of multi > parameters > virtio: vring_new_virtqueue(): pass struct instead of multi parameters > virtio_ring: simplify the parameters of the funcs related to > vring_create/new_virtqueue() > > arch/um/drivers/virtio_uml.c | 36 +++-- > drivers/platform/mellanox/mlxbf-tmfifo.c | 23 +-- > drivers/remoteproc/remoteproc_virtio.c | 37 +++-- > drivers/s390/virtio/virtio_ccw.c | 38 ++--- > drivers/virtio/virtio_balloon.c | 48 +++--- > drivers/virtio/virtio_mmio.c | 36 +++-- > drivers/virtio/virtio_pci_common.c | 69 ++++----- > drivers/virtio/virtio_pci_common.h | 9 +- > drivers/virtio/virtio_pci_legacy.c | 16 +- > drivers/virtio/virtio_pci_modern.c | 37 +++-- > drivers/virtio/virtio_ring.c | 177 ++++++++--------------- > drivers/virtio/virtio_vdpa.c | 51 +++---- > include/linux/virtio_config.h | 76 +++++++--- > include/linux/virtio_ring.h | 93 +++++++----- > tools/virtio/virtio_test.c | 4 +- > tools/virtio/vringh_test.c | 28 ++-- > 16 files changed, 384 insertions(+), 394 deletions(-) > > -- > 2.32.0.3.g01195cf9f
On Wed, 22 May 2024 08:28:43 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Apr 24, 2024 at 05:15:27PM +0800, Xuan Zhuo wrote: > > This pathset is splited from the > > > > http://lore.kernel.org/all/20240229072044.77388-1-xuanzhuo@linux.alibaba.com > > > > That may needs some cycles to discuss. But that notifies too many people. > > > > But just the four commits need to notify so many people. > > And four commits are independent. So I split that patch set, > > let us review these first. > > > > The patch set try to refactor the params of find_vqs(). > > Then we can just change the structure, when introducing new > > features. > > > > Thanks. > > It's nice but I'd like to see something that uses this before I bother > merging. IIUC premapped is dropped - are we going to use this in practice? 1. You know this modification makes sense. 2. This modification is difficult. Unlike modifying virtio ring or virtio-net, this patch set requires modifying many modules and being reviewed by many people. 3. If you do not merge it now, then this patch set will most likely be abandoned. And I worked a lot on that. 4. premapped has not been abandoned, I have been advancing this work. What was abandoned was just virtio-net big mode's support for premapped. 5. My plan is to complete virtio-net support for af-xdp in 6.10. This must depend on premapped. So, I hope you merge this patch set. Thanks. > > > v8: > > 1. rebase the vhost branch > > > > v7: > > 1. fix two bugs. @Jason > > > > v6: > > 1. virtio_balloon: a single variable for both purposes. > > 2. if names[i] is null, return error > > > > v5: > > 1. virtio_balloon: follow David Hildenbrand's suggest > > http://lore.kernel.org/all/3620be9c-e288-4ff2-a7be-1fcf806e6e6e@redhat.com > > 2. fix bug of the reference of "cfg_idx" > > http://lore.kernel.org/all/202403222227.Sdp23Lcb-lkp@intel.com > > > > v4: > > 1. remove support for names array entries being null > > 2. remove cfg_idx from virtio_vq_config > > > > v3: > > 1. fix the bug: "assignment of read-only location '*cfg.names'" > > > > v2: > > 1. add kerneldoc for "struct vq_transport_config" @ilpo.jarvinen > > > > v1: > > 1. fix some comments from ilpo.jarvinen@linux.intel.com > > > > > > > > > > > > > > > > > > > > Xuan Zhuo (6): > > virtio_balloon: remove the dependence where names[] is null > > virtio: remove support for names array entries being null. > > virtio: find_vqs: pass struct instead of multi parameters > > virtio: vring_create_virtqueue: pass struct instead of multi > > parameters > > virtio: vring_new_virtqueue(): pass struct instead of multi parameters > > virtio_ring: simplify the parameters of the funcs related to > > vring_create/new_virtqueue() > > > > arch/um/drivers/virtio_uml.c | 36 +++-- > > drivers/platform/mellanox/mlxbf-tmfifo.c | 23 +-- > > drivers/remoteproc/remoteproc_virtio.c | 37 +++-- > > drivers/s390/virtio/virtio_ccw.c | 38 ++--- > > drivers/virtio/virtio_balloon.c | 48 +++--- > > drivers/virtio/virtio_mmio.c | 36 +++-- > > drivers/virtio/virtio_pci_common.c | 69 ++++----- > > drivers/virtio/virtio_pci_common.h | 9 +- > > drivers/virtio/virtio_pci_legacy.c | 16 +- > > drivers/virtio/virtio_pci_modern.c | 37 +++-- > > drivers/virtio/virtio_ring.c | 177 ++++++++--------------- > > drivers/virtio/virtio_vdpa.c | 51 +++---- > > include/linux/virtio_config.h | 76 +++++++--- > > include/linux/virtio_ring.h | 93 +++++++----- > > tools/virtio/virtio_test.c | 4 +- > > tools/virtio/vringh_test.c | 28 ++-- > > 16 files changed, 384 insertions(+), 394 deletions(-) > > > > -- > > 2.32.0.3.g01195cf9f >
On Wed, May 22, 2024 at 08:35:21PM +0800, Xuan Zhuo wrote: > On Wed, 22 May 2024 08:28:43 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Wed, Apr 24, 2024 at 05:15:27PM +0800, Xuan Zhuo wrote: > > > This pathset is splited from the > > > > > > http://lore.kernel.org/all/20240229072044.77388-1-xuanzhuo@linux.alibaba.com > > > > > > That may needs some cycles to discuss. But that notifies too many people. > > > > > > But just the four commits need to notify so many people. > > > And four commits are independent. So I split that patch set, > > > let us review these first. > > > > > > The patch set try to refactor the params of find_vqs(). > > > Then we can just change the structure, when introducing new > > > features. > > > > > > Thanks. > > > > It's nice but I'd like to see something that uses this before I bother > > merging. IIUC premapped is dropped - are we going to use this in practice? > > > 1. You know this modification makes sense. > 2. This modification is difficult. Unlike modifying virtio ring or virtio-net, > this patch set requires modifying many modules and being reviewed by > many people. > 3. If you do not merge it now, then this patch set will most likely be > abandoned. And I worked a lot on that. > 4. premapped has not been abandoned, I have been advancing this work. What was > abandoned was just virtio-net big mode's support for premapped. > 5. My plan is to complete virtio-net support for af-xdp in 6.10. This must > depend on premapped. > > So, I hope you merge this patch set. > > Thanks. If this makes thing easier for you, I can put it in my tree post release. This way you do not need to keep reposting it. I'll then merge it with premapped. > > > > > > v8: > > > 1. rebase the vhost branch > > > > > > v7: > > > 1. fix two bugs. @Jason > > > > > > v6: > > > 1. virtio_balloon: a single variable for both purposes. > > > 2. if names[i] is null, return error > > > > > > v5: > > > 1. virtio_balloon: follow David Hildenbrand's suggest > > > http://lore.kernel.org/all/3620be9c-e288-4ff2-a7be-1fcf806e6e6e@redhat.com > > > 2. fix bug of the reference of "cfg_idx" > > > http://lore.kernel.org/all/202403222227.Sdp23Lcb-lkp@intel.com > > > > > > v4: > > > 1. remove support for names array entries being null > > > 2. remove cfg_idx from virtio_vq_config > > > > > > v3: > > > 1. fix the bug: "assignment of read-only location '*cfg.names'" > > > > > > v2: > > > 1. add kerneldoc for "struct vq_transport_config" @ilpo.jarvinen > > > > > > v1: > > > 1. fix some comments from ilpo.jarvinen@linux.intel.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Xuan Zhuo (6): > > > virtio_balloon: remove the dependence where names[] is null > > > virtio: remove support for names array entries being null. > > > virtio: find_vqs: pass struct instead of multi parameters > > > virtio: vring_create_virtqueue: pass struct instead of multi > > > parameters > > > virtio: vring_new_virtqueue(): pass struct instead of multi parameters > > > virtio_ring: simplify the parameters of the funcs related to > > > vring_create/new_virtqueue() > > > > > > arch/um/drivers/virtio_uml.c | 36 +++-- > > > drivers/platform/mellanox/mlxbf-tmfifo.c | 23 +-- > > > drivers/remoteproc/remoteproc_virtio.c | 37 +++-- > > > drivers/s390/virtio/virtio_ccw.c | 38 ++--- > > > drivers/virtio/virtio_balloon.c | 48 +++--- > > > drivers/virtio/virtio_mmio.c | 36 +++-- > > > drivers/virtio/virtio_pci_common.c | 69 ++++----- > > > drivers/virtio/virtio_pci_common.h | 9 +- > > > drivers/virtio/virtio_pci_legacy.c | 16 +- > > > drivers/virtio/virtio_pci_modern.c | 37 +++-- > > > drivers/virtio/virtio_ring.c | 177 ++++++++--------------- > > > drivers/virtio/virtio_vdpa.c | 51 +++---- > > > include/linux/virtio_config.h | 76 +++++++--- > > > include/linux/virtio_ring.h | 93 +++++++----- > > > tools/virtio/virtio_test.c | 4 +- > > > tools/virtio/vringh_test.c | 28 ++-- > > > 16 files changed, 384 insertions(+), 394 deletions(-) > > > > > > -- > > > 2.32.0.3.g01195cf9f > >
On Wed, May 22, 2024 at 08:43:52AM -0400, Michael S. Tsirkin wrote: > On Wed, May 22, 2024 at 08:35:21PM +0800, Xuan Zhuo wrote: > > On Wed, 22 May 2024 08:28:43 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Wed, Apr 24, 2024 at 05:15:27PM +0800, Xuan Zhuo wrote: > > > > This pathset is splited from the > > > > > > > > http://lore.kernel.org/all/20240229072044.77388-1-xuanzhuo@linux.alibaba.com > > > > > > > > That may needs some cycles to discuss. But that notifies too many people. > > > > > > > > But just the four commits need to notify so many people. > > > > And four commits are independent. So I split that patch set, > > > > let us review these first. > > > > > > > > The patch set try to refactor the params of find_vqs(). > > > > Then we can just change the structure, when introducing new > > > > features. > > > > > > > > Thanks. > > > > > > It's nice but I'd like to see something that uses this before I bother > > > merging. IIUC premapped is dropped - are we going to use this in practice? > > > > > > 1. You know this modification makes sense. > > 2. This modification is difficult. Unlike modifying virtio ring or virtio-net, > > this patch set requires modifying many modules and being reviewed by > > many people. > > 3. If you do not merge it now, then this patch set will most likely be > > abandoned. And I worked a lot on that. > > 4. premapped has not been abandoned, I have been advancing this work. What was > > abandoned was just virtio-net big mode's support for premapped. > > 5. My plan is to complete virtio-net support for af-xdp in 6.10. This must > > depend on premapped. > > > > So, I hope you merge this patch set. > > > > Thanks. > > If this makes thing easier for you, I can put it in my tree post > release. I meant post -rc1. > This way you do not need to keep reposting it. > I'll then merge it with premapped. > > > > > > > > > > > v8: > > > > 1. rebase the vhost branch > > > > > > > > v7: > > > > 1. fix two bugs. @Jason > > > > > > > > v6: > > > > 1. virtio_balloon: a single variable for both purposes. > > > > 2. if names[i] is null, return error > > > > > > > > v5: > > > > 1. virtio_balloon: follow David Hildenbrand's suggest > > > > http://lore.kernel.org/all/3620be9c-e288-4ff2-a7be-1fcf806e6e6e@redhat.com > > > > 2. fix bug of the reference of "cfg_idx" > > > > http://lore.kernel.org/all/202403222227.Sdp23Lcb-lkp@intel.com > > > > > > > > v4: > > > > 1. remove support for names array entries being null > > > > 2. remove cfg_idx from virtio_vq_config > > > > > > > > v3: > > > > 1. fix the bug: "assignment of read-only location '*cfg.names'" > > > > > > > > v2: > > > > 1. add kerneldoc for "struct vq_transport_config" @ilpo.jarvinen > > > > > > > > v1: > > > > 1. fix some comments from ilpo.jarvinen@linux.intel.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Xuan Zhuo (6): > > > > virtio_balloon: remove the dependence where names[] is null > > > > virtio: remove support for names array entries being null. > > > > virtio: find_vqs: pass struct instead of multi parameters > > > > virtio: vring_create_virtqueue: pass struct instead of multi > > > > parameters > > > > virtio: vring_new_virtqueue(): pass struct instead of multi parameters > > > > virtio_ring: simplify the parameters of the funcs related to > > > > vring_create/new_virtqueue() > > > > > > > > arch/um/drivers/virtio_uml.c | 36 +++-- > > > > drivers/platform/mellanox/mlxbf-tmfifo.c | 23 +-- > > > > drivers/remoteproc/remoteproc_virtio.c | 37 +++-- > > > > drivers/s390/virtio/virtio_ccw.c | 38 ++--- > > > > drivers/virtio/virtio_balloon.c | 48 +++--- > > > > drivers/virtio/virtio_mmio.c | 36 +++-- > > > > drivers/virtio/virtio_pci_common.c | 69 ++++----- > > > > drivers/virtio/virtio_pci_common.h | 9 +- > > > > drivers/virtio/virtio_pci_legacy.c | 16 +- > > > > drivers/virtio/virtio_pci_modern.c | 37 +++-- > > > > drivers/virtio/virtio_ring.c | 177 ++++++++--------------- > > > > drivers/virtio/virtio_vdpa.c | 51 +++---- > > > > include/linux/virtio_config.h | 76 +++++++--- > > > > include/linux/virtio_ring.h | 93 +++++++----- > > > > tools/virtio/virtio_test.c | 4 +- > > > > tools/virtio/vringh_test.c | 28 ++-- > > > > 16 files changed, 384 insertions(+), 394 deletions(-) > > > > > > > > -- > > > > 2.32.0.3.g01195cf9f > > >
This pathset is splited from the http://lore.kernel.org/all/20240229072044.77388-1-xuanzhuo@linux.alibaba.com That may needs some cycles to discuss. But that notifies too many people. But just the four commits need to notify so many people. And four commits are independent. So I split that patch set, let us review these first. The patch set try to refactor the params of find_vqs(). Then we can just change the structure, when introducing new features. Thanks. v8: 1. rebase the vhost branch v7: 1. fix two bugs. @Jason v6: 1. virtio_balloon: a single variable for both purposes. 2. if names[i] is null, return error v5: 1. virtio_balloon: follow David Hildenbrand's suggest http://lore.kernel.org/all/3620be9c-e288-4ff2-a7be-1fcf806e6e6e@redhat.com 2. fix bug of the reference of "cfg_idx" http://lore.kernel.org/all/202403222227.Sdp23Lcb-lkp@intel.com v4: 1. remove support for names array entries being null 2. remove cfg_idx from virtio_vq_config v3: 1. fix the bug: "assignment of read-only location '*cfg.names'" v2: 1. add kerneldoc for "struct vq_transport_config" @ilpo.jarvinen v1: 1. fix some comments from ilpo.jarvinen@linux.intel.com Xuan Zhuo (6): virtio_balloon: remove the dependence where names[] is null virtio: remove support for names array entries being null. virtio: find_vqs: pass struct instead of multi parameters virtio: vring_create_virtqueue: pass struct instead of multi parameters virtio: vring_new_virtqueue(): pass struct instead of multi parameters virtio_ring: simplify the parameters of the funcs related to vring_create/new_virtqueue() arch/um/drivers/virtio_uml.c | 36 +++-- drivers/platform/mellanox/mlxbf-tmfifo.c | 23 +-- drivers/remoteproc/remoteproc_virtio.c | 37 +++-- drivers/s390/virtio/virtio_ccw.c | 38 ++--- drivers/virtio/virtio_balloon.c | 48 +++--- drivers/virtio/virtio_mmio.c | 36 +++-- drivers/virtio/virtio_pci_common.c | 69 ++++----- drivers/virtio/virtio_pci_common.h | 9 +- drivers/virtio/virtio_pci_legacy.c | 16 +- drivers/virtio/virtio_pci_modern.c | 37 +++-- drivers/virtio/virtio_ring.c | 177 ++++++++--------------- drivers/virtio/virtio_vdpa.c | 51 +++---- include/linux/virtio_config.h | 76 +++++++--- include/linux/virtio_ring.h | 93 +++++++----- tools/virtio/virtio_test.c | 4 +- tools/virtio/vringh_test.c | 28 ++-- 16 files changed, 384 insertions(+), 394 deletions(-)