Re: [PATCH] vdpa: move to drivers/vdpa
On 3/31/20 12:19 PM, Michael S. Tsirkin wrote:
> We have both vhost and virtio drivers that depend on vdpa.
> It's easier to locate it at a top level directory otherwise
> we run into issues e.g. if vhost is built-in but virtio
> is modular. Let's just move it up a level.
>
> Reported-by: Randy Dunlap
> Signed-off-by: Michael S. Tsirkin
> ---
>
> Randy I'd say the issue you are reporting (vhost=y, virtio=m)
> is esoteric enough not to require a rebase for this.
> So I'd just apply this on top.
> Do you agree?
Sure, looks fine from just reading it.
I haven't had a chance to test it yet.
Thanks.
> MAINTAINERS | 1 +
> drivers/Kconfig | 2 ++
> drivers/Makefile | 1 +
> drivers/{virtio => }/vdpa/Kconfig | 0
> drivers/{virtio => }/vdpa/Makefile| 0
> drivers/{virtio => }/vdpa/ifcvf/Makefile | 0
> drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.c | 0
> drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.h | 0
> drivers/{virtio => }/vdpa/ifcvf/ifcvf_main.c | 0
> drivers/{virtio => }/vdpa/vdpa.c | 0
> drivers/{virtio => }/vdpa/vdpa_sim/Makefile | 0
> drivers/{virtio => }/vdpa/vdpa_sim/vdpa_sim.c | 0
> drivers/virtio/Kconfig| 2 --
> 13 files changed, 4 insertions(+), 2 deletions(-)
> rename drivers/{virtio => }/vdpa/Kconfig (100%)
> rename drivers/{virtio => }/vdpa/Makefile (100%)
> rename drivers/{virtio => }/vdpa/ifcvf/Makefile (100%)
> rename drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.c (100%)
> rename drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.h (100%)
> rename drivers/{virtio => }/vdpa/ifcvf/ifcvf_main.c (100%)
> rename drivers/{virtio => }/vdpa/vdpa.c (100%)
> rename drivers/{virtio => }/vdpa/vdpa_sim/Makefile (100%)
> rename drivers/{virtio => }/vdpa/vdpa_sim/vdpa_sim.c (100%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 70c47bc55343..7cfa55c765fd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17695,6 +17695,7 @@ L:virtualization@lists.linux-foundation.org
> S: Maintained
> F: Documentation/devicetree/bindings/virtio/
> F: drivers/virtio/
> +F: drivers/vdpa/
> F: tools/virtio/
> F: drivers/net/virtio_net.c
> F: drivers/block/virtio_blk.c
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 7a6d8b2b68b4..ac23d520e916 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -138,6 +138,8 @@ source "drivers/virt/Kconfig"
>
> source "drivers/virtio/Kconfig"
>
> +source "drivers/vdpa/Kconfig"
> +
> source "drivers/vhost/Kconfig"
>
> source "drivers/hv/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 31cf17dee252..21688f3b1588 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_DMADEVICES)+= dma/
> obj-y+= soc/
>
> obj-$(CONFIG_VIRTIO) += virtio/
> +obj-$(CONFIG_VDPA) += vdpa/
> obj-$(CONFIG_XEN)+= xen/
>
> # regulators early, since some subsystems rely on them to initialize
> diff --git a/drivers/virtio/vdpa/Kconfig b/drivers/vdpa/Kconfig
> similarity index 100%
> rename from drivers/virtio/vdpa/Kconfig
> rename to drivers/vdpa/Kconfig
> diff --git a/drivers/virtio/vdpa/Makefile b/drivers/vdpa/Makefile
> similarity index 100%
> rename from drivers/virtio/vdpa/Makefile
> rename to drivers/vdpa/Makefile
> diff --git a/drivers/virtio/vdpa/ifcvf/Makefile b/drivers/vdpa/ifcvf/Makefile
> similarity index 100%
> rename from drivers/virtio/vdpa/ifcvf/Makefile
> rename to drivers/vdpa/ifcvf/Makefile
> diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
> b/drivers/vdpa/ifcvf/ifcvf_base.c
> similarity index 100%
> rename from drivers/virtio/vdpa/ifcvf/ifcvf_base.c
> rename to drivers/vdpa/ifcvf/ifcvf_base.c
> diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_base.h
> b/drivers/vdpa/ifcvf/ifcvf_base.h
> similarity index 100%
> rename from drivers/virtio/vdpa/ifcvf/ifcvf_base.h
> rename to drivers/vdpa/ifcvf/ifcvf_base.h
> diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
> b/drivers/vdpa/ifcvf/ifcvf_main.c
> similarity index 100%
> rename from drivers/virtio/vdpa/ifcvf/ifcvf_main.c
> rename to drivers/vdpa/ifcvf/ifcvf_main.c
> diff --git a/drivers/virtio/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> similarity index 100%
> rename from drivers/virtio/vdpa/vdpa.c
> rename to drivers/vdpa/vdpa.c
> diff --git a/drivers/virtio/vdpa/vdpa_sim/Makefile
> b/drivers/vdpa/vdpa_sim/Makefile
> similarity index 100%
> rename from drivers/virtio/vdpa/vdpa_sim/Makefile
> rename to drivers/vdpa/vdpa_sim/Makefile
> diff --git a/drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
> b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> similarity index 100%
> rename from drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
> rename to drivers/vdpa/vdpa_sim/vdpa_sim.c
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 99e424570644..2aadf398d8cc 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@
Re: [PATCH v3 0/8] vhost: Reset batched descriptors on SET_VRING_BASE call
On Tue, Mar 31, 2020 at 09:27:56PM +0200, Eugenio Pérez wrote: > Vhost did not reset properly the batched descriptors on SET_VRING_BASE > event. Because of that, is possible to return an invalid descriptor to > the guest. > > This series ammend this, resetting them every time backend changes, and > creates a test to assert correct behavior. To do that, they need to > expose a new function in virtio_ring, virtqueue_reset_free_head, only > on test code. > > Another useful thing would be to check if mutex is properly get in > vq private_data accessors. Not sure if mutex debug code allow that, > similar to C++ unique lock::owns_lock. Not acquiring in the function > because caller code holds the mutex in order to perform more actions. I pushed vhost branch with patch 1, pls send patches on top of that! > v3: > * Rename accesors functions. > * Make scsi and test use the accesors too. > > v2: > * Squashed commits. > * Create vq private_data accesors (mst). > > This is meant to be applied on top of > c4f1c41a6094582903c75c0dcfacb453c959d457 in > git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git. > > Eugenio Pérez (5): > vhost: Create accessors for virtqueues private_data > tools/virtio: Add --batch option > tools/virtio: Add --batch=random option > tools/virtio: Add --reset=random > tools/virtio: Make --reset reset ring idx > > Michael S. Tsirkin (3): > vhost: option to fetch descriptors through an independent struct > vhost: use batched version by default > vhost: batching fetches > > drivers/vhost/net.c | 28 ++-- > drivers/vhost/scsi.c | 14 +- > drivers/vhost/test.c | 69 - > drivers/vhost/test.h | 1 + > drivers/vhost/vhost.c| 271 +++ > drivers/vhost/vhost.h| 44 +- > drivers/vhost/vsock.c| 14 +- > drivers/virtio/virtio_ring.c | 29 > tools/virtio/linux/virtio.h | 2 + > tools/virtio/virtio_test.c | 123 ++-- > 10 files changed, 456 insertions(+), 139 deletions(-) > > -- > 2.18.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: mmotm 2020-03-30-18-46 uploaded (VDPA + vhost)
On Tue, Mar 31, 2020 at 11:42:47AM -0700, Randy Dunlap wrote: > On 3/31/20 11:37 AM, Michael S. Tsirkin wrote: > > On Tue, Mar 31, 2020 at 11:27:54AM -0700, Randy Dunlap wrote: > >> On 3/30/20 6:47 PM, a...@linux-foundation.org wrote: > >>> The mm-of-the-moment snapshot 2020-03-30-18-46 has been uploaded to > >>> > >>>http://www.ozlabs.org/~akpm/mmotm/ > >>> > >>> mmotm-readme.txt says > >>> > >>> README for mm-of-the-moment: > >>> > >>> http://www.ozlabs.org/~akpm/mmotm/ > >>> > >>> This is a snapshot of my -mm patch queue. Uploaded at random hopefully > >>> more than once a week. > >>> > >>> You will need quilt to apply these patches to the latest Linus release > >>> (5.x > >>> or 5.x-rcY). The series file is in broken-out.tar.gz and is duplicated in > >>> http://ozlabs.org/~akpm/mmotm/series > >>> > >>> The file broken-out.tar.gz contains two datestamp files: .DATE and > >>> .DATE--mm-dd-hh-mm-ss. Both contain the string -mm-dd-hh-mm-ss, > >>> followed by the base kernel version against which this patch series is to > >>> be applied. > >>> > >>> This tree is partially included in linux-next. To see which patches are > >>> included in linux-next, consult the `series' file. Only the patches > >>> within the #NEXT_PATCHES_START/#NEXT_PATCHES_END markers are included in > >>> linux-next. > >>> > >>> > >>> A full copy of the full kernel tree with the linux-next and mmotm patches > >>> already applied is available through git within an hour of the mmotm > >>> release. Individual mmotm releases are tagged. The master branch always > >>> points to the latest release, so it's constantly rebasing. > >>> > >>> https://github.com/hnaz/linux-mm > >> > >> on i386: > >> > >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_init': > >> vdpa.c:(.init.text+0x52): undefined reference to `__vdpa_register_driver' > >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_exit': > >> vdpa.c:(.exit.text+0x14): undefined reference to `vdpa_unregister_driver' > >> > >> > >> > >> drivers/virtio/vdpa/ is not being built. (confusing!) > >> > >> CONFIG_VIRTIO=m > >> # CONFIG_VIRTIO_MENU is not set > >> CONFIG_VDPA=y > > > > Hmm. OK. Can't figure it out. CONFIG_VDPA is set why isn't > > drivers/virtio/vdpa/ built? > > we have: > > > > Ack. Hopefully Yamada-san can tell us what is happening here. OK I pushed a fix (moving the vdpa subsystem up a level) and pushed into my tree, refs/heads/next . Seems to build fine now, but I'd appreciate it if you can give it a spin. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vdpa: move to drivers/vdpa
We have both vhost and virtio drivers that depend on vdpa.
It's easier to locate it at a top level directory otherwise
we run into issues e.g. if vhost is built-in but virtio
is modular. Let's just move it up a level.
Reported-by: Randy Dunlap
Signed-off-by: Michael S. Tsirkin
---
Randy I'd say the issue you are reporting (vhost=y, virtio=m)
is esoteric enough not to require a rebase for this.
So I'd just apply this on top.
Do you agree?
MAINTAINERS | 1 +
drivers/Kconfig | 2 ++
drivers/Makefile | 1 +
drivers/{virtio => }/vdpa/Kconfig | 0
drivers/{virtio => }/vdpa/Makefile| 0
drivers/{virtio => }/vdpa/ifcvf/Makefile | 0
drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.c | 0
drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.h | 0
drivers/{virtio => }/vdpa/ifcvf/ifcvf_main.c | 0
drivers/{virtio => }/vdpa/vdpa.c | 0
drivers/{virtio => }/vdpa/vdpa_sim/Makefile | 0
drivers/{virtio => }/vdpa/vdpa_sim/vdpa_sim.c | 0
drivers/virtio/Kconfig| 2 --
13 files changed, 4 insertions(+), 2 deletions(-)
rename drivers/{virtio => }/vdpa/Kconfig (100%)
rename drivers/{virtio => }/vdpa/Makefile (100%)
rename drivers/{virtio => }/vdpa/ifcvf/Makefile (100%)
rename drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.c (100%)
rename drivers/{virtio => }/vdpa/ifcvf/ifcvf_base.h (100%)
rename drivers/{virtio => }/vdpa/ifcvf/ifcvf_main.c (100%)
rename drivers/{virtio => }/vdpa/vdpa.c (100%)
rename drivers/{virtio => }/vdpa/vdpa_sim/Makefile (100%)
rename drivers/{virtio => }/vdpa/vdpa_sim/vdpa_sim.c (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 70c47bc55343..7cfa55c765fd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17695,6 +17695,7 @@ L: virtualization@lists.linux-foundation.org
S: Maintained
F: Documentation/devicetree/bindings/virtio/
F: drivers/virtio/
+F: drivers/vdpa/
F: tools/virtio/
F: drivers/net/virtio_net.c
F: drivers/block/virtio_blk.c
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 7a6d8b2b68b4..ac23d520e916 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -138,6 +138,8 @@ source "drivers/virt/Kconfig"
source "drivers/virtio/Kconfig"
+source "drivers/vdpa/Kconfig"
+
source "drivers/vhost/Kconfig"
source "drivers/hv/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 31cf17dee252..21688f3b1588 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_DMADEVICES) += dma/
obj-y += soc/
obj-$(CONFIG_VIRTIO) += virtio/
+obj-$(CONFIG_VDPA) += vdpa/
obj-$(CONFIG_XEN) += xen/
# regulators early, since some subsystems rely on them to initialize
diff --git a/drivers/virtio/vdpa/Kconfig b/drivers/vdpa/Kconfig
similarity index 100%
rename from drivers/virtio/vdpa/Kconfig
rename to drivers/vdpa/Kconfig
diff --git a/drivers/virtio/vdpa/Makefile b/drivers/vdpa/Makefile
similarity index 100%
rename from drivers/virtio/vdpa/Makefile
rename to drivers/vdpa/Makefile
diff --git a/drivers/virtio/vdpa/ifcvf/Makefile b/drivers/vdpa/ifcvf/Makefile
similarity index 100%
rename from drivers/virtio/vdpa/ifcvf/Makefile
rename to drivers/vdpa/ifcvf/Makefile
diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
b/drivers/vdpa/ifcvf/ifcvf_base.c
similarity index 100%
rename from drivers/virtio/vdpa/ifcvf/ifcvf_base.c
rename to drivers/vdpa/ifcvf/ifcvf_base.c
diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_base.h
b/drivers/vdpa/ifcvf/ifcvf_base.h
similarity index 100%
rename from drivers/virtio/vdpa/ifcvf/ifcvf_base.h
rename to drivers/vdpa/ifcvf/ifcvf_base.h
diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
b/drivers/vdpa/ifcvf/ifcvf_main.c
similarity index 100%
rename from drivers/virtio/vdpa/ifcvf/ifcvf_main.c
rename to drivers/vdpa/ifcvf/ifcvf_main.c
diff --git a/drivers/virtio/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
similarity index 100%
rename from drivers/virtio/vdpa/vdpa.c
rename to drivers/vdpa/vdpa.c
diff --git a/drivers/virtio/vdpa/vdpa_sim/Makefile
b/drivers/vdpa/vdpa_sim/Makefile
similarity index 100%
rename from drivers/virtio/vdpa/vdpa_sim/Makefile
rename to drivers/vdpa/vdpa_sim/Makefile
diff --git a/drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
b/drivers/vdpa/vdpa_sim/vdpa_sim.c
similarity index 100%
rename from drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
rename to drivers/vdpa/vdpa_sim/vdpa_sim.c
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 99e424570644..2aadf398d8cc 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -109,5 +109,3 @@ config VIRTIO_MMIO_CMDLINE_DEVICES
If unsure, say 'N'.
endif # VIRTIO_MENU
-
-source "drivers/virtio/vdpa/Kconfig"
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: mmotm 2020-03-30-18-46 uploaded (VDPA + vhost)
On Tue, Mar 31, 2020 at 11:42:47AM -0700, Randy Dunlap wrote: > On 3/31/20 11:37 AM, Michael S. Tsirkin wrote: > > On Tue, Mar 31, 2020 at 11:27:54AM -0700, Randy Dunlap wrote: > >> On 3/30/20 6:47 PM, a...@linux-foundation.org wrote: > >>> The mm-of-the-moment snapshot 2020-03-30-18-46 has been uploaded to > >>> > >>>http://www.ozlabs.org/~akpm/mmotm/ > >>> > >>> mmotm-readme.txt says > >>> > >>> README for mm-of-the-moment: > >>> > >>> http://www.ozlabs.org/~akpm/mmotm/ > >>> > >>> This is a snapshot of my -mm patch queue. Uploaded at random hopefully > >>> more than once a week. > >>> > >>> You will need quilt to apply these patches to the latest Linus release > >>> (5.x > >>> or 5.x-rcY). The series file is in broken-out.tar.gz and is duplicated in > >>> http://ozlabs.org/~akpm/mmotm/series > >>> > >>> The file broken-out.tar.gz contains two datestamp files: .DATE and > >>> .DATE--mm-dd-hh-mm-ss. Both contain the string -mm-dd-hh-mm-ss, > >>> followed by the base kernel version against which this patch series is to > >>> be applied. > >>> > >>> This tree is partially included in linux-next. To see which patches are > >>> included in linux-next, consult the `series' file. Only the patches > >>> within the #NEXT_PATCHES_START/#NEXT_PATCHES_END markers are included in > >>> linux-next. > >>> > >>> > >>> A full copy of the full kernel tree with the linux-next and mmotm patches > >>> already applied is available through git within an hour of the mmotm > >>> release. Individual mmotm releases are tagged. The master branch always > >>> points to the latest release, so it's constantly rebasing. > >>> > >>> https://github.com/hnaz/linux-mm > >> > >> on i386: > >> > >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_init': > >> vdpa.c:(.init.text+0x52): undefined reference to `__vdpa_register_driver' > >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_exit': > >> vdpa.c:(.exit.text+0x14): undefined reference to `vdpa_unregister_driver' > >> > >> > >> > >> drivers/virtio/vdpa/ is not being built. (confusing!) > >> > >> CONFIG_VIRTIO=m > >> # CONFIG_VIRTIO_MENU is not set > >> CONFIG_VDPA=y > > > > Hmm. OK. Can't figure it out. CONFIG_VDPA is set why isn't > > drivers/virtio/vdpa/ built? > > we have: > > > > Ack. Hopefully Yamada-san can tell us what is happening here. Oh wait a second: obj-$(CONFIG_VIRTIO)+= virtio/ So CONFIG_VIRTIO=m and build does not even interate into drivers/virtio. > > > > obj-$(CONFIG_VDPA) += vdpa/ > > > > and under that: > > > > obj-$(CONFIG_VDPA) += vdpa.o > > > > > >> CONFIG_VDPA_MENU=y > >> # CONFIG_VDPA_SIM is not set > >> CONFIG_VHOST_IOTLB=y > >> CONFIG_VHOST_RING=m > >> CONFIG_VHOST=y > >> CONFIG_VHOST_SCSI=m > >> CONFIG_VHOST_VDPA=y > >> > >> Full randconfig file is attached. > >> > >> (This same build failure happens with today's linux-next, Mar. 31.) > >> > >> @Yamada-san: Is this a kbuild problem (or feature)? > >> > >> -- > >> ~Randy > >> Reported-by: Randy Dunlap > > > > > -- > ~Randy > Reported-by: Randy Dunlap ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: mmotm 2020-03-30-18-46 uploaded (VDPA + vhost)
On Tue, Mar 31, 2020 at 11:42:47AM -0700, Randy Dunlap wrote: > On 3/31/20 11:37 AM, Michael S. Tsirkin wrote: > > On Tue, Mar 31, 2020 at 11:27:54AM -0700, Randy Dunlap wrote: > >> On 3/30/20 6:47 PM, a...@linux-foundation.org wrote: > >>> The mm-of-the-moment snapshot 2020-03-30-18-46 has been uploaded to > >>> > >>>http://www.ozlabs.org/~akpm/mmotm/ > >>> > >>> mmotm-readme.txt says > >>> > >>> README for mm-of-the-moment: > >>> > >>> http://www.ozlabs.org/~akpm/mmotm/ > >>> > >>> This is a snapshot of my -mm patch queue. Uploaded at random hopefully > >>> more than once a week. > >>> > >>> You will need quilt to apply these patches to the latest Linus release > >>> (5.x > >>> or 5.x-rcY). The series file is in broken-out.tar.gz and is duplicated in > >>> http://ozlabs.org/~akpm/mmotm/series > >>> > >>> The file broken-out.tar.gz contains two datestamp files: .DATE and > >>> .DATE--mm-dd-hh-mm-ss. Both contain the string -mm-dd-hh-mm-ss, > >>> followed by the base kernel version against which this patch series is to > >>> be applied. > >>> > >>> This tree is partially included in linux-next. To see which patches are > >>> included in linux-next, consult the `series' file. Only the patches > >>> within the #NEXT_PATCHES_START/#NEXT_PATCHES_END markers are included in > >>> linux-next. > >>> > >>> > >>> A full copy of the full kernel tree with the linux-next and mmotm patches > >>> already applied is available through git within an hour of the mmotm > >>> release. Individual mmotm releases are tagged. The master branch always > >>> points to the latest release, so it's constantly rebasing. > >>> > >>> https://github.com/hnaz/linux-mm > >> > >> on i386: > >> > >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_init': > >> vdpa.c:(.init.text+0x52): undefined reference to `__vdpa_register_driver' > >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_exit': > >> vdpa.c:(.exit.text+0x14): undefined reference to `vdpa_unregister_driver' > >> > >> > >> > >> drivers/virtio/vdpa/ is not being built. (confusing!) > >> > >> CONFIG_VIRTIO=m > >> # CONFIG_VIRTIO_MENU is not set > >> CONFIG_VDPA=y > > > > Hmm. OK. Can't figure it out. CONFIG_VDPA is set why isn't > > drivers/virtio/vdpa/ built? > > we have: > > > > Ack. Hopefully Yamada-san can tell us what is happening here. I reproduced this without difficulty btw, thanks for the report! > > > > obj-$(CONFIG_VDPA) += vdpa/ > > > > and under that: > > > > obj-$(CONFIG_VDPA) += vdpa.o > > > > > >> CONFIG_VDPA_MENU=y > >> # CONFIG_VDPA_SIM is not set > >> CONFIG_VHOST_IOTLB=y > >> CONFIG_VHOST_RING=m > >> CONFIG_VHOST=y > >> CONFIG_VHOST_SCSI=m > >> CONFIG_VHOST_VDPA=y > >> > >> Full randconfig file is attached. > >> > >> (This same build failure happens with today's linux-next, Mar. 31.) > >> > >> @Yamada-san: Is this a kbuild problem (or feature)? > >> > >> -- > >> ~Randy > >> Reported-by: Randy Dunlap > > > > > -- > ~Randy > Reported-by: Randy Dunlap ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: mmotm 2020-03-30-18-46 uploaded (VDPA + vhost)
On 3/31/20 11:37 AM, Michael S. Tsirkin wrote: > On Tue, Mar 31, 2020 at 11:27:54AM -0700, Randy Dunlap wrote: >> On 3/30/20 6:47 PM, a...@linux-foundation.org wrote: >>> The mm-of-the-moment snapshot 2020-03-30-18-46 has been uploaded to >>> >>>http://www.ozlabs.org/~akpm/mmotm/ >>> >>> mmotm-readme.txt says >>> >>> README for mm-of-the-moment: >>> >>> http://www.ozlabs.org/~akpm/mmotm/ >>> >>> This is a snapshot of my -mm patch queue. Uploaded at random hopefully >>> more than once a week. >>> >>> You will need quilt to apply these patches to the latest Linus release (5.x >>> or 5.x-rcY). The series file is in broken-out.tar.gz and is duplicated in >>> http://ozlabs.org/~akpm/mmotm/series >>> >>> The file broken-out.tar.gz contains two datestamp files: .DATE and >>> .DATE--mm-dd-hh-mm-ss. Both contain the string -mm-dd-hh-mm-ss, >>> followed by the base kernel version against which this patch series is to >>> be applied. >>> >>> This tree is partially included in linux-next. To see which patches are >>> included in linux-next, consult the `series' file. Only the patches >>> within the #NEXT_PATCHES_START/#NEXT_PATCHES_END markers are included in >>> linux-next. >>> >>> >>> A full copy of the full kernel tree with the linux-next and mmotm patches >>> already applied is available through git within an hour of the mmotm >>> release. Individual mmotm releases are tagged. The master branch always >>> points to the latest release, so it's constantly rebasing. >>> >>> https://github.com/hnaz/linux-mm >> >> on i386: >> >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_init': >> vdpa.c:(.init.text+0x52): undefined reference to `__vdpa_register_driver' >> ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_exit': >> vdpa.c:(.exit.text+0x14): undefined reference to `vdpa_unregister_driver' >> >> >> >> drivers/virtio/vdpa/ is not being built. (confusing!) >> >> CONFIG_VIRTIO=m >> # CONFIG_VIRTIO_MENU is not set >> CONFIG_VDPA=y > > Hmm. OK. Can't figure it out. CONFIG_VDPA is set why isn't > drivers/virtio/vdpa/ built? > we have: > Ack. Hopefully Yamada-san can tell us what is happening here. > > obj-$(CONFIG_VDPA) += vdpa/ > > and under that: > > obj-$(CONFIG_VDPA) += vdpa.o > > >> CONFIG_VDPA_MENU=y >> # CONFIG_VDPA_SIM is not set >> CONFIG_VHOST_IOTLB=y >> CONFIG_VHOST_RING=m >> CONFIG_VHOST=y >> CONFIG_VHOST_SCSI=m >> CONFIG_VHOST_VDPA=y >> >> Full randconfig file is attached. >> >> (This same build failure happens with today's linux-next, Mar. 31.) >> >> @Yamada-san: Is this a kbuild problem (or feature)? >> >> -- >> ~Randy >> Reported-by: Randy Dunlap > -- ~Randy Reported-by: Randy Dunlap ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: mmotm 2020-03-30-18-46 uploaded (VDPA + vhost)
On Tue, Mar 31, 2020 at 11:27:54AM -0700, Randy Dunlap wrote: > On 3/30/20 6:47 PM, a...@linux-foundation.org wrote: > > The mm-of-the-moment snapshot 2020-03-30-18-46 has been uploaded to > > > >http://www.ozlabs.org/~akpm/mmotm/ > > > > mmotm-readme.txt says > > > > README for mm-of-the-moment: > > > > http://www.ozlabs.org/~akpm/mmotm/ > > > > This is a snapshot of my -mm patch queue. Uploaded at random hopefully > > more than once a week. > > > > You will need quilt to apply these patches to the latest Linus release (5.x > > or 5.x-rcY). The series file is in broken-out.tar.gz and is duplicated in > > http://ozlabs.org/~akpm/mmotm/series > > > > The file broken-out.tar.gz contains two datestamp files: .DATE and > > .DATE--mm-dd-hh-mm-ss. Both contain the string -mm-dd-hh-mm-ss, > > followed by the base kernel version against which this patch series is to > > be applied. > > > > This tree is partially included in linux-next. To see which patches are > > included in linux-next, consult the `series' file. Only the patches > > within the #NEXT_PATCHES_START/#NEXT_PATCHES_END markers are included in > > linux-next. > > > > > > A full copy of the full kernel tree with the linux-next and mmotm patches > > already applied is available through git within an hour of the mmotm > > release. Individual mmotm releases are tagged. The master branch always > > points to the latest release, so it's constantly rebasing. > > > > https://github.com/hnaz/linux-mm > > on i386: > > ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_init': > vdpa.c:(.init.text+0x52): undefined reference to `__vdpa_register_driver' > ld: drivers/vhost/vdpa.o: in function `vhost_vdpa_exit': > vdpa.c:(.exit.text+0x14): undefined reference to `vdpa_unregister_driver' > > > > drivers/virtio/vdpa/ is not being built. (confusing!) > > CONFIG_VIRTIO=m > # CONFIG_VIRTIO_MENU is not set > CONFIG_VDPA=y Hmm. OK. Can't figure it out. CONFIG_VDPA is set why isn't drivers/virtio/vdpa/ built? we have: obj-$(CONFIG_VDPA) += vdpa/ and under that: obj-$(CONFIG_VDPA) += vdpa.o > CONFIG_VDPA_MENU=y > # CONFIG_VDPA_SIM is not set > CONFIG_VHOST_IOTLB=y > CONFIG_VHOST_RING=m > CONFIG_VHOST=y > CONFIG_VHOST_SCSI=m > CONFIG_VHOST_VDPA=y > > Full randconfig file is attached. > > (This same build failure happens with today's linux-next, Mar. 31.) > > @Yamada-san: Is this a kbuild problem (or feature)? > > -- > ~Randy > Reported-by: Randy Dunlap ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/8] vhost: Create accessors for virtqueues private_data
On Tue, Mar 31, 2020 at 07:59:59PM +0200, Eugenio Pérez wrote:
> Signed-off-by: Eugenio Pérez
> ---
> drivers/vhost/net.c | 28 +++-
> drivers/vhost/vhost.h | 28
> drivers/vhost/vsock.c | 14 +++---
Seems to be missing scsi and test.
> 3 files changed, 50 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index e158159671fa..6c5e7a6f712c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -424,7 +424,7 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> struct vhost_net_virtqueue *nvq =
> container_of(vq, struct vhost_net_virtqueue, vq);
> struct vhost_poll *poll = n->poll + (nvq - n->vqs);
> - if (!vq->private_data)
> + if (!vhost_vq_get_backend_opaque(vq))
> return;
> vhost_poll_stop(poll);
> }
> @@ -437,7 +437,7 @@ static int vhost_net_enable_vq(struct vhost_net *n,
> struct vhost_poll *poll = n->poll + (nvq - n->vqs);
> struct socket *sock;
>
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> if (!sock)
> return 0;
>
> @@ -524,7 +524,7 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> return;
>
> vhost_disable_notify(>dev, vq);
> - sock = rvq->private_data;
> + sock = vhost_vq_get_backend_opaque(rvq);
>
> busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
>tvq->busyloop_timeout;
> @@ -570,8 +570,10 @@ static int vhost_net_tx_get_vq_desc(struct vhost_net
> *net,
>
> if (r == tvq->num && tvq->busyloop_timeout) {
> /* Flush batched packets first */
> - if (!vhost_sock_zcopy(tvq->private_data))
> - vhost_tx_batch(net, tnvq, tvq->private_data, msghdr);
> + if (!vhost_sock_zcopy(vhost_vq_get_backend_opaque(tvq)))
> + vhost_tx_batch(net, tnvq,
> +vhost_vq_get_backend_opaque(tvq),
> +msghdr);
>
> vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, false);
>
> @@ -685,7 +687,7 @@ static int vhost_net_build_xdp(struct vhost_net_virtqueue
> *nvq,
> struct vhost_virtqueue *vq = >vq;
> struct vhost_net *net = container_of(vq->dev, struct vhost_net,
>dev);
> - struct socket *sock = vq->private_data;
> + struct socket *sock = vhost_vq_get_backend_opaque(vq);
> struct page_frag *alloc_frag = >page_frag;
> struct virtio_net_hdr *gso;
> struct xdp_buff *xdp = >xdp[nvq->batched_xdp];
> @@ -952,7 +954,7 @@ static void handle_tx(struct vhost_net *net)
> struct socket *sock;
>
> mutex_lock_nested(>mutex, VHOST_NET_VQ_TX);
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> if (!sock)
> goto out;
>
> @@ -1121,7 +1123,7 @@ static void handle_rx(struct vhost_net *net)
> int recv_pkts = 0;
>
> mutex_lock_nested(>mutex, VHOST_NET_VQ_RX);
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> if (!sock)
> goto out;
>
> @@ -1344,9 +1346,9 @@ static struct socket *vhost_net_stop_vq(struct
> vhost_net *n,
> container_of(vq, struct vhost_net_virtqueue, vq);
>
> mutex_lock(>mutex);
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> vhost_net_disable_vq(n, vq);
> - vq->private_data = NULL;
> + vhost_vq_set_backend_opaque(vq, NULL);
> vhost_net_buf_unproduce(nvq);
> nvq->rx_ring = NULL;
> mutex_unlock(>mutex);
> @@ -1528,7 +1530,7 @@ static long vhost_net_set_backend(struct vhost_net *n,
> unsigned index, int fd)
> }
>
> /* start polling new socket */
> - oldsock = vq->private_data;
> + oldsock = vhost_vq_get_backend_opaque(vq);
> if (sock != oldsock) {
> ubufs = vhost_net_ubuf_alloc(vq,
>sock && vhost_sock_zcopy(sock));
> @@ -1538,7 +1540,7 @@ static long vhost_net_set_backend(struct vhost_net *n,
> unsigned index, int fd)
> }
>
> vhost_net_disable_vq(n, vq);
> - vq->private_data = sock;
> + vhost_vq_set_backend_opaque(vq, sock);
> vhost_net_buf_unproduce(nvq);
> r = vhost_vq_init_access(vq);
> if (r)
> @@ -1575,7 +1577,7 @@ static long vhost_net_set_backend(struct vhost_net *n,
> unsigned index, int fd)
> return 0;
>
> err_used:
> - vq->private_data = oldsock;
> + vhost_vq_set_backend_opaque(vq, oldsock);
> vhost_net_enable_vq(n, vq);
> if (ubufs)
> vhost_net_ubuf_put_wait_and_free(ubufs);
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index a123fd70847e..0808188f7e8f 100644
> ---
Re: [PATCH v2 1/8] vhost: Create accessors for virtqueues private_data
On Tue, Mar 31, 2020 at 07:59:59PM +0200, Eugenio Pérez wrote:
> Signed-off-by: Eugenio Pérez
> ---
> drivers/vhost/net.c | 28 +++-
> drivers/vhost/vhost.h | 28
> drivers/vhost/vsock.c | 14 +++---
> 3 files changed, 50 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index e158159671fa..6c5e7a6f712c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -424,7 +424,7 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> struct vhost_net_virtqueue *nvq =
> container_of(vq, struct vhost_net_virtqueue, vq);
> struct vhost_poll *poll = n->poll + (nvq - n->vqs);
> - if (!vq->private_data)
> + if (!vhost_vq_get_backend_opaque(vq))
> return;
> vhost_poll_stop(poll);
> }
> @@ -437,7 +437,7 @@ static int vhost_net_enable_vq(struct vhost_net *n,
> struct vhost_poll *poll = n->poll + (nvq - n->vqs);
> struct socket *sock;
>
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> if (!sock)
> return 0;
>
> @@ -524,7 +524,7 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> return;
>
> vhost_disable_notify(>dev, vq);
> - sock = rvq->private_data;
> + sock = vhost_vq_get_backend_opaque(rvq);
>
> busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
>tvq->busyloop_timeout;
> @@ -570,8 +570,10 @@ static int vhost_net_tx_get_vq_desc(struct vhost_net
> *net,
>
> if (r == tvq->num && tvq->busyloop_timeout) {
> /* Flush batched packets first */
> - if (!vhost_sock_zcopy(tvq->private_data))
> - vhost_tx_batch(net, tnvq, tvq->private_data, msghdr);
> + if (!vhost_sock_zcopy(vhost_vq_get_backend_opaque(tvq)))
> + vhost_tx_batch(net, tnvq,
> +vhost_vq_get_backend_opaque(tvq),
> +msghdr);
>
> vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, false);
>
> @@ -685,7 +687,7 @@ static int vhost_net_build_xdp(struct vhost_net_virtqueue
> *nvq,
> struct vhost_virtqueue *vq = >vq;
> struct vhost_net *net = container_of(vq->dev, struct vhost_net,
>dev);
> - struct socket *sock = vq->private_data;
> + struct socket *sock = vhost_vq_get_backend_opaque(vq);
> struct page_frag *alloc_frag = >page_frag;
> struct virtio_net_hdr *gso;
> struct xdp_buff *xdp = >xdp[nvq->batched_xdp];
> @@ -952,7 +954,7 @@ static void handle_tx(struct vhost_net *net)
> struct socket *sock;
>
> mutex_lock_nested(>mutex, VHOST_NET_VQ_TX);
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> if (!sock)
> goto out;
>
> @@ -1121,7 +1123,7 @@ static void handle_rx(struct vhost_net *net)
> int recv_pkts = 0;
>
> mutex_lock_nested(>mutex, VHOST_NET_VQ_RX);
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> if (!sock)
> goto out;
>
> @@ -1344,9 +1346,9 @@ static struct socket *vhost_net_stop_vq(struct
> vhost_net *n,
> container_of(vq, struct vhost_net_virtqueue, vq);
>
> mutex_lock(>mutex);
> - sock = vq->private_data;
> + sock = vhost_vq_get_backend_opaque(vq);
> vhost_net_disable_vq(n, vq);
> - vq->private_data = NULL;
> + vhost_vq_set_backend_opaque(vq, NULL);
> vhost_net_buf_unproduce(nvq);
> nvq->rx_ring = NULL;
> mutex_unlock(>mutex);
> @@ -1528,7 +1530,7 @@ static long vhost_net_set_backend(struct vhost_net *n,
> unsigned index, int fd)
> }
>
> /* start polling new socket */
> - oldsock = vq->private_data;
> + oldsock = vhost_vq_get_backend_opaque(vq);
> if (sock != oldsock) {
> ubufs = vhost_net_ubuf_alloc(vq,
>sock && vhost_sock_zcopy(sock));
> @@ -1538,7 +1540,7 @@ static long vhost_net_set_backend(struct vhost_net *n,
> unsigned index, int fd)
> }
>
> vhost_net_disable_vq(n, vq);
> - vq->private_data = sock;
> + vhost_vq_set_backend_opaque(vq, sock);
> vhost_net_buf_unproduce(nvq);
> r = vhost_vq_init_access(vq);
> if (r)
> @@ -1575,7 +1577,7 @@ static long vhost_net_set_backend(struct vhost_net *n,
> unsigned index, int fd)
> return 0;
>
> err_used:
> - vq->private_data = oldsock;
> + vhost_vq_set_backend_opaque(vq, oldsock);
> vhost_net_enable_vq(n, vq);
> if (ubufs)
> vhost_net_ubuf_put_wait_and_free(ubufs);
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index a123fd70847e..0808188f7e8f 100644
> --- a/drivers/vhost/vhost.h
> +++
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
> On Mar 31, 2020, at 7:09 AM, David Hildenbrand wrote:
>
> On 31.03.20 16:07, Michael S. Tsirkin wrote:
>> On Tue, Mar 31, 2020 at 04:03:18PM +0200, David Hildenbrand wrote:
>>> On 31.03.20 15:37, Michael S. Tsirkin wrote:
On Tue, Mar 31, 2020 at 03:32:05PM +0200, David Hildenbrand wrote:
> On 31.03.20 15:24, Michael S. Tsirkin wrote:
>> On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
>>> On 26.03.20 10:49, Michael S. Tsirkin wrote:
On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
>> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
>>
>> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
On 12.03.20 09:47, Michael S. Tsirkin wrote:
On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
> 2. You are essentially stealing THPs in the guest. So the fastest
> mapping (THP in guest and host) is gone. The guest won't be able
> to make
> use of THP where it previously was able to. I can imagine this
> implies a
> performance degradation for some workloads. This needs a proper
> performance evaluation.
I think the problem is more with the alloc_pages API.
That gives you exactly the given order, and if there's
a larger chunk available, it will split it up.
But for balloon - I suspect lots of other users,
we do not want to stress the system but if a large
chunk is available anyway, then we could handle
that more optimally by getting it all in one go.
So if we want to address this, IMHO this calls for a new API.
Along the lines of
struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
unsigned int max_order, unsigned int *order)
the idea would then be to return at a number of pages in the given
range.
What do you think? Want to try implementing that?
>>>
>>> You can just start with the highest order and decrement the order
>>> until
>>> your allocation succeeds using alloc_pages(), which would be enough
>>> for
>>> a first version. At least I don't see the immediate need for a new
>>> kernel API.
>>
>> OK I remember now. The problem is with reclaim. Unless reclaim is
>> completely disabled, any of these calls can sleep. After it wakes up,
>> we would like to get the larger order that has become available
>> meanwhile.
>
> Yes, but that‘s a pure optimization IMHO.
> So I think we should do a trivial implementation first and then see
> what we gain from a new allocator API. Then we might also be able to
> justify it using real numbers.
Well how do you propose implement the necessary semantics?
I think we are both agreed that alloc_page_range is more or
less what's necessary anyway - so how would you approximate it
on top of existing APIs?
>>> diff --git a/include/linux/balloon_compaction.h
>>> b/include/linux/balloon_compaction.h
.
>>> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
>>> index 26de020aae7b..067810b32813 100644
>>> --- a/mm/balloon_compaction.c
>>> +++ b/mm/balloon_compaction.c
>>> @@ -112,23 +112,35 @@ size_t balloon_page_list_dequeue(struct
>>> balloon_dev_info *b_dev_info,
>>> EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
>>>
>>> /*
>>> - * balloon_page_alloc - allocates a new page for insertion into the
>>> balloon
>>> - * page list.
>>> + * balloon_pages_alloc - allocates a new page (of at most the given
>>> order)
>>> + * for insertion into the balloon page list.
>>> *
>>> * Driver must call this function to properly allocate a new balloon
>>> page.
>>> * Driver must call balloon_page_enqueue before definitively removing
>>> the page
>>> * from the guest system.
>>> *
>>> + * Will fall back to smaller orders if allocation fails. The order of
>>> the
>>> + * allocated page is stored in page->private.
>>> + *
>>> * Return: struct page for the allocated page or NULL on allocation
>>> failure.
>>> */
>>> -struct page *balloon_page_alloc(void)
>>> +struct page *balloon_pages_alloc(int order)
>>> {
>>> - struct page *page = alloc_page(balloon_mapping_gfp_mask() |
>>> - __GFP_NOMEMALLOC | __GFP_NORETRY
>>> |
>>> - __GFP_NOWARN);
>>> - return page;
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
> On Mar 31, 2020, at 6:32 AM, David Hildenbrand wrote:
>
> On 31.03.20 15:24, Michael S. Tsirkin wrote:
>> On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
>>> On 26.03.20 10:49, Michael S. Tsirkin wrote:
On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
>> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
>>
>> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
On 12.03.20 09:47, Michael S. Tsirkin wrote:
On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
> 2. You are essentially stealing THPs in the guest. So the fastest
> mapping (THP in guest and host) is gone. The guest won't be able to
> make
> use of THP where it previously was able to. I can imagine this
> implies a
> performance degradation for some workloads. This needs a proper
> performance evaluation.
I think the problem is more with the alloc_pages API.
That gives you exactly the given order, and if there's
a larger chunk available, it will split it up.
But for balloon - I suspect lots of other users,
we do not want to stress the system but if a large
chunk is available anyway, then we could handle
that more optimally by getting it all in one go.
So if we want to address this, IMHO this calls for a new API.
Along the lines of
struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
unsigned int max_order, unsigned int *order)
the idea would then be to return at a number of pages in the given
range.
What do you think? Want to try implementing that?
>>>
>>> You can just start with the highest order and decrement the order until
>>> your allocation succeeds using alloc_pages(), which would be enough for
>>> a first version. At least I don't see the immediate need for a new
>>> kernel API.
>>
>> OK I remember now. The problem is with reclaim. Unless reclaim is
>> completely disabled, any of these calls can sleep. After it wakes up,
>> we would like to get the larger order that has become available
>> meanwhile.
>
> Yes, but that‘s a pure optimization IMHO.
> So I think we should do a trivial implementation first and then see what
> we gain from a new allocator API. Then we might also be able to justify
> it using real numbers.
Well how do you propose implement the necessary semantics?
I think we are both agreed that alloc_page_range is more or
less what's necessary anyway - so how would you approximate it
on top of existing APIs?
>>>
>>> Looking at drivers/misc/vmw_balloon.c:vmballoon_inflate(), it first
>>> tries to allocate huge pages using
>>>
>>> alloc_pages(__GFP_HIGHMEM|__GFP_NOWARN| __GFP_NOMEMALLOC,
>>>VMW_BALLOON_2M_ORDER)
>>>
>>> And then falls back to 4k allocations (balloon_page_alloc()) in case
>>> allocation fails.
>>>
>>> I'm roughly thinking of something like the following, but with an
>>> optimized reporting interface/bigger pfn array so we can report >
>>> 1MB at a time. Also, it might make sense to remember the order that
>>> succeeded across some fill_balloon() calls.
>>>
>>> Don't even expect it to compile ...
>>>
>>>
>>>
From 4305f989672ccca4be9293e6d4167e929f3e299b Mon Sep 17 00:00:00 2001
>>> From: David Hildenbrand
>>> Date: Tue, 31 Mar 2020 12:28:07 +0200
>>> Subject: [PATCH RFC] tmp
>>>
>>> Signed-off-by: David Hildenbrand
>>> ---
>>> drivers/virtio/virtio_balloon.c| 38 ++
>>> include/linux/balloon_compaction.h | 7 -
>>> mm/balloon_compaction.c| 43 +++---
>>> 3 files changed, 67 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/virtio/virtio_balloon.c
>>> b/drivers/virtio/virtio_balloon.c
>>> index 8511d258dbb4..0660b1b988f0 100644
>>> --- a/drivers/virtio/virtio_balloon.c
>>> +++ b/drivers/virtio/virtio_balloon.c
>>> @@ -187,7 +187,7 @@ int virtballoon_free_page_report(struct
>>> page_reporting_dev_info *pr_dev_info,
>>> }
>>>
>>> static void set_page_pfns(struct virtio_balloon *vb,
>>> - __virtio32 pfns[], struct page *page)
>>> + __virtio32 pfns[], struct page *page, int order)
>>> {
>>> unsigned int i;
>>>
>>> @@ -197,7 +197,7 @@ static void set_page_pfns(struct virtio_balloon *vb,
>>> * Set balloon pfns pointing at this page.
>>> * Note that the first pfn points at start of the page.
>>> */
>>> - for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
>>> + for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE * (1 << order); i++)
>>> pfns[i] = cpu_to_virtio32(vb->vdev,
>>> page_to_balloon_pfn(page) + i);
>>> }
>>> @@
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On Tue, Mar 31, 2020 at 04:34:48PM +0200, David Hildenbrand wrote: > On 31.03.20 16:29, David Hildenbrand wrote: > > On 31.03.20 16:18, Michael S. Tsirkin wrote: > >> On Tue, Mar 31, 2020 at 04:09:59PM +0200, David Hildenbrand wrote: > >> > >> ... > >> > >> So if we want to address this, IMHO this calls for a new API. > >> Along the lines of > >> > >>struct page *alloc_page_range(gfp_t gfp, unsigned int > >> min_order, > >>unsigned int max_order, unsigned int *order) > >> > >> the idea would then be to return at a number of pages in the > >> given > >> range. > >> > >> What do you think? Want to try implementing that? > >> > >> .. > >> > >>> I expect the whole "steal huge pages from your guest" to be problematic, > >>> as I already mentioned to Alex. This needs a performance evaluation. > >>> > >>> This all smells like a lot of workload dependent fine-tuning. :) > >> > >> > >> So that's why I proposed the API above. > >> > >> The idea is that *if we are allocating a huge page anyway*, > >> rather than break it up let's send it whole to the device. > >> If we have smaller pages, return smaller pages. > >> > > > > Sorry, I still fail to see why you cannot do that with my version of > > balloon_pages_alloc(). But maybe I haven't understood the magic you > > expect to happen in alloc_page_range() :) > > > > It's just going via a different inflate queue once we have that page, as > > I stated in front of my draft patch "but with an > > optimized reporting interface". > > > >> That seems like it would always be an improvement, whatever the > >> workload. > >> > > > > Don't think so. Assume there are plenty of 4k pages lying around. It > > might actually be *bad* for guest performance if you take a huge page > > instead of all the leftover 4k pages that cannot be merged. Only at the > > point where you would want to break a bigger page up and report it in > > pieces, where it would definitely make no difference. > > I just understood what you mean :) and now it makes sense - it avoids > exactly that. Basically > > 1. Try to allocate order-0. No split necessary? return the page > 2. Try to allocate order-1. No split necessary? return the page > ... > > up to MAX_ORDER - 1. > > Yeah, I guess this will need a new kernel API. Exactly what I meant. And whever we fail and block for reclaim, we restart this. > > -- > Thanks, > > David / dhildenb ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH Resend] drm/qxl: Use correct notify port address when creating cursor ring
On Tue, Mar 31, 2020 at 02:18:08PM +0800, Huacai Chen wrote: > The command ring and cursor ring use different notify port addresses > definition: QXL_IO_NOTIFY_CMD and QXL_IO_NOTIFY_CURSOR. However, in > qxl_device_init() we use QXL_IO_NOTIFY_CMD to create both command ring > and cursor ring. This doesn't cause any problems now, because QEMU's > behaviors on QXL_IO_NOTIFY_CMD and QXL_IO_NOTIFY_CURSOR are the same. > However, QEMU's behavior may be change in future, so let's fix it. > > P.S.: In the X.org QXL driver, the notify port address of cursor ring > is correct. > > Cc: > Signed-off-by: Huacai Chen Pushed to drm-misc-next. thanks, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On 31.03.20 16:29, David Hildenbrand wrote: > On 31.03.20 16:18, Michael S. Tsirkin wrote: >> On Tue, Mar 31, 2020 at 04:09:59PM +0200, David Hildenbrand wrote: >> >> ... >> >> So if we want to address this, IMHO this calls for a new API. >> Along the lines of >> >>struct page *alloc_page_range(gfp_t gfp, unsigned int >> min_order, >>unsigned int max_order, unsigned int *order) >> >> the idea would then be to return at a number of pages in the >> given >> range. >> >> What do you think? Want to try implementing that? >> >> .. >> >>> I expect the whole "steal huge pages from your guest" to be problematic, >>> as I already mentioned to Alex. This needs a performance evaluation. >>> >>> This all smells like a lot of workload dependent fine-tuning. :) >> >> >> So that's why I proposed the API above. >> >> The idea is that *if we are allocating a huge page anyway*, >> rather than break it up let's send it whole to the device. >> If we have smaller pages, return smaller pages. >> > > Sorry, I still fail to see why you cannot do that with my version of > balloon_pages_alloc(). But maybe I haven't understood the magic you > expect to happen in alloc_page_range() :) > > It's just going via a different inflate queue once we have that page, as > I stated in front of my draft patch "but with an > optimized reporting interface". > >> That seems like it would always be an improvement, whatever the >> workload. >> > > Don't think so. Assume there are plenty of 4k pages lying around. It > might actually be *bad* for guest performance if you take a huge page > instead of all the leftover 4k pages that cannot be merged. Only at the > point where you would want to break a bigger page up and report it in > pieces, where it would definitely make no difference. I just understood what you mean :) and now it makes sense - it avoids exactly that. Basically 1. Try to allocate order-0. No split necessary? return the page 2. Try to allocate order-1. No split necessary? return the page ... up to MAX_ORDER - 1. Yeah, I guess this will need a new kernel API. -- Thanks, David / dhildenb ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On 31.03.20 16:18, Michael S. Tsirkin wrote: > On Tue, Mar 31, 2020 at 04:09:59PM +0200, David Hildenbrand wrote: > > ... > > So if we want to address this, IMHO this calls for a new API. > Along the lines of > >struct page *alloc_page_range(gfp_t gfp, unsigned int > min_order, >unsigned int max_order, unsigned int *order) > > the idea would then be to return at a number of pages in the given > range. > > What do you think? Want to try implementing that? > > .. > >> I expect the whole "steal huge pages from your guest" to be problematic, >> as I already mentioned to Alex. This needs a performance evaluation. >> >> This all smells like a lot of workload dependent fine-tuning. :) > > > So that's why I proposed the API above. > > The idea is that *if we are allocating a huge page anyway*, > rather than break it up let's send it whole to the device. > If we have smaller pages, return smaller pages. > Sorry, I still fail to see why you cannot do that with my version of balloon_pages_alloc(). But maybe I haven't understood the magic you expect to happen in alloc_page_range() :) It's just going via a different inflate queue once we have that page, as I stated in front of my draft patch "but with an optimized reporting interface". > That seems like it would always be an improvement, whatever the > workload. > Don't think so. Assume there are plenty of 4k pages lying around. It might actually be *bad* for guest performance if you take a huge page instead of all the leftover 4k pages that cannot be merged. Only at the point where you would want to break a bigger page up and report it in pieces, where it would definitely make no difference. I guess Hui Zhu now has something to look into/work on :) -- Thanks, David / dhildenb ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On Tue, Mar 31, 2020 at 04:09:59PM +0200, David Hildenbrand wrote: ... > >>> So if we want to address this, IMHO this calls for a new API. > >>> Along the lines of > >>> > >>>struct page *alloc_page_range(gfp_t gfp, unsigned int > >>> min_order, > >>>unsigned int max_order, unsigned int *order) > >>> > >>> the idea would then be to return at a number of pages in the given > >>> range. > >>> > >>> What do you think? Want to try implementing that? .. > I expect the whole "steal huge pages from your guest" to be problematic, > as I already mentioned to Alex. This needs a performance evaluation. > > This all smells like a lot of workload dependent fine-tuning. :) So that's why I proposed the API above. The idea is that *if we are allocating a huge page anyway*, rather than break it up let's send it whole to the device. If we have smaller pages, return smaller pages. That seems like it would always be an improvement, whatever the workload. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On 31.03.20 16:07, Michael S. Tsirkin wrote:
> On Tue, Mar 31, 2020 at 04:03:18PM +0200, David Hildenbrand wrote:
>> On 31.03.20 15:37, Michael S. Tsirkin wrote:
>>> On Tue, Mar 31, 2020 at 03:32:05PM +0200, David Hildenbrand wrote:
On 31.03.20 15:24, Michael S. Tsirkin wrote:
> On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
>> On 26.03.20 10:49, Michael S. Tsirkin wrote:
>>> On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
>
> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
>>> On 12.03.20 09:47, Michael S. Tsirkin wrote:
>>> On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
2. You are essentially stealing THPs in the guest. So the fastest
mapping (THP in guest and host) is gone. The guest won't be able
to make
use of THP where it previously was able to. I can imagine this
implies a
performance degradation for some workloads. This needs a proper
performance evaluation.
>>>
>>> I think the problem is more with the alloc_pages API.
>>> That gives you exactly the given order, and if there's
>>> a larger chunk available, it will split it up.
>>>
>>> But for balloon - I suspect lots of other users,
>>> we do not want to stress the system but if a large
>>> chunk is available anyway, then we could handle
>>> that more optimally by getting it all in one go.
>>>
>>>
>>> So if we want to address this, IMHO this calls for a new API.
>>> Along the lines of
>>>
>>>struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
>>>unsigned int max_order, unsigned int *order)
>>>
>>> the idea would then be to return at a number of pages in the given
>>> range.
>>>
>>> What do you think? Want to try implementing that?
>>
>> You can just start with the highest order and decrement the order
>> until
>> your allocation succeeds using alloc_pages(), which would be enough
>> for
>> a first version. At least I don't see the immediate need for a new
>> kernel API.
>
> OK I remember now. The problem is with reclaim. Unless reclaim is
> completely disabled, any of these calls can sleep. After it wakes up,
> we would like to get the larger order that has become available
> meanwhile.
>
Yes, but that‘s a pure optimization IMHO.
So I think we should do a trivial implementation first and then see
what we gain from a new allocator API. Then we might also be able to
justify it using real numbers.
>>>
>>> Well how do you propose implement the necessary semantics?
>>> I think we are both agreed that alloc_page_range is more or
>>> less what's necessary anyway - so how would you approximate it
>>> on top of existing APIs?
>> diff --git a/include/linux/balloon_compaction.h
>> b/include/linux/balloon_compaction.h
>>>
>>> .
>>>
>>>
>> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
>> index 26de020aae7b..067810b32813 100644
>> --- a/mm/balloon_compaction.c
>> +++ b/mm/balloon_compaction.c
>> @@ -112,23 +112,35 @@ size_t balloon_page_list_dequeue(struct
>> balloon_dev_info *b_dev_info,
>> EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
>>
>> /*
>> - * balloon_page_alloc - allocates a new page for insertion into the
>> balloon
>> - * page list.
>> + * balloon_pages_alloc - allocates a new page (of at most the given
>> order)
>> + * for insertion into the balloon page list.
>> *
>> * Driver must call this function to properly allocate a new balloon
>> page.
>> * Driver must call balloon_page_enqueue before definitively removing
>> the page
>> * from the guest system.
>> *
>> + * Will fall back to smaller orders if allocation fails. The order of
>> the
>> + * allocated page is stored in page->private.
>> + *
>> * Return: struct page for the allocated page or NULL on allocation
>> failure.
>> */
>> -struct page *balloon_page_alloc(void)
>> +struct page *balloon_pages_alloc(int order)
>> {
>> -struct page *page = alloc_page(balloon_mapping_gfp_mask() |
>> - __GFP_NOMEMALLOC | __GFP_NORETRY
>> |
>> - __GFP_NOWARN);
>> -return page;
>> +struct page *page;
>> +
>> +while (order >= 0) {
>> +page =
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On Tue, Mar 31, 2020 at 04:03:18PM +0200, David Hildenbrand wrote:
> On 31.03.20 15:37, Michael S. Tsirkin wrote:
> > On Tue, Mar 31, 2020 at 03:32:05PM +0200, David Hildenbrand wrote:
> >> On 31.03.20 15:24, Michael S. Tsirkin wrote:
> >>> On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
> On 26.03.20 10:49, Michael S. Tsirkin wrote:
> > On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
> >>
> >>
> >>> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
> >>>
> >>> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
> > On 12.03.20 09:47, Michael S. Tsirkin wrote:
> > On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
> >> 2. You are essentially stealing THPs in the guest. So the fastest
> >> mapping (THP in guest and host) is gone. The guest won't be able
> >> to make
> >> use of THP where it previously was able to. I can imagine this
> >> implies a
> >> performance degradation for some workloads. This needs a proper
> >> performance evaluation.
> >
> > I think the problem is more with the alloc_pages API.
> > That gives you exactly the given order, and if there's
> > a larger chunk available, it will split it up.
> >
> > But for balloon - I suspect lots of other users,
> > we do not want to stress the system but if a large
> > chunk is available anyway, then we could handle
> > that more optimally by getting it all in one go.
> >
> >
> > So if we want to address this, IMHO this calls for a new API.
> > Along the lines of
> >
> >struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
> >unsigned int max_order, unsigned int *order)
> >
> > the idea would then be to return at a number of pages in the given
> > range.
> >
> > What do you think? Want to try implementing that?
>
> You can just start with the highest order and decrement the order
> until
> your allocation succeeds using alloc_pages(), which would be enough
> for
> a first version. At least I don't see the immediate need for a new
> kernel API.
> >>>
> >>> OK I remember now. The problem is with reclaim. Unless reclaim is
> >>> completely disabled, any of these calls can sleep. After it wakes up,
> >>> we would like to get the larger order that has become available
> >>> meanwhile.
> >>>
> >>
> >> Yes, but that‘s a pure optimization IMHO.
> >> So I think we should do a trivial implementation first and then see
> >> what we gain from a new allocator API. Then we might also be able to
> >> justify it using real numbers.
> >>
> >
> > Well how do you propose implement the necessary semantics?
> > I think we are both agreed that alloc_page_range is more or
> > less what's necessary anyway - so how would you approximate it
> > on top of existing APIs?
> diff --git a/include/linux/balloon_compaction.h
> b/include/linux/balloon_compaction.h
> >
> > .
> >
> >
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index 26de020aae7b..067810b32813 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -112,23 +112,35 @@ size_t balloon_page_list_dequeue(struct
> balloon_dev_info *b_dev_info,
> EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
>
> /*
> - * balloon_page_alloc - allocates a new page for insertion into the
> balloon
> - * page list.
> + * balloon_pages_alloc - allocates a new page (of at most the given
> order)
> + * for insertion into the balloon page list.
> *
> * Driver must call this function to properly allocate a new balloon
> page.
> * Driver must call balloon_page_enqueue before definitively removing
> the page
> * from the guest system.
> *
> + * Will fall back to smaller orders if allocation fails. The order of
> the
> + * allocated page is stored in page->private.
> + *
> * Return: struct page for the allocated page or NULL on allocation
> failure.
> */
> -struct page *balloon_page_alloc(void)
> +struct page *balloon_pages_alloc(int order)
> {
> -struct page *page = alloc_page(balloon_mapping_gfp_mask() |
> - __GFP_NOMEMALLOC | __GFP_NORETRY
> |
> - __GFP_NOWARN);
> -return page;
> +struct page *page;
> +
> +while (order >= 0) {
> +page = alloc_pages(balloon_mapping_gfp_mask() |
> +
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On 31.03.20 15:37, Michael S. Tsirkin wrote:
> On Tue, Mar 31, 2020 at 03:32:05PM +0200, David Hildenbrand wrote:
>> On 31.03.20 15:24, Michael S. Tsirkin wrote:
>>> On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
On 26.03.20 10:49, Michael S. Tsirkin wrote:
> On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
>>
>>
>>> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
>>>
>>> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
> On 12.03.20 09:47, Michael S. Tsirkin wrote:
> On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
>> 2. You are essentially stealing THPs in the guest. So the fastest
>> mapping (THP in guest and host) is gone. The guest won't be able to
>> make
>> use of THP where it previously was able to. I can imagine this
>> implies a
>> performance degradation for some workloads. This needs a proper
>> performance evaluation.
>
> I think the problem is more with the alloc_pages API.
> That gives you exactly the given order, and if there's
> a larger chunk available, it will split it up.
>
> But for balloon - I suspect lots of other users,
> we do not want to stress the system but if a large
> chunk is available anyway, then we could handle
> that more optimally by getting it all in one go.
>
>
> So if we want to address this, IMHO this calls for a new API.
> Along the lines of
>
>struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
>unsigned int max_order, unsigned int *order)
>
> the idea would then be to return at a number of pages in the given
> range.
>
> What do you think? Want to try implementing that?
You can just start with the highest order and decrement the order until
your allocation succeeds using alloc_pages(), which would be enough for
a first version. At least I don't see the immediate need for a new
kernel API.
>>>
>>> OK I remember now. The problem is with reclaim. Unless reclaim is
>>> completely disabled, any of these calls can sleep. After it wakes up,
>>> we would like to get the larger order that has become available
>>> meanwhile.
>>>
>>
>> Yes, but that‘s a pure optimization IMHO.
>> So I think we should do a trivial implementation first and then see what
>> we gain from a new allocator API. Then we might also be able to justify
>> it using real numbers.
>>
>
> Well how do you propose implement the necessary semantics?
> I think we are both agreed that alloc_page_range is more or
> less what's necessary anyway - so how would you approximate it
> on top of existing APIs?
diff --git a/include/linux/balloon_compaction.h
b/include/linux/balloon_compaction.h
>
> .
>
>
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index 26de020aae7b..067810b32813 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -112,23 +112,35 @@ size_t balloon_page_list_dequeue(struct
balloon_dev_info *b_dev_info,
EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
/*
- * balloon_page_alloc - allocates a new page for insertion into the
balloon
- *page list.
+ * balloon_pages_alloc - allocates a new page (of at most the given order)
+ * for insertion into the balloon page list.
*
* Driver must call this function to properly allocate a new balloon page.
* Driver must call balloon_page_enqueue before definitively removing the
page
* from the guest system.
*
+ * Will fall back to smaller orders if allocation fails. The order of the
+ * allocated page is stored in page->private.
+ *
* Return: struct page for the allocated page or NULL on allocation
failure.
*/
-struct page *balloon_page_alloc(void)
+struct page *balloon_pages_alloc(int order)
{
- struct page *page = alloc_page(balloon_mapping_gfp_mask() |
- __GFP_NOMEMALLOC | __GFP_NORETRY |
- __GFP_NOWARN);
- return page;
+ struct page *page;
+
+ while (order >= 0) {
+ page = alloc_pages(balloon_mapping_gfp_mask() |
+ __GFP_NOMEMALLOC | __GFP_NORETRY |
+ __GFP_NOWARN, order);
+ if (page) {
+ set_page_private(page, order);
+ return page;
+ }
+ order--;
+ }
+ return NULL;
}
-EXPORT_SYMBOL_GPL(balloon_page_alloc);
Re: [PATCH] vhost: vdpa: remove unnecessary null check
On Mon, Mar 30, 2020 at 06:50:40PM -0500, Gustavo A. R. Silva wrote:
> container_of is never null, so this null check is
> unnecessary.
>
> Addresses-Coverity-ID: 1492006 ("Logically dead code")
> Fixes: 20453a45fb06 ("vhost: introduce vDPA-based backend")
> Signed-off-by: Gustavo A. R. Silva
Yes weird. Was the point to test i_cdev? Tiwei?
> ---
> drivers/vhost/vdpa.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 421f02a8530a..3d2cb811757a 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -678,8 +678,6 @@ static int vhost_vdpa_open(struct inode *inode, struct
> file *filep)
> int nvqs, i, r, opened;
>
> v = container_of(inode->i_cdev, struct vhost_vdpa, cdev);
> - if (!v)
> - return -ENODEV;
>
> opened = atomic_cmpxchg(>opened, 0, 1);
> if (opened)
> --
> 2.26.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On Tue, Mar 31, 2020 at 03:32:05PM +0200, David Hildenbrand wrote:
> On 31.03.20 15:24, Michael S. Tsirkin wrote:
> > On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
> >> On 26.03.20 10:49, Michael S. Tsirkin wrote:
> >>> On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
>
>
> > Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
> >
> > On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
> >>> On 12.03.20 09:47, Michael S. Tsirkin wrote:
> >>> On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
> 2. You are essentially stealing THPs in the guest. So the fastest
> mapping (THP in guest and host) is gone. The guest won't be able to
> make
> use of THP where it previously was able to. I can imagine this
> implies a
> performance degradation for some workloads. This needs a proper
> performance evaluation.
> >>>
> >>> I think the problem is more with the alloc_pages API.
> >>> That gives you exactly the given order, and if there's
> >>> a larger chunk available, it will split it up.
> >>>
> >>> But for balloon - I suspect lots of other users,
> >>> we do not want to stress the system but if a large
> >>> chunk is available anyway, then we could handle
> >>> that more optimally by getting it all in one go.
> >>>
> >>>
> >>> So if we want to address this, IMHO this calls for a new API.
> >>> Along the lines of
> >>>
> >>>struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
> >>>unsigned int max_order, unsigned int *order)
> >>>
> >>> the idea would then be to return at a number of pages in the given
> >>> range.
> >>>
> >>> What do you think? Want to try implementing that?
> >>
> >> You can just start with the highest order and decrement the order until
> >> your allocation succeeds using alloc_pages(), which would be enough for
> >> a first version. At least I don't see the immediate need for a new
> >> kernel API.
> >
> > OK I remember now. The problem is with reclaim. Unless reclaim is
> > completely disabled, any of these calls can sleep. After it wakes up,
> > we would like to get the larger order that has become available
> > meanwhile.
> >
>
> Yes, but that‘s a pure optimization IMHO.
> So I think we should do a trivial implementation first and then see what
> we gain from a new allocator API. Then we might also be able to justify
> it using real numbers.
>
> >>>
> >>> Well how do you propose implement the necessary semantics?
> >>> I think we are both agreed that alloc_page_range is more or
> >>> less what's necessary anyway - so how would you approximate it
> >>> on top of existing APIs?
> >> diff --git a/include/linux/balloon_compaction.h
> >> b/include/linux/balloon_compaction.h
.
> >> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> >> index 26de020aae7b..067810b32813 100644
> >> --- a/mm/balloon_compaction.c
> >> +++ b/mm/balloon_compaction.c
> >> @@ -112,23 +112,35 @@ size_t balloon_page_list_dequeue(struct
> >> balloon_dev_info *b_dev_info,
> >> EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
> >>
> >> /*
> >> - * balloon_page_alloc - allocates a new page for insertion into the
> >> balloon
> >> - *page list.
> >> + * balloon_pages_alloc - allocates a new page (of at most the given order)
> >> + * for insertion into the balloon page list.
> >> *
> >> * Driver must call this function to properly allocate a new balloon page.
> >> * Driver must call balloon_page_enqueue before definitively removing the
> >> page
> >> * from the guest system.
> >> *
> >> + * Will fall back to smaller orders if allocation fails. The order of the
> >> + * allocated page is stored in page->private.
> >> + *
> >> * Return: struct page for the allocated page or NULL on allocation
> >> failure.
> >> */
> >> -struct page *balloon_page_alloc(void)
> >> +struct page *balloon_pages_alloc(int order)
> >> {
> >> - struct page *page = alloc_page(balloon_mapping_gfp_mask() |
> >> - __GFP_NOMEMALLOC | __GFP_NORETRY |
> >> - __GFP_NOWARN);
> >> - return page;
> >> + struct page *page;
> >> +
> >> + while (order >= 0) {
> >> + page = alloc_pages(balloon_mapping_gfp_mask() |
> >> + __GFP_NOMEMALLOC | __GFP_NORETRY |
> >> + __GFP_NOWARN, order);
> >> + if (page) {
> >> + set_page_private(page, order);
> >> + return page;
> >> + }
> >> + order--;
> >> + }
> >> + return NULL;
> >> }
> >> -EXPORT_SYMBOL_GPL(balloon_page_alloc);
> >> +EXPORT_SYMBOL_GPL(balloon_pages_alloc);
> >>
> >> /*
> >> *
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On 31.03.20 15:24, Michael S. Tsirkin wrote:
> On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
>> On 26.03.20 10:49, Michael S. Tsirkin wrote:
>>> On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
>
> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
>>> On 12.03.20 09:47, Michael S. Tsirkin wrote:
>>> On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
2. You are essentially stealing THPs in the guest. So the fastest
mapping (THP in guest and host) is gone. The guest won't be able to
make
use of THP where it previously was able to. I can imagine this implies
a
performance degradation for some workloads. This needs a proper
performance evaluation.
>>>
>>> I think the problem is more with the alloc_pages API.
>>> That gives you exactly the given order, and if there's
>>> a larger chunk available, it will split it up.
>>>
>>> But for balloon - I suspect lots of other users,
>>> we do not want to stress the system but if a large
>>> chunk is available anyway, then we could handle
>>> that more optimally by getting it all in one go.
>>>
>>>
>>> So if we want to address this, IMHO this calls for a new API.
>>> Along the lines of
>>>
>>>struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
>>>unsigned int max_order, unsigned int *order)
>>>
>>> the idea would then be to return at a number of pages in the given
>>> range.
>>>
>>> What do you think? Want to try implementing that?
>>
>> You can just start with the highest order and decrement the order until
>> your allocation succeeds using alloc_pages(), which would be enough for
>> a first version. At least I don't see the immediate need for a new
>> kernel API.
>
> OK I remember now. The problem is with reclaim. Unless reclaim is
> completely disabled, any of these calls can sleep. After it wakes up,
> we would like to get the larger order that has become available
> meanwhile.
>
Yes, but that‘s a pure optimization IMHO.
So I think we should do a trivial implementation first and then see what
we gain from a new allocator API. Then we might also be able to justify it
using real numbers.
>>>
>>> Well how do you propose implement the necessary semantics?
>>> I think we are both agreed that alloc_page_range is more or
>>> less what's necessary anyway - so how would you approximate it
>>> on top of existing APIs?
>>
>> Looking at drivers/misc/vmw_balloon.c:vmballoon_inflate(), it first
>> tries to allocate huge pages using
>>
>> alloc_pages(__GFP_HIGHMEM|__GFP_NOWARN| __GFP_NOMEMALLOC,
>> VMW_BALLOON_2M_ORDER)
>>
>> And then falls back to 4k allocations (balloon_page_alloc()) in case
>> allocation fails.
>>
>> I'm roughly thinking of something like the following, but with an
>> optimized reporting interface/bigger pfn array so we can report >
>> 1MB at a time. Also, it might make sense to remember the order that
>> succeeded across some fill_balloon() calls.
>>
>> Don't even expect it to compile ...
>>
>>
>>
>> >From 4305f989672ccca4be9293e6d4167e929f3e299b Mon Sep 17 00:00:00 2001
>> From: David Hildenbrand
>> Date: Tue, 31 Mar 2020 12:28:07 +0200
>> Subject: [PATCH RFC] tmp
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> drivers/virtio/virtio_balloon.c| 38 ++
>> include/linux/balloon_compaction.h | 7 -
>> mm/balloon_compaction.c| 43 +++---
>> 3 files changed, 67 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c
>> b/drivers/virtio/virtio_balloon.c
>> index 8511d258dbb4..0660b1b988f0 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -187,7 +187,7 @@ int virtballoon_free_page_report(struct
>> page_reporting_dev_info *pr_dev_info,
>> }
>>
>> static void set_page_pfns(struct virtio_balloon *vb,
>> - __virtio32 pfns[], struct page *page)
>> + __virtio32 pfns[], struct page *page, int order)
>> {
>> unsigned int i;
>>
>> @@ -197,7 +197,7 @@ static void set_page_pfns(struct virtio_balloon *vb,
>> * Set balloon pfns pointing at this page.
>> * Note that the first pfn points at start of the page.
>> */
>> -for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
>> +for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE * (1 << order); i++)
>> pfns[i] = cpu_to_virtio32(vb->vdev,
>>page_to_balloon_pfn(page) + i);
>> }
>> @@ -205,6 +205,7 @@ static void set_page_pfns(struct virtio_balloon *vb,
>> static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>> {
>>
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On Tue, Mar 31, 2020 at 12:35:24PM +0200, David Hildenbrand wrote:
> On 26.03.20 10:49, Michael S. Tsirkin wrote:
> > On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
> >>
> >>
> >>> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
> >>>
> >>> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
> > On 12.03.20 09:47, Michael S. Tsirkin wrote:
> > On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
> >> 2. You are essentially stealing THPs in the guest. So the fastest
> >> mapping (THP in guest and host) is gone. The guest won't be able to
> >> make
> >> use of THP where it previously was able to. I can imagine this implies
> >> a
> >> performance degradation for some workloads. This needs a proper
> >> performance evaluation.
> >
> > I think the problem is more with the alloc_pages API.
> > That gives you exactly the given order, and if there's
> > a larger chunk available, it will split it up.
> >
> > But for balloon - I suspect lots of other users,
> > we do not want to stress the system but if a large
> > chunk is available anyway, then we could handle
> > that more optimally by getting it all in one go.
> >
> >
> > So if we want to address this, IMHO this calls for a new API.
> > Along the lines of
> >
> >struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
> >unsigned int max_order, unsigned int *order)
> >
> > the idea would then be to return at a number of pages in the given
> > range.
> >
> > What do you think? Want to try implementing that?
>
> You can just start with the highest order and decrement the order until
> your allocation succeeds using alloc_pages(), which would be enough for
> a first version. At least I don't see the immediate need for a new
> kernel API.
> >>>
> >>> OK I remember now. The problem is with reclaim. Unless reclaim is
> >>> completely disabled, any of these calls can sleep. After it wakes up,
> >>> we would like to get the larger order that has become available
> >>> meanwhile.
> >>>
> >>
> >> Yes, but that‘s a pure optimization IMHO.
> >> So I think we should do a trivial implementation first and then see what
> >> we gain from a new allocator API. Then we might also be able to justify it
> >> using real numbers.
> >>
> >
> > Well how do you propose implement the necessary semantics?
> > I think we are both agreed that alloc_page_range is more or
> > less what's necessary anyway - so how would you approximate it
> > on top of existing APIs?
>
> Looking at drivers/misc/vmw_balloon.c:vmballoon_inflate(), it first
> tries to allocate huge pages using
>
> alloc_pages(__GFP_HIGHMEM|__GFP_NOWARN| __GFP_NOMEMALLOC,
> VMW_BALLOON_2M_ORDER)
>
> And then falls back to 4k allocations (balloon_page_alloc()) in case
> allocation fails.
>
> I'm roughly thinking of something like the following, but with an
> optimized reporting interface/bigger pfn array so we can report >
> 1MB at a time. Also, it might make sense to remember the order that
> succeeded across some fill_balloon() calls.
>
> Don't even expect it to compile ...
>
>
>
> >From 4305f989672ccca4be9293e6d4167e929f3e299b Mon Sep 17 00:00:00 2001
> From: David Hildenbrand
> Date: Tue, 31 Mar 2020 12:28:07 +0200
> Subject: [PATCH RFC] tmp
>
> Signed-off-by: David Hildenbrand
> ---
> drivers/virtio/virtio_balloon.c| 38 ++
> include/linux/balloon_compaction.h | 7 -
> mm/balloon_compaction.c| 43 +++---
> 3 files changed, 67 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 8511d258dbb4..0660b1b988f0 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -187,7 +187,7 @@ int virtballoon_free_page_report(struct
> page_reporting_dev_info *pr_dev_info,
> }
>
> static void set_page_pfns(struct virtio_balloon *vb,
> - __virtio32 pfns[], struct page *page)
> + __virtio32 pfns[], struct page *page, int order)
> {
> unsigned int i;
>
> @@ -197,7 +197,7 @@ static void set_page_pfns(struct virtio_balloon *vb,
>* Set balloon pfns pointing at this page.
>* Note that the first pfn points at start of the page.
>*/
> - for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
> + for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE * (1 << order); i++)
> pfns[i] = cpu_to_virtio32(vb->vdev,
> page_to_balloon_pfn(page) + i);
> }
> @@ -205,6 +205,7 @@ static void set_page_pfns(struct virtio_balloon *vb,
> static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> {
> unsigned num_allocated_pages;
> + int order = MAX_ORDER - 1;
> unsigned
Re: [PATCH 11/70] x86/boot/compressed/64: Disable red-zone usage
On Thu, Mar 19, 2020 at 10:13:08AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel
>
> The x86-64 ABI defines a red-zone on the stack:
>
> The 128-byte area beyond the location pointed to by %rsp is
> considered to be reserved and shall not be modified by signal or
> interrupt handlers. 10 Therefore, functions may use this area for
^^
That 10 is the footnote number from the pdf. :)
> temporary data that is not needed across function calls. In
> particular, leaf functions may use this area for their entire stack
> frame, rather than adjusting the stack pointer in the prologue and
> epilogue. This area is known as the red zone.
>
> This is not compatible with exception handling, so disable it for the
I could use some blurb as to what the problem is, for future reference.
> pre-decompression boot code.
>
> Signed-off-by: Joerg Roedel
> ---
> arch/x86/boot/Makefile| 2 +-
> arch/x86/boot/compressed/Makefile | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
> index 012b82fc8617..8f55e4ce1ccc 100644
> --- a/arch/x86/boot/Makefile
> +++ b/arch/x86/boot/Makefile
> @@ -65,7 +65,7 @@ clean-files += cpustr.h
>
> # ---
>
> -KBUILD_CFLAGS:= $(REALMODE_CFLAGS) -D_SETUP
> +KBUILD_CFLAGS:= $(REALMODE_CFLAGS) -D_SETUP -mno-red-zone
> KBUILD_AFLAGS:= $(KBUILD_CFLAGS) -D__ASSEMBLY__
> KBUILD_CFLAGS+= $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
> GCOV_PROFILE := n
> diff --git a/arch/x86/boot/compressed/Makefile
> b/arch/x86/boot/compressed/Makefile
> index 26050ae0b27e..e186cc0b628d 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -30,7 +30,7 @@ KBUILD_CFLAGS := -m$(BITS) -O2
> KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
> KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
> cflags-$(CONFIG_X86_32) := -march=i386
> -cflags-$(CONFIG_X86_64) := -mcmodel=small
> +cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
> KBUILD_CFLAGS += $(cflags-y)
> KBUILD_CFLAGS += -mno-mmx -mno-sse
> KBUILD_CFLAGS += $(call cc-option,-ffreestanding)
> @@ -87,7 +87,7 @@ endif
>
> vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
>
> -$(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
> +$(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar
>
> vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o \
> $(objtree)/drivers/firmware/efi/libstub/lib.a
That last chunk is not needed anymore after
c2d0b470154c ("efi/libstub/x86: Incorporate eboot.c into libstub")
AFAICT.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: linux-next: Tree for Mar 30 (vhost)
On Tue, Mar 31, 2020 at 10:27:48AM +0800, Jason Wang wrote: > > On 2020/3/31 上午1:22, Randy Dunlap wrote: > > On 3/30/20 2:43 AM, Stephen Rothwell wrote: > > > Hi all, > > > > > > The merge window has opened, so please do not add any material for the > > > next release into your linux-next included trees/branches until after > > > the merge window closes. > > > > > > Changes since 20200327: > > > > > > The vhost tree gained a conflict against the kvm-arm tree. > > > > > (note: today's linux-next is on 5.6-rc7.) > > > > on x86_64: > > > > # CONFIG_EVENTFD is not set > > > > ../drivers/vhost/vhost.c: In function 'vhost_vring_ioctl': > > ../drivers/vhost/vhost.c:1577:33: error: implicit declaration of function > > 'eventfd_fget'; did you mean 'eventfd_signal'? > > [-Werror=implicit-function-declaration] > > eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd); > > ^~~~ > > eventfd_signal > > ../drivers/vhost/vhost.c:1577:31: warning: pointer/integer type mismatch in > > conditional expression > > eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd); > > > Will fix. > > VHOST should depend on EVENTFD now. > > Thanks I did that and pushed. Pls take a look. > > > ^ > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: linux-next: Tree for Mar 30 (vhost)
On Mon, Mar 30, 2020 at 10:22:22AM -0700, Randy Dunlap wrote: > On 3/30/20 2:43 AM, Stephen Rothwell wrote: > > Hi all, > > > > The merge window has opened, so please do not add any material for the > > next release into your linux-next included trees/branches until after > > the merge window closes. > > > > Changes since 20200327: > > > > The vhost tree gained a conflict against the kvm-arm tree. > > > > (note: today's linux-next is on 5.6-rc7.) > > on x86_64: > > # CONFIG_EVENTFD is not set Oh, this is Jason's Kconfig refactoring. Vhost must depend on eventfd of course. I fixed the relevant commit up and pushed the new tree again. Would appreciate a report on whether any problems are left. > ../drivers/vhost/vhost.c: In function 'vhost_vring_ioctl': > ../drivers/vhost/vhost.c:1577:33: error: implicit declaration of function > 'eventfd_fget'; did you mean 'eventfd_signal'? > [-Werror=implicit-function-declaration] >eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd); > ^~~~ > eventfd_signal > ../drivers/vhost/vhost.c:1577:31: warning: pointer/integer type mismatch in > conditional expression >eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd); >^ > > -- > ~Randy > Reported-by: Randy Dunlap ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC for Linux] virtio_balloon: Add VIRTIO_BALLOON_F_THP_ORDER to handle THP spilt issue
On 26.03.20 10:49, Michael S. Tsirkin wrote:
> On Thu, Mar 26, 2020 at 08:54:04AM +0100, David Hildenbrand wrote:
>>
>>
>>> Am 26.03.2020 um 08:21 schrieb Michael S. Tsirkin :
>>>
>>> On Thu, Mar 12, 2020 at 09:51:25AM +0100, David Hildenbrand wrote:
> On 12.03.20 09:47, Michael S. Tsirkin wrote:
> On Thu, Mar 12, 2020 at 09:37:32AM +0100, David Hildenbrand wrote:
>> 2. You are essentially stealing THPs in the guest. So the fastest
>> mapping (THP in guest and host) is gone. The guest won't be able to make
>> use of THP where it previously was able to. I can imagine this implies a
>> performance degradation for some workloads. This needs a proper
>> performance evaluation.
>
> I think the problem is more with the alloc_pages API.
> That gives you exactly the given order, and if there's
> a larger chunk available, it will split it up.
>
> But for balloon - I suspect lots of other users,
> we do not want to stress the system but if a large
> chunk is available anyway, then we could handle
> that more optimally by getting it all in one go.
>
>
> So if we want to address this, IMHO this calls for a new API.
> Along the lines of
>
>struct page *alloc_page_range(gfp_t gfp, unsigned int min_order,
>unsigned int max_order, unsigned int *order)
>
> the idea would then be to return at a number of pages in the given
> range.
>
> What do you think? Want to try implementing that?
You can just start with the highest order and decrement the order until
your allocation succeeds using alloc_pages(), which would be enough for
a first version. At least I don't see the immediate need for a new
kernel API.
>>>
>>> OK I remember now. The problem is with reclaim. Unless reclaim is
>>> completely disabled, any of these calls can sleep. After it wakes up,
>>> we would like to get the larger order that has become available
>>> meanwhile.
>>>
>>
>> Yes, but that‘s a pure optimization IMHO.
>> So I think we should do a trivial implementation first and then see what we
>> gain from a new allocator API. Then we might also be able to justify it
>> using real numbers.
>>
>
> Well how do you propose implement the necessary semantics?
> I think we are both agreed that alloc_page_range is more or
> less what's necessary anyway - so how would you approximate it
> on top of existing APIs?
Looking at drivers/misc/vmw_balloon.c:vmballoon_inflate(), it first
tries to allocate huge pages using
alloc_pages(__GFP_HIGHMEM|__GFP_NOWARN| __GFP_NOMEMALLOC,
VMW_BALLOON_2M_ORDER)
And then falls back to 4k allocations (balloon_page_alloc()) in case
allocation fails.
I'm roughly thinking of something like the following, but with an
optimized reporting interface/bigger pfn array so we can report >
1MB at a time. Also, it might make sense to remember the order that
succeeded across some fill_balloon() calls.
Don't even expect it to compile ...
From 4305f989672ccca4be9293e6d4167e929f3e299b Mon Sep 17 00:00:00 2001
From: David Hildenbrand
Date: Tue, 31 Mar 2020 12:28:07 +0200
Subject: [PATCH RFC] tmp
Signed-off-by: David Hildenbrand
---
drivers/virtio/virtio_balloon.c| 38 ++
include/linux/balloon_compaction.h | 7 -
mm/balloon_compaction.c| 43 +++---
3 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 8511d258dbb4..0660b1b988f0 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -187,7 +187,7 @@ int virtballoon_free_page_report(struct
page_reporting_dev_info *pr_dev_info,
}
static void set_page_pfns(struct virtio_balloon *vb,
- __virtio32 pfns[], struct page *page)
+ __virtio32 pfns[], struct page *page, int order)
{
unsigned int i;
@@ -197,7 +197,7 @@ static void set_page_pfns(struct virtio_balloon *vb,
* Set balloon pfns pointing at this page.
* Note that the first pfn points at start of the page.
*/
- for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
+ for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE * (1 << order); i++)
pfns[i] = cpu_to_virtio32(vb->vdev,
page_to_balloon_pfn(page) + i);
}
@@ -205,6 +205,7 @@ static void set_page_pfns(struct virtio_balloon *vb,
static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
{
unsigned num_allocated_pages;
+ int order = MAX_ORDER - 1;
unsigned num_pfns;
struct page *page;
LIST_HEAD(pages);
@@ -212,9 +213,20 @@ static unsigned fill_balloon(struct virtio_balloon *vb,
size_t num)
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
+ /*
+* Note: we will
Re: [PATCH 20/22] drm/vkms: Use simple encoder
Hi
Am 24.03.20 um 12:59 schrieb Rodrigo Siqueira:
> Hi Thomas,
>
> First of all, thanks for your patch!
>
> I applied all your series, compiled it, and when I tried
> `make INSTALL_MOD_PATH=/PATH/ modules_instal` I got the following
> message:
>
> depmod: ERROR: Cycle detected: drm_kms_helper -> drm -> drm_kms_helper
> depmod: ERROR: Found 2 modules in dependency cycles!
> make: *** [Makefile:1317: _modinst_post] Error 1
>
> I cleaned up my local files and tried again, but I got the same error;
> If I just use `drm-misc-next` everything is fine. Did I miss something?
I didn't change any module dependencies. Does it happen without this
patches?
Best regards
Thomas
>
> Thanks
>
> On 03/05, Thomas Zimmermann wrote:
>> The vkms driver uses an empty implementation for its encoder. Replace
>> the code with the generic simple encoder.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/vkms/vkms_output.c | 8 ++--
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vkms/vkms_output.c
>> b/drivers/gpu/drm/vkms/vkms_output.c
>> index fb1941a6522c..85afb77e97f0 100644
>> --- a/drivers/gpu/drm/vkms/vkms_output.c
>> +++ b/drivers/gpu/drm/vkms/vkms_output.c
>> @@ -3,6 +3,7 @@
>> #include "vkms_drv.h"
>> #include
>> #include
>> +#include
>>
>> static void vkms_connector_destroy(struct drm_connector *connector)
>> {
>> @@ -17,10 +18,6 @@ static const struct drm_connector_funcs
>> vkms_connector_funcs = {
>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> };
>>
>> -static const struct drm_encoder_funcs vkms_encoder_funcs = {
>> -.destroy = drm_encoder_cleanup,
>> -};
>> -
>> static int vkms_conn_get_modes(struct drm_connector *connector)
>> {
>> int count;
>> @@ -70,8 +67,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int
>> index)
>>
>> drm_connector_helper_add(connector, _conn_helper_funcs);
>>
>> -ret = drm_encoder_init(dev, encoder, _encoder_funcs,
>> - DRM_MODE_ENCODER_VIRTUAL, NULL);
>> +ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_VIRTUAL);
>> if (ret) {
>> DRM_ERROR("Failed to init encoder\n");
>> goto err_encoder;
>> --
>> 2.25.1
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -next] virtio: vdpa: remove unused variables 'ifcvf' and 'ifcvf_lm'
On 2020/3/31 下午4:02, YueHaibing wrote:
drivers/virtio/vdpa/ifcvf/ifcvf_main.c:34:24:
warning: variable ‘ifcvf’ set but not used [-Wunused-but-set-variable]
drivers/virtio/vdpa/ifcvf/ifcvf_base.c:304:31:
warning: variable ‘ifcvf_lm’ set but not used [-Wunused-but-set-variable]
Reported-by: Hulk Robot
Signed-off-by: YueHaibing
Acked-by: Jason Wang
Thanks
---
drivers/virtio/vdpa/ifcvf/ifcvf_base.c | 2 --
drivers/virtio/vdpa/ifcvf/ifcvf_main.c | 2 --
2 files changed, 4 deletions(-)
diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
b/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
index b61b06ea26d3..e24371d644b5 100644
--- a/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
@@ -301,12 +301,10 @@ int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u64
num)
static int ifcvf_hw_enable(struct ifcvf_hw *hw)
{
- struct ifcvf_lm_cfg __iomem *ifcvf_lm;
struct virtio_pci_common_cfg __iomem *cfg;
struct ifcvf_adapter *ifcvf;
u32 i;
- ifcvf_lm = (struct ifcvf_lm_cfg __iomem *)hw->lm_cfg;
ifcvf = vf_to_adapter(hw);
cfg = hw->common_cfg;
ifc_iowrite16(IFCVF_MSI_CONFIG_OFF, >msix_config);
diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
b/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
index 8d54dc5b08d2..28d9e5de5675 100644
--- a/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
@@ -31,11 +31,9 @@ static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
static int ifcvf_start_datapath(void *private)
{
struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
- struct ifcvf_adapter *ifcvf;
u8 status;
int ret;
- ifcvf = vf_to_adapter(vf);
vf->nr_vring = IFCVF_MAX_QUEUE_PAIRS * 2;
ret = ifcvf_start_hw(vf);
if (ret < 0) {
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [vhost:linux-next 8/13] include/linux/vringh.h:18:10: fatal error: linux/vhost_iotlb.h: No such file or directory
On 2020/3/31 下午3:31, Xia, Hui wrote: -Original Message- From: Jason Wang Sent: 2020年3月30日 10:47 To: lkp Cc:kbuild-...@lists.01.org;k...@vger.kernel.org;virtualization@lists.linux- foundation.org;net...@vger.kernel.org; Michael S. Tsirkin Subject: Re: [vhost:linux-next 8/13] include/linux/vringh.h:18:10: fatal error: linux/vhost_iotlb.h: No such file or directory On 2020/3/29 下午8:08, kbuild test robot wrote: tree:https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next head: f44a63f9ebf66a450c101084a35a3ef158ead209 commit: c43908b0b9a900bd51f861f4c57b83cfd932f4d2 [8/13] vringh: IOTLB support config: arm-em_x270_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce: wgethttps://raw.githubusercontent.com/intel/lkp- tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout c43908b0b9a900bd51f861f4c57b83cfd932f4d2 I could not find this commit in the above branch. # save the attached .config to linux build tree GCC_VERSION=9.3.0 make.cross ARCH=arm Try to use commit dc3b0673ae5efb73edab66ec5c2f074272e9a4df. But this command does not work (I remember it used to work): # GCC_VERSION=9.3.0 make.cross ARCH=arm cd: received redirection to `https://download.01.org/0day-ci/cross-package/' lftpget -c https://download.01.org/0day-ci/cross-package/./gcc-9.3.0-nolibc/x86_64-gcc- 9.3.0-nolibc_arm-linux-gnueabihf.tar.xz tar Jxf gcc-9.3.0-nolibc/x86_64-gcc-9.3.0-nolibc_arm-linux-gnueabihf.tar.xz -C /root/0day No cross compiler for arm setup_crosstool failed Hi Jason, thanks for report this issue. It is caused by wrong finding in 2 cross tools for arm. And has been fixed. Thanks. Regarding to the vhost build issue itself, it has gone in latest vhost/linux-next. The cause is the code kbuild captured didn't have patch " vhost: factor out IOTLB " which introduce linux/vhost_iotlb.h at that moment. So just ignore this issue since the missed patch has been added in latest vhost/linux-next. Good to know this. Thanks for the updating. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH -next] virtio: vdpa: remove unused variables 'ifcvf' and 'ifcvf_lm'
drivers/virtio/vdpa/ifcvf/ifcvf_main.c:34:24:
warning: variable ‘ifcvf’ set but not used [-Wunused-but-set-variable]
drivers/virtio/vdpa/ifcvf/ifcvf_base.c:304:31:
warning: variable ‘ifcvf_lm’ set but not used [-Wunused-but-set-variable]
Reported-by: Hulk Robot
Signed-off-by: YueHaibing
---
drivers/virtio/vdpa/ifcvf/ifcvf_base.c | 2 --
drivers/virtio/vdpa/ifcvf/ifcvf_main.c | 2 --
2 files changed, 4 deletions(-)
diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
b/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
index b61b06ea26d3..e24371d644b5 100644
--- a/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/virtio/vdpa/ifcvf/ifcvf_base.c
@@ -301,12 +301,10 @@ int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u64
num)
static int ifcvf_hw_enable(struct ifcvf_hw *hw)
{
- struct ifcvf_lm_cfg __iomem *ifcvf_lm;
struct virtio_pci_common_cfg __iomem *cfg;
struct ifcvf_adapter *ifcvf;
u32 i;
- ifcvf_lm = (struct ifcvf_lm_cfg __iomem *)hw->lm_cfg;
ifcvf = vf_to_adapter(hw);
cfg = hw->common_cfg;
ifc_iowrite16(IFCVF_MSI_CONFIG_OFF, >msix_config);
diff --git a/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
b/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
index 8d54dc5b08d2..28d9e5de5675 100644
--- a/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/virtio/vdpa/ifcvf/ifcvf_main.c
@@ -31,11 +31,9 @@ static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
static int ifcvf_start_datapath(void *private)
{
struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
- struct ifcvf_adapter *ifcvf;
u8 status;
int ret;
- ifcvf = vf_to_adapter(vf);
vf->nr_vring = IFCVF_MAX_QUEUE_PAIRS * 2;
ret = ifcvf_start_hw(vf);
if (ret < 0) {
--
2.17.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH Resend] drm/qxl: Use correct notify port address when creating cursor ring
The command ring and cursor ring use different notify port addresses definition: QXL_IO_NOTIFY_CMD and QXL_IO_NOTIFY_CURSOR. However, in qxl_device_init() we use QXL_IO_NOTIFY_CMD to create both command ring and cursor ring. This doesn't cause any problems now, because QEMU's behaviors on QXL_IO_NOTIFY_CMD and QXL_IO_NOTIFY_CURSOR are the same. However, QEMU's behavior may be change in future, so let's fix it. P.S.: In the X.org QXL driver, the notify port address of cursor ring is correct. Cc: Signed-off-by: Huacai Chen --- drivers/gpu/drm/qxl/qxl_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index bfc1631..9bdbe0d 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -218,7 +218,7 @@ int qxl_device_init(struct qxl_device *qdev, &(qdev->ram_header->cursor_ring_hdr), sizeof(struct qxl_command), QXL_CURSOR_RING_SIZE, - qdev->io_base + QXL_IO_NOTIFY_CMD, + qdev->io_base + QXL_IO_NOTIFY_CURSOR, false, >cursor_event); -- 2.7.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 -next] vdpasim: Return status in vdpasim_get_status
vdpasim->status should acquired under spin lock.
Fixes: 870448c31775 ("vdpasim: vDPA device simulator")
Signed-off-by: YueHaibing
Acked-by: Jason Wang
---
v2: Fix patch title typo
---
drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
b/drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
index 6e8a0cf2fdeb..72863d01a12a 100644
--- a/drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/virtio/vdpa/vdpa_sim/vdpa_sim.c
@@ -488,7 +488,7 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa)
status = vdpasim->status;
spin_unlock(>lock);
- return vdpasim->status;
+ return status;
}
static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status)
--
2.17.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
