* Re: [PATCH] vhost/scsi: truncate T10 PI iov_iter to prot_bytes
From: Greg Edwards @ 2018-09-21 17:49 UTC (permalink / raw)
To: virtualization; +Cc: Paolo Bonzini, Mike Christie, Michael S. Tsirkin
In-Reply-To: <20180822192153.24217-1-gedwards@ddn.com>
On Wed, Aug 22, 2018 at 01:21:53PM -0600, Greg Edwards wrote:
> Commands with protection information included were not truncating the
> protection iov_iter to the number of protection bytes in the command.
> This resulted in vhost_scsi mis-calculating the size of the protection
> SGL in vhost_scsi_calc_sgls(), and including both the protection and
> data SG entries in the protection SGL.
>
> Fixes: 09b13fa8c1a1 ("vhost/scsi: Add ANY_LAYOUT support in vhost_scsi_handle_vq")
> Signed-off-by: Greg Edwards <gedwards@ddn.com>
Any thoughts on this patch?
> ---
> drivers/vhost/scsi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 76f8d649147b..cbe0ea26c1ff 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -964,7 +964,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
> prot_bytes = vhost32_to_cpu(vq, v_req_pi.pi_bytesin);
> }
> /*
> - * Set prot_iter to data_iter, and advance past any
> + * Set prot_iter to data_iter and truncate it to
> + * prot_bytes, and advance data_iter past any
> * preceeding prot_bytes that may be present.
> *
> * Also fix up the exp_data_len to reflect only the
> @@ -973,6 +974,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
> if (prot_bytes) {
> exp_data_len -= prot_bytes;
> prot_iter = data_iter;
> + iov_iter_truncate(&prot_iter, prot_bytes);
> iov_iter_advance(&data_iter, prot_bytes);
> }
> tag = vhost64_to_cpu(vq, v_req_pi.tag);
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH] vhost/scsi: truncate T10 PI iov_iter to prot_bytes
From: Michael S. Tsirkin @ 2018-09-21 17:57 UTC (permalink / raw)
To: Greg Edwards; +Cc: Paolo Bonzini, Mike Christie, virtualization
In-Reply-To: <20180921174912.GA19193@psuche>
On Fri, Sep 21, 2018 at 11:49:12AM -0600, Greg Edwards wrote:
> On Wed, Aug 22, 2018 at 01:21:53PM -0600, Greg Edwards wrote:
> > Commands with protection information included were not truncating the
> > protection iov_iter to the number of protection bytes in the command.
> > This resulted in vhost_scsi mis-calculating the size of the protection
> > SGL in vhost_scsi_calc_sgls(), and including both the protection and
> > data SG entries in the protection SGL.
> >
> > Fixes: 09b13fa8c1a1 ("vhost/scsi: Add ANY_LAYOUT support in vhost_scsi_handle_vq")
> > Signed-off-by: Greg Edwards <gedwards@ddn.com>
>
>
> Any thoughts on this patch?
Paolo could you comment on the virtio-scsi aspects pls?
>
> > ---
> > drivers/vhost/scsi.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > index 76f8d649147b..cbe0ea26c1ff 100644
> > --- a/drivers/vhost/scsi.c
> > +++ b/drivers/vhost/scsi.c
> > @@ -964,7 +964,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
> > prot_bytes = vhost32_to_cpu(vq, v_req_pi.pi_bytesin);
> > }
> > /*
> > - * Set prot_iter to data_iter, and advance past any
> > + * Set prot_iter to data_iter and truncate it to
> > + * prot_bytes, and advance data_iter past any
> > * preceeding prot_bytes that may be present.
> > *
> > * Also fix up the exp_data_len to reflect only the
> > @@ -973,6 +974,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
> > if (prot_bytes) {
> > exp_data_len -= prot_bytes;
> > prot_iter = data_iter;
> > + iov_iter_truncate(&prot_iter, prot_bytes);
> > iov_iter_advance(&data_iter, prot_bytes);
> > }
> > tag = vhost64_to_cpu(vq, v_req_pi.tag);
> > --
> > 2.17.1
^ permalink raw reply
* Re: [PATCH v8 00/10] x86: macrofying inline asm for better compilation
From: Kees Cook @ 2018-09-21 18:26 UTC (permalink / raw)
To: Nadav Amit
Cc: Kate Stewart, Peter Zijlstra, Christopher Li, virtualization,
Max Filippov, Jan Beulich, H. Peter Anvin, Sam Ravnborg,
Thomas Gleixner, X86 ML, Sparse Mailing-list, Ingo Molnar,
linux-xtensa, Josh Poimboeuf, Alok Kataria, Juergen Gross,
Chris Zankel, Masahiro Yamada, Greg Kroah-Hartman, LKML,
Philippe Ombredanne, Linus Torvalds
In-Reply-To: <20180918212847.199085-1-namit@vmware.com>
On Tue, Sep 18, 2018 at 2:28 PM, Nadav Amit <namit@vmware.com> wrote:
> This patch-set deals with an interesting yet stupid problem: kernel code
> that does not get inlined despite its simplicity. There are several
> causes for this behavior: "cold" attribute on __init, different function
> optimization levels; conditional constant computations based on
> __builtin_constant_p(); and finally large inline assembly blocks.
>
> This patch-set deals with the inline assembly problem. I separated these
> patches from the others (that were sent in the RFC) for easier
> inclusion. I also separated the removal of unnecessary new-lines which
> would be sent separately.
>
> The problem with inline assembly is that inline assembly is often used
> by the kernel for things that are other than code - for example,
> assembly directives and data. GCC however is oblivious to the content of
> the blocks and assumes their cost in space and time is proportional to
> the number of the perceived assembly "instruction", according to the
> number of newlines and semicolons. Alternatives, paravirt and other
> mechanisms are affected, causing code not to be inlined, and degrading
> compilation quality in general.
>
> The solution that this patch-set carries for this problem is to create
> an assembly macro, and then call it from the inline assembly block. As
> a result, the compiler sees a single "instruction" and assigns the more
> appropriate cost to the code.
>
> To avoid uglification of the code, as many noted, the macros are first
> precompiled into an assembly file, which is later assembled together
> with the C files. This also enables to avoid duplicate implementation
> that was set before for the asm and C code. This can be seen in the
> exception table changes.
>
> Overall this patch-set slightly increases the kernel size (my build was
> done using my Ubuntu 18.04 config + localyesconfig for the record):
>
> text data bss dec hex filename
> 18140829 10224724 2957312 31322865 1ddf2f1 ./vmlinux before
> 18163608 10227348 2957312 31348268 1de562c ./vmlinux after (+0.1%)
>
> The number of static functions in the image is reduced by 379, but
> actually inlining is even better, which does not always shows in these
> numbers: a function may be inlined causing the calling function not to
> be inlined.
>
> I ran some limited number of benchmarks, and in general the performance
> impact is not very notable. You can still see >10 cycles shaved off some
> syscalls that manipulate page-tables (e.g., mprotect()), in which
> paravirt caused many functions not to be inlined. In addition this
> patch-set can prevent issues such as [1], and improves code readability
> and maintainability.
>
> [1] https://patchwork.kernel.org/patch/10450037/
>
> v7->v8: * Add acks (Masahiro, Max)
> * Rebase on 4.19 (Ingo)
I've tested the series for booting and with the refcount lkdtm tests.
Looks good, thanks!
Tested-by: Kees Cook <keescook@chromium.org>
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH net-next v9 0/6] net: vhost: improve performance when enable busyloop
From: Tonghao Zhang @ 2018-09-23 13:48 UTC (permalink / raw)
To: jasowang, mst, makita.toshiaki
Cc: Linux Kernel Network Developers, virtualization
In-Reply-To: <1536493887-2637-1-git-send-email-xiangxia.m.yue@gmail.com>
On Sun, Sep 9, 2018 at 7:52 PM <xiangxia.m.yue@gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patches improve the guest receive performance.
> On the handle_tx side, we poll the sock receive queue
> at the same time. handle_rx do that in the same way.
>
> For more performance report, see patch 4, 5, 6
>
> Tonghao Zhang (6):
> net: vhost: lock the vqs one by one
> net: vhost: replace magic number of lock annotation
> net: vhost: factor out busy polling logic to vhost_net_busy_poll()
> net: vhost: add rx busy polling in tx path
> net: vhost: disable rx wakeup during tx busypoll
> net: vhost: make busyloop_intr more accurate
>
> drivers/vhost/net.c | 163 +++++++++++++++++++++++++++++++-------------------
> drivers/vhost/vhost.c | 24 +++-----
> 2 files changed, 108 insertions(+), 79 deletions(-)
>
> --
> 1.8.3.1
>
ping ?
^ permalink raw reply
* Re: [PATCH] vhost/scsi: truncate T10 PI iov_iter to prot_bytes
From: Paolo Bonzini @ 2018-09-24 9:09 UTC (permalink / raw)
To: Greg Edwards, virtualization; +Cc: Mike Christie, Michael S. Tsirkin
In-Reply-To: <20180921174912.GA19193@psuche>
On 21/09/2018 19:49, Greg Edwards wrote:
> On Wed, Aug 22, 2018 at 01:21:53PM -0600, Greg Edwards wrote:
>> Commands with protection information included were not truncating the
>> protection iov_iter to the number of protection bytes in the command.
>> This resulted in vhost_scsi mis-calculating the size of the protection
>> SGL in vhost_scsi_calc_sgls(), and including both the protection and
>> data SG entries in the protection SGL.
>>
>> Fixes: 09b13fa8c1a1 ("vhost/scsi: Add ANY_LAYOUT support in vhost_scsi_handle_vq")
>> Signed-off-by: Greg Edwards <gedwards@ddn.com>
>
>
> Any thoughts on this patch?
>
>
>> ---
>> drivers/vhost/scsi.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
>> index 76f8d649147b..cbe0ea26c1ff 100644
>> --- a/drivers/vhost/scsi.c
>> +++ b/drivers/vhost/scsi.c
>> @@ -964,7 +964,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
>> prot_bytes = vhost32_to_cpu(vq, v_req_pi.pi_bytesin);
>> }
>> /*
>> - * Set prot_iter to data_iter, and advance past any
>> + * Set prot_iter to data_iter and truncate it to
>> + * prot_bytes, and advance data_iter past any
>> * preceeding prot_bytes that may be present.
>> *
>> * Also fix up the exp_data_len to reflect only the
>> @@ -973,6 +974,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
>> if (prot_bytes) {
>> exp_data_len -= prot_bytes;
>> prot_iter = data_iter;
>> + iov_iter_truncate(&prot_iter, prot_bytes);
>> iov_iter_advance(&data_iter, prot_bytes);
>> }
>> tag = vhost64_to_cpu(vq, v_req_pi.tag);
>> --
>> 2.17.1
Fixes: 09b13fa8c1a1093e9458549ac8bb203a7c65c62a
Cc: stable@vger.kernel.org
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply
* Re: [PATCH 0/2] virtio/s390: fix some races in virtio-ccw
From: Christian Borntraeger @ 2018-09-24 11:52 UTC (permalink / raw)
To: Halil Pasic, Cornelia Huck, linux-s390, virtualization, kvm
Cc: Colin Ian King
In-Reply-To: <20180921124621.43649-1-pasic@linux.ibm.com>
Can you consider adding stable tags to the (final version of the) fixes?
On 09/21/2018 02:46 PM, Halil Pasic wrote:
> The trigger for the series is this bug report:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1788432
>
> Changelog:
> RFC -> v1:
> * do mutual exclusion on a per device basis in ccw_io_helper()
>
> Halil Pasic (2):
> virtio/s390: avoid race on vcdev->config
> virtio/s390: fix race in ccw_io_helper()
>
> drivers/s390/virtio/virtio_ccw.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 1/2] virtio/s390: avoid race on vcdev->config
From: Cornelia Huck @ 2018-09-24 12:55 UTC (permalink / raw)
To: Farhan Ali, Halil Pasic, Christian Borntraeger
Cc: linux-s390, Colin Ian King, kvm, virtualization
In-Reply-To: <80a80dc6-ccb1-f245-b367-c2f9345ca151@linux.ibm.com>
On Fri, 21 Sep 2018 17:47:47 -0400
Farhan Ali <alifm@linux.ibm.com> wrote:
> On 09/21/2018 09:28 AM, Halil Pasic wrote:
> > Anyway I'm fine with swapping the old out and your new version in,
> > if you prefer it that way.
> >
> > If you do, would you like to have a respin?
Just send me a respin with something that you find useful :) (maybe a
mashup of our descriptions) (while at it, you could also add the
cc:stable, which I agree make sense)
> >
> > Regards,
> > Halil
> >
>
> I had been looking into this code recently, and shouldn't vcdev->status
> (function get/set_status functions) also have a lock around it? Or is it
> not possible to have a race condition on vcdev->status?
I don't think so, as status is only a byte.
>
> Thanks
> Farhan
>
>
> >>>
> >>> Let us protect the shared state using vcdev->lock.
> >>>
> >>> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> >>> ---
> >>> drivers/s390/virtio/virtio_ccw.c | 10 ++++++++--
> >>> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >
> >
>
^ permalink raw reply
* Re: [PATCH 2/2] virtio/s390: fix race in ccw_io_helper()
From: Cornelia Huck @ 2018-09-24 13:32 UTC (permalink / raw)
To: Halil Pasic; +Cc: linux-s390, Colin Ian King, kvm, virtualization
In-Reply-To: <a55df275-d982-c051-1b17-465e3a30777f@linux.ibm.com>
On Mon, 24 Sep 2018 14:57:26 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On 09/21/2018 03:30 PM, Cornelia Huck wrote:
> > On Fri, 21 Sep 2018 14:46:21 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> Again, not what I tried to say. By 'This' I meant 'this race'. How
> about?
>
> """
> This race normally does not cause any problems. The operations provided
> by struct virtio_config_ops are usually invoked in a well defined sequence,
> normally don't fail, and are normally usually quite infrequent too.
s/usually/used/
>
> Yet, if some of the these operations are directly triggered via
> sysfs attributes, like in the case described by the referenced bug,
> userspace is given an opportunity to force races by increasing the
> frequency of the given operations.
> """
Fine with me.
^ permalink raw reply
* Re: [PATCH 1/2] virtio/s390: avoid race on vcdev->config
From: Cornelia Huck @ 2018-09-24 14:50 UTC (permalink / raw)
To: Halil Pasic; +Cc: linux-s390, kvm, Farhan Ali, virtualization, Colin Ian King
In-Reply-To: <2b8982be-9066-cf08-657b-99ecf889e4a7@linux.ibm.com>
On Mon, 24 Sep 2018 16:27:30 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On 09/24/2018 02:55 PM, Cornelia Huck wrote:
> > On Fri, 21 Sep 2018 17:47:47 -0400
> > Farhan Ali <alifm@linux.ibm.com> wrote:
> >
> >> On 09/21/2018 09:28 AM, Halil Pasic wrote:
> >
> >>> Anyway I'm fine with swapping the old out and your new version in,
> >>> if you prefer it that way.
> >>>
> >>> If you do, would you like to have a respin?
> >
> > Just send me a respin with something that you find useful :) (maybe a
> > mashup of our descriptions) (while at it, you could also add the
> > cc:stable, which I agree make sense)
> >
>
> Will do! I would like to go with.
>
>
> """
> This normally does not cause problems, as these are usually infrequent
> operations. However, for some devices writing to/reading from the config space
> can be triggered through sysfs attributes. For these userspace can force the
> race by increasing the frequency.
> """
Sounds good.
^ permalink raw reply
* Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support
From: Arnd Bergmann @ 2018-09-24 21:08 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Florian Weimer, Juergen Gross, Peter Zijlstra,
the arch/x86 maintainers, Linux Kernel Mailing List,
virtualization, Stephen Boyd, John Stultz, Deepa Dinamani,
Andy Lutomirski, Paolo Bonzini, devel, matt
In-Reply-To: <alpine.DEB.2.21.1809171451380.16580@nanos.tec.linutronix.de>
On Mon, Sep 17, 2018 at 3:00 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Fri, 14 Sep 2018, Arnd Bergmann wrote:
> > On Fri, Sep 14, 2018 at 2:52 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> > A couple of architectures (s390, ia64, riscv, powerpc, arm64)
> > implement the vdso as assembler code at the moment, so they
> > won't be as easy to consolidate (other than outright replacing all
> > the code).
> >
> > The other five:
> > arch/x86/entry/vdso/vclock_gettime.c
> > arch/sparc/vdso/vclock_gettime.c
> > arch/nds32/kernel/vdso/gettimeofday.c
> > arch/mips/vdso/gettimeofday.c
> > arch/arm/vdso/vgettimeofday.c
> >
> > are basically all minor variations of the same code base and could be
> > consolidated to some degree.
> > Any suggestions here? Should we plan to do that consolitdation based on
> > your new version, or just add clock_gettime64 in arm32 and x86-32, and then
> > be done with it? The other ones will obviously still be fast for 32-bit time_t
> > and will have a working non-vdso sys_clock_getttime64().
>
> In principle consolidating all those implementations should be possible to
> some extent and probably worthwhile. What's arch specific are the actual
> accessors to the hardware clocks.
Ok.
> > I also wonder about clock_getres(): half the architectures seem to implement
> > it in vdso, but notably arm32 and x86 don't, and I had not expected it to be
> > performance critical given that the result is easily cached in user space.
>
> getres() is not really performance critical, but adding it does not create
> a huge problem either.
Right, I'd just not add a getres_time64() to the vdso then.
Arnd
^ permalink raw reply
* Re: [PATCH net-next v9 0/6] net: vhost: improve performance when enable busyloop
From: Jason Wang @ 2018-09-25 2:21 UTC (permalink / raw)
To: Tonghao Zhang, mst, makita.toshiaki
Cc: Linux Kernel Network Developers, virtualization
In-Reply-To: <CAMDZJNWk7UThRus4fZeznx4SvWjtj38ruC6Y_xPc1LV95EfLnA@mail.gmail.com>
On 2018年09月23日 21:48, Tonghao Zhang wrote:
> On Sun, Sep 9, 2018 at 7:52 PM <xiangxia.m.yue@gmail.com> wrote:
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> This patches improve the guest receive performance.
>> On the handle_tx side, we poll the sock receive queue
>> at the same time. handle_rx do that in the same way.
>>
>> For more performance report, see patch 4, 5, 6
>>
>> Tonghao Zhang (6):
>> net: vhost: lock the vqs one by one
>> net: vhost: replace magic number of lock annotation
>> net: vhost: factor out busy polling logic to vhost_net_busy_poll()
>> net: vhost: add rx busy polling in tx path
>> net: vhost: disable rx wakeup during tx busypoll
>> net: vhost: make busyloop_intr more accurate
>>
>> drivers/vhost/net.c | 163 +++++++++++++++++++++++++++++++-------------------
>> drivers/vhost/vhost.c | 24 +++-----
>> 2 files changed, 108 insertions(+), 79 deletions(-)
>>
>> --
>> 1.8.3.1
>>
> ping ?
HI Tonghao:
As David pointed out, we only receive first 4 patches. You probably need
to resend the series.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v9 0/6] net: vhost: improve performance when enable busyloop
From: Tonghao Zhang @ 2018-09-25 2:51 UTC (permalink / raw)
To: Jason Wang; +Cc: Linux Kernel Network Developers, virtualization, mst
In-Reply-To: <08f01199-ad0f-446c-0bcf-187dc31f2b42@redhat.com>
On Tue, Sep 25, 2018 at 10:21 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 2018年09月23日 21:48, Tonghao Zhang wrote:
> > On Sun, Sep 9, 2018 at 7:52 PM <xiangxia.m.yue@gmail.com> wrote:
> >> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >>
> >> This patches improve the guest receive performance.
> >> On the handle_tx side, we poll the sock receive queue
> >> at the same time. handle_rx do that in the same way.
> >>
> >> For more performance report, see patch 4, 5, 6
> >>
> >> Tonghao Zhang (6):
> >> net: vhost: lock the vqs one by one
> >> net: vhost: replace magic number of lock annotation
> >> net: vhost: factor out busy polling logic to vhost_net_busy_poll()
> >> net: vhost: add rx busy polling in tx path
> >> net: vhost: disable rx wakeup during tx busypoll
> >> net: vhost: make busyloop_intr more accurate
> >>
> >> drivers/vhost/net.c | 163 +++++++++++++++++++++++++++++++-------------------
> >> drivers/vhost/vhost.c | 24 +++-----
> >> 2 files changed, 108 insertions(+), 79 deletions(-)
> >>
> >> --
> >> 1.8.3.1
> >>
> > ping ?
>
> HI Tonghao:
>
> As David pointed out, we only receive first 4 patches. You probably need
> to resend the series.
OK
> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [REBASE PATCH net-next v9 0/4] net: vhost: improve performance when enable busyloop
From: xiangxia.m.yue @ 2018-09-25 12:36 UTC (permalink / raw)
To: jasowang, mst, makita.toshiaki, davem; +Cc: netdev, virtualization
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patches improve the guest receive performance.
On the handle_tx side, we poll the sock receive queue
at the same time. handle_rx do that in the same way.
For more performance report, see patch 4
Tonghao Zhang (4):
net: vhost: lock the vqs one by one
net: vhost: replace magic number of lock annotation
net: vhost: factor out busy polling logic to vhost_net_busy_poll()
net: vhost: add rx busy polling in tx path
drivers/vhost/net.c | 147 +++++++++++++++++++++++++++++---------------------
drivers/vhost/vhost.c | 24 +++------
2 files changed, 92 insertions(+), 79 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [REBASE PATCH net-next v9 1/4] net: vhost: lock the vqs one by one
From: xiangxia.m.yue @ 2018-09-25 12:36 UTC (permalink / raw)
To: jasowang, mst, makita.toshiaki, davem; +Cc: netdev, virtualization
In-Reply-To: <1537879012-20859-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patch changes the way that lock all vqs
at the same, to lock them one by one. It will
be used for next patch to avoid the deadlock.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index b13c6b4..f52008b 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -294,8 +294,11 @@ static void vhost_vq_meta_reset(struct vhost_dev *d)
{
int i;
- for (i = 0; i < d->nvqs; ++i)
+ for (i = 0; i < d->nvqs; ++i) {
+ mutex_lock(&d->vqs[i]->mutex);
__vhost_vq_meta_reset(d->vqs[i]);
+ mutex_unlock(&d->vqs[i]->mutex);
+ }
}
static void vhost_vq_reset(struct vhost_dev *dev,
@@ -891,20 +894,6 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
#define vhost_get_used(vq, x, ptr) \
vhost_get_user(vq, x, ptr, VHOST_ADDR_USED)
-static void vhost_dev_lock_vqs(struct vhost_dev *d)
-{
- int i = 0;
- for (i = 0; i < d->nvqs; ++i)
- mutex_lock_nested(&d->vqs[i]->mutex, i);
-}
-
-static void vhost_dev_unlock_vqs(struct vhost_dev *d)
-{
- int i = 0;
- for (i = 0; i < d->nvqs; ++i)
- mutex_unlock(&d->vqs[i]->mutex);
-}
-
static int vhost_new_umem_range(struct vhost_umem *umem,
u64 start, u64 size, u64 end,
u64 userspace_addr, int perm)
@@ -954,7 +943,10 @@ static void vhost_iotlb_notify_vq(struct vhost_dev *d,
if (msg->iova <= vq_msg->iova &&
msg->iova + msg->size - 1 >= vq_msg->iova &&
vq_msg->type == VHOST_IOTLB_MISS) {
+ mutex_lock(&node->vq->mutex);
vhost_poll_queue(&node->vq->poll);
+ mutex_unlock(&node->vq->mutex);
+
list_del(&node->node);
kfree(node);
}
@@ -986,7 +978,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,
int ret = 0;
mutex_lock(&dev->mutex);
- vhost_dev_lock_vqs(dev);
switch (msg->type) {
case VHOST_IOTLB_UPDATE:
if (!dev->iotlb) {
@@ -1020,7 +1011,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,
break;
}
- vhost_dev_unlock_vqs(dev);
mutex_unlock(&dev->mutex);
return ret;
--
1.8.3.1
^ permalink raw reply related
* [REBASE PATCH net-next v9 2/4] net: vhost: replace magic number of lock annotation
From: xiangxia.m.yue @ 2018-09-25 12:36 UTC (permalink / raw)
To: jasowang, mst, makita.toshiaki, davem; +Cc: netdev, virtualization
In-Reply-To: <1537879012-20859-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Use the VHOST_NET_VQ_XXX as a subclass for mutex_lock_nested.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 1bff6bc..5fe57ab 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -856,7 +856,7 @@ static void handle_tx(struct vhost_net *net)
struct vhost_virtqueue *vq = &nvq->vq;
struct socket *sock;
- mutex_lock(&vq->mutex);
+ mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
sock = vq->private_data;
if (!sock)
goto out;
@@ -921,7 +921,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
/* Flush batched heads first */
vhost_net_signal_used(rnvq);
/* Both tx vq and rx socket were polled here */
- mutex_lock_nested(&tvq->mutex, 1);
+ mutex_lock_nested(&tvq->mutex, VHOST_NET_VQ_TX);
vhost_disable_notify(&net->dev, tvq);
preempt_disable();
@@ -1063,7 +1063,7 @@ static void handle_rx(struct vhost_net *net)
__virtio16 num_buffers;
int recv_pkts = 0;
- mutex_lock_nested(&vq->mutex, 0);
+ mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_RX);
sock = vq->private_data;
if (!sock)
goto out;
--
1.8.3.1
^ permalink raw reply related
* [REBASE PATCH net-next v9 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: xiangxia.m.yue @ 2018-09-25 12:36 UTC (permalink / raw)
To: jasowang, mst, makita.toshiaki, davem; +Cc: netdev, virtualization
In-Reply-To: <1537879012-20859-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Factor out generic busy polling logic and will be
used for in tx path in the next patch. And with the patch,
qemu can set differently the busyloop_timeout for rx queue.
To avoid duplicate codes, introduce the helper functions:
* sock_has_rx_data(changed from sk_has_rx_data)
* vhost_net_busy_poll_try_queue
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/vhost/net.c | 110 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 70 insertions(+), 40 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 5fe57ab..ac0b954 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -480,6 +480,74 @@ static void vhost_tx_batch(struct vhost_net *net,
nvq->batched_xdp = 0;
}
+static int sock_has_rx_data(struct socket *sock)
+{
+ if (unlikely(!sock))
+ return 0;
+
+ if (sock->ops->peek_len)
+ return sock->ops->peek_len(sock);
+
+ return skb_queue_empty(&sock->sk->sk_receive_queue);
+}
+
+static void vhost_net_busy_poll_try_queue(struct vhost_net *net,
+ struct vhost_virtqueue *vq)
+{
+ if (!vhost_vq_avail_empty(&net->dev, vq)) {
+ vhost_poll_queue(&vq->poll);
+ } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
+ vhost_disable_notify(&net->dev, vq);
+ vhost_poll_queue(&vq->poll);
+ }
+}
+
+static void vhost_net_busy_poll(struct vhost_net *net,
+ struct vhost_virtqueue *rvq,
+ struct vhost_virtqueue *tvq,
+ bool *busyloop_intr,
+ bool poll_rx)
+{
+ unsigned long busyloop_timeout;
+ unsigned long endtime;
+ struct socket *sock;
+ struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
+
+ mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
+ vhost_disable_notify(&net->dev, vq);
+ sock = rvq->private_data;
+
+ busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
+ tvq->busyloop_timeout;
+
+ preempt_disable();
+ endtime = busy_clock() + busyloop_timeout;
+
+ while (vhost_can_busy_poll(endtime)) {
+ if (vhost_has_work(&net->dev)) {
+ *busyloop_intr = true;
+ break;
+ }
+
+ if ((sock_has_rx_data(sock) &&
+ !vhost_vq_avail_empty(&net->dev, rvq)) ||
+ !vhost_vq_avail_empty(&net->dev, tvq))
+ break;
+
+ cpu_relax();
+ }
+
+ preempt_enable();
+
+ if (poll_rx || sock_has_rx_data(sock))
+ vhost_net_busy_poll_try_queue(net, vq);
+ else if (!poll_rx) /* On tx here, sock has no rx data. */
+ vhost_enable_notify(&net->dev, rvq);
+
+ mutex_unlock(&vq->mutex);
+}
+
+
static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
struct vhost_net_virtqueue *nvq,
unsigned int *out_num, unsigned int *in_num,
@@ -897,16 +965,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
return len;
}
-static int sk_has_rx_data(struct sock *sk)
-{
- struct socket *sock = sk->sk_socket;
-
- if (sock->ops->peek_len)
- return sock->ops->peek_len(sock);
-
- return skb_queue_empty(&sk->sk_receive_queue);
-}
-
static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
bool *busyloop_intr)
{
@@ -914,41 +972,13 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
struct vhost_virtqueue *rvq = &rnvq->vq;
struct vhost_virtqueue *tvq = &tnvq->vq;
- unsigned long uninitialized_var(endtime);
int len = peek_head_len(rnvq, sk);
- if (!len && tvq->busyloop_timeout) {
+ if (!len && rvq->busyloop_timeout) {
/* Flush batched heads first */
vhost_net_signal_used(rnvq);
/* Both tx vq and rx socket were polled here */
- mutex_lock_nested(&tvq->mutex, VHOST_NET_VQ_TX);
- vhost_disable_notify(&net->dev, tvq);
-
- preempt_disable();
- endtime = busy_clock() + tvq->busyloop_timeout;
-
- while (vhost_can_busy_poll(endtime)) {
- if (vhost_has_work(&net->dev)) {
- *busyloop_intr = true;
- break;
- }
- if ((sk_has_rx_data(sk) &&
- !vhost_vq_avail_empty(&net->dev, rvq)) ||
- !vhost_vq_avail_empty(&net->dev, tvq))
- break;
- cpu_relax();
- }
-
- preempt_enable();
-
- if (!vhost_vq_avail_empty(&net->dev, tvq)) {
- vhost_poll_queue(&tvq->poll);
- } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
- vhost_disable_notify(&net->dev, tvq);
- vhost_poll_queue(&tvq->poll);
- }
-
- mutex_unlock(&tvq->mutex);
+ vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, true);
len = peek_head_len(rnvq, sk);
}
--
1.8.3.1
^ permalink raw reply related
* [REBASE PATCH net-next v9 4/4] net: vhost: add rx busy polling in tx path
From: xiangxia.m.yue @ 2018-09-25 12:36 UTC (permalink / raw)
To: jasowang, mst, makita.toshiaki, davem; +Cc: netdev, virtualization
In-Reply-To: <1537879012-20859-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patch improves the guest receive performance.
On the handle_tx side, we poll the sock receive queue at the
same time. handle_rx do that in the same way.
We set the poll-us=100us and use the netperf to test throughput
and mean latency. When running the tests, the vhost-net kthread
of that VM, is alway 100% CPU. The commands are shown as below.
Rx performance is greatly improved by this patch. There is not
notable performance change on tx with this series though. This
patch is useful for bi-directional traffic.
netperf -H IP -t TCP_STREAM -l 20 -- -O "THROUGHPUT, THROUGHPUT_UNITS, MEAN_LATENCY"
Topology:
[Host] ->linux bridge -> tap vhost-net ->[Guest]
TCP_STREAM:
* Without the patch: 19842.95 Mbps, 6.50 us mean latency
* With the patch: 37598.20 Mbps, 3.43 us mean latency
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/vhost/net.c | 35 ++++++++++++++---------------------
1 file changed, 14 insertions(+), 21 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index ac0b954..015abf3 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -547,34 +547,27 @@ static void vhost_net_busy_poll(struct vhost_net *net,
mutex_unlock(&vq->mutex);
}
-
static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
- struct vhost_net_virtqueue *nvq,
+ struct vhost_net_virtqueue *tnvq,
unsigned int *out_num, unsigned int *in_num,
struct msghdr *msghdr, bool *busyloop_intr)
{
- struct vhost_virtqueue *vq = &nvq->vq;
- unsigned long uninitialized_var(endtime);
- int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+ struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
+ struct vhost_virtqueue *rvq = &rnvq->vq;
+ struct vhost_virtqueue *tvq = &tnvq->vq;
+
+ int r = vhost_get_vq_desc(tvq, tvq->iov, ARRAY_SIZE(tvq->iov),
out_num, in_num, NULL, NULL);
- if (r == vq->num && vq->busyloop_timeout) {
+ if (r == tvq->num && tvq->busyloop_timeout) {
/* Flush batched packets first */
- if (!vhost_sock_zcopy(vq->private_data))
- vhost_tx_batch(net, nvq, vq->private_data, msghdr);
- preempt_disable();
- endtime = busy_clock() + vq->busyloop_timeout;
- while (vhost_can_busy_poll(endtime)) {
- if (vhost_has_work(vq->dev)) {
- *busyloop_intr = true;
- break;
- }
- if (!vhost_vq_avail_empty(vq->dev, vq))
- break;
- cpu_relax();
- }
- preempt_enable();
- r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+ if (!vhost_sock_zcopy(tvq->private_data))
+ // vhost_net_signal_used(tnvq);
+ vhost_tx_batch(net, tnvq, tvq->private_data, msghdr);
+
+ vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, false);
+
+ r = vhost_get_vq_desc(tvq, tvq->iov, ARRAY_SIZE(tvq->iov),
out_num, in_num, NULL, NULL);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 0/2] virtio/s390: fix some races in virtio-ccw
From: Cornelia Huck @ 2018-09-25 15:09 UTC (permalink / raw)
To: Halil Pasic; +Cc: linux-s390, kvm, stable, virtualization, Colin Ian King
In-Reply-To: <20180925121309.58524-1-pasic@linux.ibm.com>
On Tue, 25 Sep 2018 14:13:07 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> The trigger for the series is this bug report:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1788432
>
> Changelog:
> v1 -> v2:
> * improve on commit messages, add cc:stable
> RFC -> v1:
> * do mutual exclusion on a per device basis in ccw_io_helper()
>
> Halil Pasic (2):
> virtio/s390: avoid race on vcdev->config
> virtio/s390: fix race in ccw_io_helper()
>
> drivers/s390/virtio/virtio_ccw.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
Thanks, queued.
^ permalink raw reply
* Re: [PATCH v2 00/20] vmw_balloon: compaction, shrinker, 64-bit, etc.
From: Greg Kroah-Hartman @ 2018-09-25 18:15 UTC (permalink / raw)
To: Nadav Amit
Cc: Arnd Bergmann, Michael S. Tsirkin, Xavier Deguillard,
linux-kernel, virtualization, linux-mm
In-Reply-To: <20180920173026.141333-1-namit@vmware.com>
On Thu, Sep 20, 2018 at 10:30:06AM -0700, Nadav Amit wrote:
> This patch-set adds the following enhancements to the VMware balloon
> driver:
>
> 1. Balloon compaction support.
> 2. Report the number of inflated/deflated ballooned pages through vmstat.
> 3. Memory shrinker to avoid balloon over-inflation (and OOM).
> 4. Support VMs with memory limit that is greater than 16TB.
> 5. Faster and more aggressive inflation.
>
> To support compaction we wish to use the existing infrastructure.
> However, we need to make slight adaptions for it. We add a new list
> interface to balloon-compaction, which is more generic and efficient,
> since it does not require as many IRQ save/restore operations. We leave
> the old interface that is used by the virtio balloon.
>
> Big parts of this patch-set are cleanup and documentation. Patches 1-13
> simplify the balloon code, document its behavior and allow the balloon
> code to run concurrently. The support for concurrency is required for
> compaction and the shrinker interface.
>
> For documentation we use the kernel-doc format. We are aware that the
> balloon interface is not public, but following the kernel-doc format may
> be useful one day.
I applied the first 16 patches. Please fix up 17 and resend.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 0/3] virtio: add vmap support for prime objects
From: Gerd Hoffmann @ 2018-09-26 6:07 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: David Airlie, dri-devel, virtualization
In-Reply-To: <20180925161606.17980-1-ezequiel@collabora.com>
Hi,
> Having the support for vmapping dmabuf's is required to share
> dmabufs to drivers that want CPU access. This is the case of
> a vivid to virtio-gpu pipeline, where the virtio-gpu driver
> exports dmabufs to the video4linux vivid driver.
>
> The first patch adds virtio_gpu_object_kunmap() and calls
> it from the TTM object destroy path. This function will be
> used to unmap prime objects.
>
> The second patch reworks virtio_gpu_object_kmap(), so it's
> balanced with virtio_gpu_object_kunmap.
>
> Finally, the third patch uses virtio_gpu_object_k{map,unmap}
> to support prime v{map,unmap}.
>
> Please review!
Pushed to drm-misc-next.
thanks,
Gerd
^ permalink raw reply
* [PATCH 0/4] Improve virtio ID allocation
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: linux-kernel, dri-devel, Matthew Wilcox, virtualization
I noticed you were using IDRs where you could be using the more efficient
IDAs, then while fixing that I noticed the lack of error handling,
and I decided to follow that up with an efficiency improvement.
There's probably a v2 of this to follow because I couldn't figure
out how to properly handle one of the error cases ... see the comment
embedded in one of the patches.
Matthew Wilcox (4):
drm/virtio: Replace IDRs with IDAs
drm/virtio: Handle context ID allocation errors
drm/virtio: Handle object ID allocation errors
drm/virtio: Use IDAs more efficiently
drivers/gpu/drm/virtio/virtgpu_drv.h | 9 ++---
drivers/gpu/drm/virtio/virtgpu_fb.c | 10 ++++--
drivers/gpu/drm/virtio/virtgpu_gem.c | 10 ++++--
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 ++-
drivers/gpu/drm/virtio/virtgpu_kms.c | 46 +++++++++-----------------
drivers/gpu/drm/virtio/virtgpu_vq.c | 19 ++++-------
6 files changed, 44 insertions(+), 55 deletions(-)
--
2.19.0
^ permalink raw reply
* [PATCH 1/4] drm/virtio: Replace IDRs with IDAs
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: linux-kernel, dri-devel, Matthew Wilcox, virtualization
In-Reply-To: <20180926160031.15721-1-willy@infradead.org>
These IDRs were only being used to allocate unique numbers, not to look
up pointers, so they can use the more space-efficient IDA instead.
Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 6 ++----
drivers/gpu/drm/virtio/virtgpu_kms.c | 18 ++++--------------
drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++----------
3 files changed, 8 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 65605e207bbe..c4468a4e454e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -180,8 +180,7 @@ struct virtio_gpu_device {
struct kmem_cache *vbufs;
bool vqs_ready;
- struct idr resource_idr;
- spinlock_t resource_idr_lock;
+ struct ida resource_ida;
wait_queue_head_t resp_wq;
/* current display info */
@@ -190,8 +189,7 @@ struct virtio_gpu_device {
struct virtio_gpu_fence_driver fence_drv;
- struct idr ctx_id_idr;
- spinlock_t ctx_id_idr_lock;
+ struct ida ctx_id_ida;
bool has_virgl_3d;
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 65060c08522d..e2604fe1b4ae 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -55,21 +55,13 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
static void virtio_gpu_ctx_id_get(struct virtio_gpu_device *vgdev,
uint32_t *resid)
{
- int handle;
-
- idr_preload(GFP_KERNEL);
- spin_lock(&vgdev->ctx_id_idr_lock);
- handle = idr_alloc(&vgdev->ctx_id_idr, NULL, 1, 0, 0);
- spin_unlock(&vgdev->ctx_id_idr_lock);
- idr_preload_end();
+ int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
*resid = handle;
}
static void virtio_gpu_ctx_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
{
- spin_lock(&vgdev->ctx_id_idr_lock);
- idr_remove(&vgdev->ctx_id_idr, id);
- spin_unlock(&vgdev->ctx_id_idr_lock);
+ ida_free(&vgdev->ctx_id_ida, id);
}
static void virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
@@ -151,10 +143,8 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
vgdev->dev = dev->dev;
spin_lock_init(&vgdev->display_info_lock);
- spin_lock_init(&vgdev->ctx_id_idr_lock);
- idr_init(&vgdev->ctx_id_idr);
- spin_lock_init(&vgdev->resource_idr_lock);
- idr_init(&vgdev->resource_idr);
+ ida_init(&vgdev->ctx_id_ida);
+ ida_init(&vgdev->resource_ida);
init_waitqueue_head(&vgdev->resp_wq);
virtio_gpu_init_vq(&vgdev->ctrlq, virtio_gpu_dequeue_ctrl_func);
virtio_gpu_init_vq(&vgdev->cursorq, virtio_gpu_dequeue_cursor_func);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 020070d483d3..58be09d2eed6 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -41,21 +41,13 @@
void virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
uint32_t *resid)
{
- int handle;
-
- idr_preload(GFP_KERNEL);
- spin_lock(&vgdev->resource_idr_lock);
- handle = idr_alloc(&vgdev->resource_idr, NULL, 1, 0, GFP_NOWAIT);
- spin_unlock(&vgdev->resource_idr_lock);
- idr_preload_end();
+ int handle = ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
*resid = handle;
}
void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
{
- spin_lock(&vgdev->resource_idr_lock);
- idr_remove(&vgdev->resource_idr, id);
- spin_unlock(&vgdev->resource_idr_lock);
+ ida_free(&vgdev->resource_ida, id);
}
void virtio_gpu_ctrl_ack(struct virtqueue *vq)
--
2.19.0
^ permalink raw reply related
* [PATCH 2/4] drm/virtio: Handle context ID allocation errors
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: linux-kernel, dri-devel, Matthew Wilcox, virtualization
In-Reply-To: <20180926160031.15721-1-willy@infradead.org>
It is possible to run out of memory while allocating IDs. The current
code would create a context with an invalid ID; change it to return
-ENOMEM to userspace.
Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 29 +++++++++++-----------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index e2604fe1b4ae..bf609dcae224 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -52,31 +52,22 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
events_clear, &events_clear);
}
-static void virtio_gpu_ctx_id_get(struct virtio_gpu_device *vgdev,
- uint32_t *resid)
+static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
+ uint32_t nlen, const char *name)
{
int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
- *resid = handle;
-}
-static void virtio_gpu_ctx_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
-{
- ida_free(&vgdev->ctx_id_ida, id);
-}
-
-static void virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
- uint32_t nlen, const char *name,
- uint32_t *ctx_id)
-{
- virtio_gpu_ctx_id_get(vgdev, ctx_id);
- virtio_gpu_cmd_context_create(vgdev, *ctx_id, nlen, name);
+ if (handle < 0)
+ return handle;
+ virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
+ return handle;
}
static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev,
uint32_t ctx_id)
{
virtio_gpu_cmd_context_destroy(vgdev, ctx_id);
- virtio_gpu_ctx_id_put(vgdev, ctx_id);
+ ida_free(&vgdev->ctx_id_ida, ctx_id);
}
static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq,
@@ -261,7 +252,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
{
struct virtio_gpu_device *vgdev = dev->dev_private;
struct virtio_gpu_fpriv *vfpriv;
- uint32_t id;
+ int id;
char dbgname[TASK_COMM_LEN];
/* can't create contexts without 3d renderer */
@@ -274,7 +265,9 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
return -ENOMEM;
get_task_comm(dbgname, current);
- virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname, &id);
+ id = virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname);
+ if (id < 0)
+ return id;
vfpriv->ctx_id = id;
file->driver_priv = vfpriv;
--
2.19.0
^ permalink raw reply related
* [PATCH 3/4] drm/virtio: Handle object ID allocation errors
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: linux-kernel, dri-devel, Matthew Wilcox, virtualization
In-Reply-To: <20180926160031.15721-1-willy@infradead.org>
It is possible to run out of memory while allocating IDs. The current
code would create an object with an invalid ID; change it to return
-ENOMEM to the caller.
Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +--
drivers/gpu/drm/virtio/virtgpu_fb.c | 10 ++++++++--
drivers/gpu/drm/virtio/virtgpu_gem.c | 10 ++++++++--
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 ++++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 6 ++----
5 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index c4468a4e454e..0a3392f2cda3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -247,8 +247,7 @@ int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *qfb,
/* virtio vg */
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev);
-void virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
- uint32_t *resid);
+int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev);
void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id);
void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
uint32_t resource_id,
diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c b/drivers/gpu/drm/virtio/virtgpu_fb.c
index a121b1c79522..74d815483487 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fb.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fb.c
@@ -244,14 +244,17 @@ static int virtio_gpufb_create(struct drm_fb_helper *helper,
if (IS_ERR(obj))
return PTR_ERR(obj);
- virtio_gpu_resource_id_get(vgdev, &resid);
+ ret = virtio_gpu_resource_id_get(vgdev);
+ if (ret < 0)
+ goto err_obj_vmap;
+ resid = ret;
virtio_gpu_cmd_create_resource(vgdev, resid, format,
mode_cmd.width, mode_cmd.height);
ret = virtio_gpu_vmap_fb(vgdev, obj);
if (ret) {
DRM_ERROR("failed to vmap fb %d\n", ret);
- goto err_obj_vmap;
+ goto err_obj_id;
}
/* attach the object to the resource */
@@ -293,8 +296,11 @@ static int virtio_gpufb_create(struct drm_fb_helper *helper,
err_fb_alloc:
virtio_gpu_cmd_resource_inval_backing(vgdev, resid);
err_obj_attach:
+err_obj_id:
+ virtio_gpu_resource_id_put(vgdev, resid);
err_obj_vmap:
virtio_gpu_gem_free_object(&obj->gem_base);
+
return ret;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 0f2768eacaee..9e3af1ec26db 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -100,7 +100,10 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
goto fail;
format = virtio_gpu_translate_format(DRM_FORMAT_XRGB8888);
- virtio_gpu_resource_id_get(vgdev, &resid);
+ ret = virtio_gpu_resource_id_get(vgdev);
+ if (ret < 0)
+ goto fail;
+ resid = ret;
virtio_gpu_cmd_create_resource(vgdev, resid, format,
args->width, args->height);
@@ -108,13 +111,16 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
obj = gem_to_virtio_gpu_obj(gobj);
ret = virtio_gpu_object_attach(vgdev, obj, resid, NULL);
if (ret)
- goto fail;
+ goto fail_id;
obj->dumb = true;
args->pitch = pitch;
return ret;
+fail_id:
+ virtio_gpu_resource_id_put(vgdev, resid);
fail:
+ /* Shouldn't we undo virtio_gpu_gem_create()? */
return ret;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 7bdf6f0e58a5..eec9f09f01f0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -244,7 +244,10 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
INIT_LIST_HEAD(&validate_list);
memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer));
- virtio_gpu_resource_id_get(vgdev, &res_id);
+ ret = virtio_gpu_resource_id_get(vgdev);
+ if (ret < 0)
+ return ret;
+ res_id = ret;
size = rc->size;
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 58be09d2eed6..387951c971d4 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -38,11 +38,9 @@
+ MAX_INLINE_CMD_SIZE \
+ MAX_INLINE_RESP_SIZE)
-void virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
- uint32_t *resid)
+int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev)
{
- int handle = ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
- *resid = handle;
+ return ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
}
void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
--
2.19.0
^ permalink raw reply related
* [PATCH 4/4] drm/virtio: Use IDAs more efficiently
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: linux-kernel, dri-devel, Matthew Wilcox, virtualization
In-Reply-To: <20180926160031.15721-1-willy@infradead.org>
0-based IDAs are more efficient than any other base. Convert the
1-based IDAs to be 0-based.
Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 3 ++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 7 +++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index bf609dcae224..b576c9ef6323 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -59,6 +59,7 @@ static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
if (handle < 0)
return handle;
+ handle++;
virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
return handle;
}
@@ -67,7 +68,7 @@ static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev,
uint32_t ctx_id)
{
virtio_gpu_cmd_context_destroy(vgdev, ctx_id);
- ida_free(&vgdev->ctx_id_ida, ctx_id);
+ ida_free(&vgdev->ctx_id_ida, ctx_id - 1);
}
static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq,
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 387951c971d4..81297fe0147d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -40,12 +40,15 @@
int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev)
{
- return ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
+ int handle = ida_alloc(&vgdev->resource_ida, GFP_KERNEL);
+ if (handle < 0)
+ return handle;
+ return handle + 1;
}
void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
{
- ida_free(&vgdev->resource_ida, id);
+ ida_free(&vgdev->resource_ida, id - 1);
}
void virtio_gpu_ctrl_ack(struct virtqueue *vq)
--
2.19.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox