* Re: [PATCH v3] tools/virtio/ringtest: fix run-on-all.sh to work without /dev/cpu
From: Mike Rapoport @ 2016-05-24 12:41 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, virtualization
In-Reply-To: <1462356950-3278-1-git-send-email-rppt@linux.vnet.ibm.com>
Michael,
Any updates on this?
On Wed, May 04, 2016 at 01:15:50PM +0300, Mike Rapoport wrote:
> /dev/cpu is only available on x86 with certain modules (e.g. msr) enabled.
> Using lscpu to get processors count is more portable.
>
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
> v3: simplify by using lscpu -p=cpu
> v2: use lspcu instead of /proc/cpuinfo as per Cornelia's suggestion
>
> tools/virtio/ringtest/run-on-all.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/virtio/ringtest/run-on-all.sh b/tools/virtio/ringtest/run-on-all.sh
> index 52b0f71..2e69ca8 100755
> --- a/tools/virtio/ringtest/run-on-all.sh
> +++ b/tools/virtio/ringtest/run-on-all.sh
> @@ -3,10 +3,10 @@
> #use last CPU for host. Why not the first?
> #many devices tend to use cpu0 by default so
> #it tends to be busier
> -HOST_AFFINITY=$(cd /dev/cpu; ls|grep -v '[a-z]'|sort -n|tail -1)
> +HOST_AFFINITY=$(lscpu -p=cpu | tail -1)
>
> #run command on all cpus
> -for cpu in $(cd /dev/cpu; ls|grep -v '[a-z]'|sort -n);
> +for cpu in $(seq 0 $HOST_AFFINITY)
> do
> #Don't run guest and host on same CPU
> #It actually works ok if using signalling
> --
> 1.9.1
>
^ permalink raw reply
* RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Li, Liang Z @ 2016-05-24 14:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <20160524111135.GA7392@redhat.com>
> > > > > This can be pre-initialized, correct?
> > > >
> > > > pre-initialized? I am not quite understand your mean.
> > >
> > > I think you can maintain sg as part of device state and init sg with the
> bitmap.
> > >
> >
> > I got it.
> >
> > > > > This is grossly inefficient if you only requested a single page.
> > > > > And it's also allocating memory very aggressively without ever
> > > > > telling the host what is going on.
> > > >
> > > > If only requested a single page, there is no need to send the
> > > > entire page bitmap, This RFC patch has already considered about this.
> > >
> > > where's that addressed in code?
> > >
> >
> > By record the start_pfn and end_pfn.
> >
> > The start_pfn & end_pfn will be updated in set_page_bitmap() and will
> > be used in the function tell_host():
> >
> > ----------------------------------------------------------------------
> > -----------
> > +static void set_page_bitmap(struct virtio_balloon *vb, struct page
> > +*page) {
> > + unsigned int i;
> > + unsigned long *bitmap = vb->page_bitmap;
> > + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> > +
> > + for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
> > + set_bit(balloon_pfn + i, bitmap);
>
> BTW, there's a page size value in header so there is no longer need to set
> multiple bits per page.
Yes, you are right.
>
> > + if (balloon_pfn < vb->start_pfn)
> > + vb->start_pfn = balloon_pfn;
> > + if (balloon_pfn > vb->end_pfn)
> > + vb->end_pfn = balloon_pfn;
> > +}
>
> Sounds good, but you also need to limit by allocated bitmap size.
Why should we limit the page bitmap size? Is it no good to send a large page bitmap?
or to save the memory used for page bitmap? Or some other reason?
>
> >
> > + unsigned long start_pfn, end_pfn, flags = 0, bmap_len;
> > + struct scatterlist sg[5];
> > +
> > + start_pfn = rounddown(vb->start_pfn, BITS_PER_LONG);
> > + end_pfn = roundup(vb->end_pfn, BITS_PER_LONG);
> > + bmap_len = (end_pfn - start_pfn) / BITS_PER_LONG *
> sizeof(long);
> > +
> > + sg_init_table(sg, 5);
> > + sg_set_buf(&sg[0], &flags, sizeof(flags));
> > + sg_set_buf(&sg[1], &start_pfn, sizeof(start_pfn));
> > + sg_set_buf(&sg[2], &page_shift, sizeof(page_shift));
> > + sg_set_buf(&sg[3], &bmap_len, sizeof(bmap_len));
> > + sg_set_buf(&sg[4], vb->page_bitmap +
> > + (start_pfn / BITS_PER_LONG), bmap_len);
>
> Looks wrong. start_pfn should start at offset 0 I think ...
I don't know what is wrong here, could you tell me why?
Thanks!
Liang
^ permalink raw reply
* Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Michael S. Tsirkin @ 2016-05-24 15:12 UTC (permalink / raw)
To: Li, Liang Z
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E041A5F5C@shsmsx102.ccr.corp.intel.com>
On Tue, May 24, 2016 at 02:36:08PM +0000, Li, Liang Z wrote:
> > > > > > This can be pre-initialized, correct?
> > > > >
> > > > > pre-initialized? I am not quite understand your mean.
> > > >
> > > > I think you can maintain sg as part of device state and init sg with the
> > bitmap.
> > > >
> > >
> > > I got it.
> > >
> > > > > > This is grossly inefficient if you only requested a single page.
> > > > > > And it's also allocating memory very aggressively without ever
> > > > > > telling the host what is going on.
> > > > >
> > > > > If only requested a single page, there is no need to send the
> > > > > entire page bitmap, This RFC patch has already considered about this.
> > > >
> > > > where's that addressed in code?
> > > >
> > >
> > > By record the start_pfn and end_pfn.
> > >
> > > The start_pfn & end_pfn will be updated in set_page_bitmap() and will
> > > be used in the function tell_host():
> > >
> > > ----------------------------------------------------------------------
> > > -----------
> > > +static void set_page_bitmap(struct virtio_balloon *vb, struct page
> > > +*page) {
> > > + unsigned int i;
> > > + unsigned long *bitmap = vb->page_bitmap;
> > > + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> > > +
> > > + for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
> > > + set_bit(balloon_pfn + i, bitmap);
> >
> > BTW, there's a page size value in header so there is no longer need to set
> > multiple bits per page.
>
> Yes, you are right.
>
> >
> > > + if (balloon_pfn < vb->start_pfn)
> > > + vb->start_pfn = balloon_pfn;
> > > + if (balloon_pfn > vb->end_pfn)
> > > + vb->end_pfn = balloon_pfn;
> > > +}
> >
> > Sounds good, but you also need to limit by allocated bitmap size.
>
> Why should we limit the page bitmap size? Is it no good to send a large page bitmap?
> or to save the memory used for page bitmap? Or some other reason?
To save memory. First allocating a large bitmap can fail, second this is
pinned memory that is wasted - it's unused most of the time while guest
is running.
> >
> > >
> > > + unsigned long start_pfn, end_pfn, flags = 0, bmap_len;
> > > + struct scatterlist sg[5];
> > > +
> > > + start_pfn = rounddown(vb->start_pfn, BITS_PER_LONG);
> > > + end_pfn = roundup(vb->end_pfn, BITS_PER_LONG);
> > > + bmap_len = (end_pfn - start_pfn) / BITS_PER_LONG *
> > sizeof(long);
> > > +
> > > + sg_init_table(sg, 5);
> > > + sg_set_buf(&sg[0], &flags, sizeof(flags));
> > > + sg_set_buf(&sg[1], &start_pfn, sizeof(start_pfn));
> > > + sg_set_buf(&sg[2], &page_shift, sizeof(page_shift));
> > > + sg_set_buf(&sg[3], &bmap_len, sizeof(bmap_len));
> > > + sg_set_buf(&sg[4], vb->page_bitmap +
> > > + (start_pfn / BITS_PER_LONG), bmap_len);
> >
> > Looks wrong. start_pfn should start at offset 0 I think ...
>
> I don't know what is wrong here, could you tell me why?
>
> Thanks!
>
> Liang
start_pfn should mean "bit 0 in bitmap refers to pfn X".
So it does not make sense to also add it as offset within bitmap.
--
MST
^ permalink raw reply
* RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Li, Liang Z @ 2016-05-25 0:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <20160524181054-mutt-send-email-mst@redhat.com>
> On Tue, May 24, 2016 at 02:36:08PM +0000, Li, Liang Z wrote:
> > > > > > > This can be pre-initialized, correct?
> > > > > >
> > > > > > pre-initialized? I am not quite understand your mean.
> > > > >
> > > > > I think you can maintain sg as part of device state and init sg
> > > > > with the
> > > bitmap.
> > > > >
> > > >
> > > > I got it.
> > > >
> > > > > > > This is grossly inefficient if you only requested a single page.
> > > > > > > And it's also allocating memory very aggressively without
> > > > > > > ever telling the host what is going on.
> > > > > >
> > > > > > If only requested a single page, there is no need to send the
> > > > > > entire page bitmap, This RFC patch has already considered about
> this.
> > > > >
> > > > > where's that addressed in code?
> > > > >
> > > >
> > > > By record the start_pfn and end_pfn.
> > > >
> > > > The start_pfn & end_pfn will be updated in set_page_bitmap() and
> > > > will be used in the function tell_host():
> > > >
> > > > ------------------------------------------------------------------
> > > > ----
> > > > -----------
> > > > +static void set_page_bitmap(struct virtio_balloon *vb, struct
> > > > +page
> > > > +*page) {
> > > > + unsigned int i;
> > > > + unsigned long *bitmap = vb->page_bitmap;
> > > > + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> > > > +
> > > > + for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
> > > > + set_bit(balloon_pfn + i, bitmap);
> > >
> > > BTW, there's a page size value in header so there is no longer need
> > > to set multiple bits per page.
> >
> > Yes, you are right.
> >
> > >
> > > > + if (balloon_pfn < vb->start_pfn)
> > > > + vb->start_pfn = balloon_pfn;
> > > > + if (balloon_pfn > vb->end_pfn)
> > > > + vb->end_pfn = balloon_pfn;
> > > > +}
> > >
> > > Sounds good, but you also need to limit by allocated bitmap size.
> >
> > Why should we limit the page bitmap size? Is it no good to send a large
> page bitmap?
> > or to save the memory used for page bitmap? Or some other reason?
>
> To save memory. First allocating a large bitmap can fail, second this is pinned
> memory that is wasted - it's unused most of the time while guest is running.
>
> > >
> > > >
> > > > + unsigned long start_pfn, end_pfn, flags = 0, bmap_len;
> > > > + struct scatterlist sg[5];
> > > > +
> > > > + start_pfn = rounddown(vb->start_pfn, BITS_PER_LONG);
> > > > + end_pfn = roundup(vb->end_pfn, BITS_PER_LONG);
> > > > + bmap_len = (end_pfn - start_pfn) / BITS_PER_LONG *
> > > sizeof(long);
> > > > +
> > > > + sg_init_table(sg, 5);
> > > > + sg_set_buf(&sg[0], &flags, sizeof(flags));
> > > > + sg_set_buf(&sg[1], &start_pfn, sizeof(start_pfn));
> > > > + sg_set_buf(&sg[2], &page_shift, sizeof(page_shift));
> > > > + sg_set_buf(&sg[3], &bmap_len, sizeof(bmap_len));
> > > > + sg_set_buf(&sg[4], vb->page_bitmap +
> > > > + (start_pfn / BITS_PER_LONG), bmap_len);
> > >
> > > Looks wrong. start_pfn should start at offset 0 I think ...
> >
> > I don't know what is wrong here, could you tell me why?
> >
> > Thanks!
> >
> > Liang
>
> start_pfn should mean "bit 0 in bitmap refers to pfn X".
> So it does not make sense to also add it as offset within bitmap.
>
> --
> MST
^ permalink raw reply
* RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Li, Liang Z @ 2016-05-25 1:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <20160524181054-mutt-send-email-mst@redhat.com>
> > > > > > > This is grossly inefficient if you only requested a single page.
> > > > > > > And it's also allocating memory very aggressively without
> > > > > > > ever telling the host what is going on.
> > > > > >
> > > > > > If only requested a single page, there is no need to send the
> > > > > > entire page bitmap, This RFC patch has already considered about
> this.
> > > > >
> > > > > where's that addressed in code?
> > > > >
> > > >
> > > > By record the start_pfn and end_pfn.
> > > >
> > > > The start_pfn & end_pfn will be updated in set_page_bitmap() and
> > > > will be used in the function tell_host():
> > > >
> > > > ------------------------------------------------------------------
> > > > ----
> > > > -----------
> > > > +static void set_page_bitmap(struct virtio_balloon *vb, struct
> > > > +page
> > > > +*page) {
> > > > + unsigned int i;
> > > > + unsigned long *bitmap = vb->page_bitmap;
> > > > + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> > > > +
> > > > + for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
> > > > + set_bit(balloon_pfn + i, bitmap);
> > >
> > > BTW, there's a page size value in header so there is no longer need
> > > to set multiple bits per page.
> >
> > Yes, you are right.
> >
> > >
> > > > + if (balloon_pfn < vb->start_pfn)
> > > > + vb->start_pfn = balloon_pfn;
> > > > + if (balloon_pfn > vb->end_pfn)
> > > > + vb->end_pfn = balloon_pfn;
> > > > +}
> > >
> > > Sounds good, but you also need to limit by allocated bitmap size.
> >
> > Why should we limit the page bitmap size? Is it no good to send a large
> page bitmap?
> > or to save the memory used for page bitmap? Or some other reason?
>
> To save memory. First allocating a large bitmap can fail, second this is pinned
> memory that is wasted - it's unused most of the time while guest is running.
Make sense.
>
> > >
> > > >
> > > > + unsigned long start_pfn, end_pfn, flags = 0, bmap_len;
> > > > + struct scatterlist sg[5];
> > > > +
> > > > + start_pfn = rounddown(vb->start_pfn, BITS_PER_LONG);
> > > > + end_pfn = roundup(vb->end_pfn, BITS_PER_LONG);
> > > > + bmap_len = (end_pfn - start_pfn) / BITS_PER_LONG *
> > > sizeof(long);
> > > > +
> > > > + sg_init_table(sg, 5);
> > > > + sg_set_buf(&sg[0], &flags, sizeof(flags));
> > > > + sg_set_buf(&sg[1], &start_pfn, sizeof(start_pfn));
> > > > + sg_set_buf(&sg[2], &page_shift, sizeof(page_shift));
> > > > + sg_set_buf(&sg[3], &bmap_len, sizeof(bmap_len));
> > > > + sg_set_buf(&sg[4], vb->page_bitmap +
> > > > + (start_pfn / BITS_PER_LONG), bmap_len);
> > >
> > > Looks wrong. start_pfn should start at offset 0 I think ...
> >
> > I don't know what is wrong here, could you tell me why?
> >
> > Thanks!
> >
> > Liang
>
> start_pfn should mean "bit 0 in bitmap refers to pfn X".
> So it does not make sense to also add it as offset within bitmap.
I got it, but in my code, it's correct, because the page_bitmap is
range from pfn 0 to max_pfn.
It should be changed if we are going to use a small page bitmap.
Thanks!
Liang
>
> --
> MST
^ permalink raw reply
* [PATCH v3 0/6] powerpc use pv-qpsinlock as the default spinlock implemention
From: Pan Xinhui @ 2016-05-25 8:18 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, virtualization
Cc: jeremy, Pan Xinhui, peterz, benh, chrisw, mingo, paulus, mpe,
akataria, paulmck
change from v2:
__spin_yeild_cpu() will yield slices to lpar if target cpu is running.
remove unnecessary rmb() in __spin_yield/wake_cpu.
__pv_wait() will check the *ptr == val.
some commit message change
change fome v1:
separate into 6 pathes from one patch
some minor code changes.
I do several tests on pseries IBM,8408-E8E with 32cpus, 64GB memory.
benchmark test results are below.
2 perf tests:
perf bench futex hash
perf bench futex lock-pi
_____test________________spinlcok______________pv-qspinlcok_____
|futex hash | 556370 ops | 629634 ops |
|futex lock-pi | 362 ops | 367 ops |
scheduler test:
Test how many loops of schedule() can finish within 10 seconds on all cpus.
_____test________________spinlcok______________pv-qspinlcok_____
|schedule() loops| 322811921 | 311449290 |
kernel compiling test:
build a linux kernel image to see how long it took
_____test________________spinlcok______________pv-qspinlcok_____
| compiling takes| 22m | 22m |
Pan Xinhui (6):
qspinlock: powerpc support qspinlock
powerpc: pseries/Kconfig: Add qspinlock build config
powerpc: lib/locks.c: Add cpu yield/wake helper function
pv-qspinlock: powerpc support pv-qspinlock
pv-qspinlock: use cmpxchg_release in __pv_queued_spin_unlock
powerpc: pseries: Add pv-qspinlock build config/make
arch/powerpc/include/asm/qspinlock.h | 39 +++++++++++++++++++
arch/powerpc/include/asm/qspinlock_paravirt.h | 38 ++++++++++++++++++
.../powerpc/include/asm/qspinlock_paravirt_types.h | 13 +++++++
arch/powerpc/include/asm/spinlock.h | 31 +++++++++------
arch/powerpc/include/asm/spinlock_types.h | 4 ++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/paravirt.c | 45 ++++++++++++++++++++++
arch/powerpc/lib/locks.c | 37 ++++++++++++++++++
arch/powerpc/platforms/pseries/Kconfig | 9 +++++
arch/powerpc/platforms/pseries/setup.c | 5 +++
kernel/locking/qspinlock_paravirt.h | 2 +-
11 files changed, 211 insertions(+), 13 deletions(-)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt_types.h
create mode 100644 arch/powerpc/kernel/paravirt.c
--
1.9.1
^ permalink raw reply
* [PATCH v3 1/6] qspinlock: powerpc support qspinlock
From: Pan Xinhui @ 2016-05-25 8:18 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, virtualization
Cc: jeremy, Pan Xinhui, peterz, benh, chrisw, mingo, paulus, mpe,
akataria, paulmck
In-Reply-To: <1464164289-6124-1-git-send-email-xinhui.pan@linux.vnet.ibm.com>
Base code to enable qspinlock on powerpc. this patch add some #ifdef
here and there. Although there is no paravirt related code, we can
successfully build a qspinlock kernel after apply this patch.
Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/qspinlock.h | 22 ++++++++++++++++++++++
arch/powerpc/include/asm/spinlock.h | 27 +++++++++++++++------------
arch/powerpc/include/asm/spinlock_types.h | 4 ++++
arch/powerpc/lib/locks.c | 4 ++++
4 files changed, 45 insertions(+), 12 deletions(-)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
new file mode 100644
index 0000000..5883954
--- /dev/null
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -0,0 +1,22 @@
+#ifndef _ASM_POWERPC_QSPINLOCK_H
+#define _ASM_POWERPC_QSPINLOCK_H
+
+#include <asm-generic/qspinlock_types.h>
+
+#define SPIN_THRESHOLD (1 << 15)
+#define queued_spin_unlock queued_spin_unlock
+
+static inline void native_queued_spin_unlock(struct qspinlock *lock)
+{
+ /* no load/store can be across the unlock()*/
+ smp_store_release((u8 *)lock, 0);
+}
+
+static inline void queued_spin_unlock(struct qspinlock *lock)
+{
+ native_queued_spin_unlock(lock);
+}
+
+#include <asm-generic/qspinlock.h>
+
+#endif /* _ASM_POWERPC_QSPINLOCK_H */
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 523673d..4359ee6 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -52,6 +52,20 @@
#define SYNC_IO
#endif
+#if defined(CONFIG_PPC_SPLPAR)
+/* We only yield to the hypervisor if we are in shared processor mode */
+#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))
+extern void __spin_yield(arch_spinlock_t *lock);
+extern void __rw_yield(arch_rwlock_t *lock);
+#else /* SPLPAR */
+#define __spin_yield(x) barrier()
+#define __rw_yield(x) barrier()
+#define SHARED_PROCESSOR 0
+#endif
+
+#ifdef CONFIG_QUEUED_SPINLOCKS
+#include <asm/qspinlock.h>
+#else
static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
{
return lock.slock == 0;
@@ -106,18 +120,6 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)
* held. Conveniently, we have a word in the paca that holds this
* value.
*/
-
-#if defined(CONFIG_PPC_SPLPAR)
-/* We only yield to the hypervisor if we are in shared processor mode */
-#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))
-extern void __spin_yield(arch_spinlock_t *lock);
-extern void __rw_yield(arch_rwlock_t *lock);
-#else /* SPLPAR */
-#define __spin_yield(x) barrier()
-#define __rw_yield(x) barrier()
-#define SHARED_PROCESSOR 0
-#endif
-
static inline void arch_spin_lock(arch_spinlock_t *lock)
{
CLEAR_IO_SYNC;
@@ -169,6 +171,7 @@ extern void arch_spin_unlock_wait(arch_spinlock_t *lock);
do { while (arch_spin_is_locked(lock)) cpu_relax(); } while (0)
#endif
+#endif /* !CONFIG_QUEUED_SPINLOCKS */
/*
* Read-write spinlocks, allowing multiple readers
* but only one writer.
diff --git a/arch/powerpc/include/asm/spinlock_types.h b/arch/powerpc/include/asm/spinlock_types.h
index 2351adc..bd7144e 100644
--- a/arch/powerpc/include/asm/spinlock_types.h
+++ b/arch/powerpc/include/asm/spinlock_types.h
@@ -5,11 +5,15 @@
# error "please don't include this file directly"
#endif
+#ifdef CONFIG_QUEUED_SPINLOCKS
+#include <asm-generic/qspinlock_types.h>
+#else
typedef struct {
volatile unsigned int slock;
} arch_spinlock_t;
#define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
+#endif
typedef struct {
volatile signed int lock;
diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
index f7deebd..a9ebd71 100644
--- a/arch/powerpc/lib/locks.c
+++ b/arch/powerpc/lib/locks.c
@@ -23,6 +23,7 @@
#include <asm/hvcall.h>
#include <asm/smp.h>
+#ifndef CONFIG_QUEUED_SPINLOCKS
void __spin_yield(arch_spinlock_t *lock)
{
unsigned int lock_value, holder_cpu, yield_count;
@@ -42,6 +43,7 @@ void __spin_yield(arch_spinlock_t *lock)
get_hard_smp_processor_id(holder_cpu), yield_count);
}
EXPORT_SYMBOL_GPL(__spin_yield);
+#endif
/*
* Waiting for a read lock or a write lock on a rwlock...
@@ -69,6 +71,7 @@ void __rw_yield(arch_rwlock_t *rw)
}
#endif
+#ifndef CONFIG_QUEUED_SPINLOCKS
void arch_spin_unlock_wait(arch_spinlock_t *lock)
{
smp_mb();
@@ -84,3 +87,4 @@ void arch_spin_unlock_wait(arch_spinlock_t *lock)
}
EXPORT_SYMBOL(arch_spin_unlock_wait);
+#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v3 2/6] powerpc: pseries/Kconfig: Add qspinlock build config
From: Pan Xinhui @ 2016-05-25 8:18 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, virtualization
Cc: jeremy, Pan Xinhui, peterz, benh, chrisw, mingo, paulus, mpe,
akataria, paulmck
In-Reply-To: <1464164289-6124-1-git-send-email-xinhui.pan@linux.vnet.ibm.com>
pseries will use qspinlock by default.
Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index bec90fb..f669323 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -21,6 +21,7 @@ config PPC_PSERIES
select HOTPLUG_CPU if SMP
select ARCH_RANDOM
select PPC_DOORBELL
+ select ARCH_USE_QUEUED_SPINLOCKS
default y
config PPC_SPLPAR
--
1.9.1
^ permalink raw reply related
* [PATCH v3 3/6] powerpc: lib/locks.c: Add cpu yield/wake helper function
From: Pan Xinhui @ 2016-05-25 8:18 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, virtualization
Cc: jeremy, Pan Xinhui, peterz, benh, chrisw, mingo, paulus, mpe,
akataria, paulmck
In-Reply-To: <1464164289-6124-1-git-send-email-xinhui.pan@linux.vnet.ibm.com>
pv-qspinlock core has pv_wait/pv_kick which will give a better
performace by yielding and kicking cpu at some cases.
lets support them by adding two corresponding helper functions.
Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/spinlock.h | 4 ++++
arch/powerpc/lib/locks.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 4359ee6..3b65372 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -56,9 +56,13 @@
/* We only yield to the hypervisor if we are in shared processor mode */
#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))
extern void __spin_yield(arch_spinlock_t *lock);
+extern void __spin_yield_cpu(int cpu);
+extern void __spin_wake_cpu(int cpu);
extern void __rw_yield(arch_rwlock_t *lock);
#else /* SPLPAR */
#define __spin_yield(x) barrier()
+#define __spin_yield_cpu(x) barrier()
+#define __spin_wake_cpu(x) barrier()
#define __rw_yield(x) barrier()
#define SHARED_PROCESSOR 0
#endif
diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
index a9ebd71..3a58834 100644
--- a/arch/powerpc/lib/locks.c
+++ b/arch/powerpc/lib/locks.c
@@ -23,6 +23,39 @@
#include <asm/hvcall.h>
#include <asm/smp.h>
+void __spin_yield_cpu(int cpu)
+{
+ unsigned int holder_cpu = cpu, yield_count;
+
+ if (cpu == -1) {
+ plpar_hcall_norets(H_CONFER, -1, 0);
+ return;
+ }
+ BUG_ON(holder_cpu >= nr_cpu_ids);
+ yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+ if ((yield_count & 1) == 0) {
+ /* if target cpu is running, confer slices to lpar*/
+ plpar_hcall_norets(H_CONFER, -1, 0);
+ return;
+ }
+ plpar_hcall_norets(H_CONFER,
+ get_hard_smp_processor_id(holder_cpu), yield_count);
+}
+EXPORT_SYMBOL_GPL(__spin_yield_cpu);
+
+void __spin_wake_cpu(int cpu)
+{
+ unsigned int holder_cpu = cpu, yield_count;
+
+ BUG_ON(holder_cpu >= nr_cpu_ids);
+ yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+ if ((yield_count & 1) == 0)
+ return; /* virtual cpu is currently running */
+ plpar_hcall_norets(H_PROD,
+ get_hard_smp_processor_id(holder_cpu));
+}
+EXPORT_SYMBOL_GPL(__spin_wake_cpu);
+
#ifndef CONFIG_QUEUED_SPINLOCKS
void __spin_yield(arch_spinlock_t *lock)
{
--
1.9.1
^ permalink raw reply related
* [PATCH v3 4/6] pv-qspinlock: powerpc support pv-qspinlock
From: Pan Xinhui @ 2016-05-25 8:18 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, virtualization
Cc: jeremy, Pan Xinhui, peterz, benh, chrisw, mingo, paulus, mpe,
akataria, paulmck
In-Reply-To: <1464164289-6124-1-git-send-email-xinhui.pan@linux.vnet.ibm.com>
As we need let pv-qspinlock-kernel run on all environment which might
have no powervm, we should runtime choose which qspinlock version to
use.
The default pv-qspinlock use native version. pv_lock initialization
should be done in bootstage with irq disabled. And if there is PHYP,
restore pv_lock_ops callbacks to pv version.
Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/qspinlock.h | 17 ++++++++
arch/powerpc/include/asm/qspinlock_paravirt.h | 38 ++++++++++++++++++
.../powerpc/include/asm/qspinlock_paravirt_types.h | 13 +++++++
arch/powerpc/kernel/paravirt.c | 45 ++++++++++++++++++++++
arch/powerpc/platforms/pseries/setup.c | 5 +++
5 files changed, 118 insertions(+)
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt_types.h
create mode 100644 arch/powerpc/kernel/paravirt.c
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index 5883954..4728f6e 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -12,10 +12,27 @@ static inline void native_queued_spin_unlock(struct qspinlock *lock)
smp_store_release((u8 *)lock, 0);
}
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+
+#define __ARCH_NEED_PV_HASH_LOOKUP
+
+#include <asm/qspinlock_paravirt.h>
+
+static inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
+{
+ pv_queued_spin_lock(lock, val);
+}
+
+static inline void queued_spin_unlock(struct qspinlock *lock)
+{
+ pv_queued_spin_unlock(lock);
+}
+#else
static inline void queued_spin_unlock(struct qspinlock *lock)
{
native_queued_spin_unlock(lock);
}
+#endif
#include <asm-generic/qspinlock.h>
diff --git a/arch/powerpc/include/asm/qspinlock_paravirt.h b/arch/powerpc/include/asm/qspinlock_paravirt.h
new file mode 100644
index 0000000..cd17a79
--- /dev/null
+++ b/arch/powerpc/include/asm/qspinlock_paravirt.h
@@ -0,0 +1,38 @@
+#ifndef CONFIG_PARAVIRT_SPINLOCKS
+#error "do not include this file"
+#endif
+
+#ifndef _ASM_QSPINLOCK_PARAVIRT_H
+#define _ASM_QSPINLOCK_PARAVIRT_H
+
+#include <asm/qspinlock_paravirt_types.h>
+
+extern void pv_lock_init(void);
+extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __pv_init_lock_hash(void);
+extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __pv_queued_spin_unlock(struct qspinlock *lock);
+
+static inline void pv_queued_spin_lock(struct qspinlock *lock, u32 val)
+{
+ CLEAR_IO_SYNC;
+ pv_lock_op.lock(lock, val);
+}
+
+static inline void pv_queued_spin_unlock(struct qspinlock *lock)
+{
+ SYNC_IO;
+ pv_lock_op.unlock(lock);
+}
+
+static inline void pv_wait(u8 *ptr, u8 val, int lockcpu)
+{
+ pv_lock_op.wait(ptr, val, lockcpu);
+}
+
+static inline void pv_kick(int cpu)
+{
+ pv_lock_op.kick(cpu);
+}
+
+#endif
diff --git a/arch/powerpc/include/asm/qspinlock_paravirt_types.h b/arch/powerpc/include/asm/qspinlock_paravirt_types.h
new file mode 100644
index 0000000..e1fdeb0
--- /dev/null
+++ b/arch/powerpc/include/asm/qspinlock_paravirt_types.h
@@ -0,0 +1,13 @@
+#ifndef _ASM_QSPINLOCK_PARAVIRT_TYPES_H
+#define _ASM_QSPINLOCK_PARAVIRT_TYPES_H
+
+struct pv_lock_ops {
+ void (*lock)(struct qspinlock *lock, u32 val);
+ void (*unlock)(struct qspinlock *lock);
+ void (*wait)(u8 *ptr, u8 val, int cpu);
+ void (*kick)(int cpu);
+};
+
+extern struct pv_lock_ops pv_lock_op;
+
+#endif
diff --git a/arch/powerpc/kernel/paravirt.c b/arch/powerpc/kernel/paravirt.c
new file mode 100644
index 0000000..4f19f7e
--- /dev/null
+++ b/arch/powerpc/kernel/paravirt.c
@@ -0,0 +1,45 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/spinlock.h>
+
+static void __native_queued_spin_unlock(struct qspinlock *lock)
+{
+ native_queued_spin_unlock(lock);
+}
+
+static void __pv_wait(u8 *ptr, u8 val, int cpu)
+{
+ HMT_low();
+ if (READ_ONCE(*ptr) == val)
+ __spin_yield_cpu(cpu);
+ HMT_medium();
+}
+
+static void __pv_kick(int cpu)
+{
+ __spin_wake_cpu(cpu);
+}
+
+struct pv_lock_ops pv_lock_op = {
+ .lock = native_queued_spin_lock_slowpath,
+ .unlock = __native_queued_spin_unlock,
+ .wait = NULL,
+ .kick = NULL,
+};
+EXPORT_SYMBOL(pv_lock_op);
+
+void __init pv_lock_init(void)
+{
+ if (SHARED_PROCESSOR) {
+ __pv_init_lock_hash();
+ pv_lock_op.lock = __pv_queued_spin_lock_slowpath;
+ pv_lock_op.unlock = __pv_queued_spin_unlock;
+ pv_lock_op.wait = __pv_wait;
+ pv_lock_op.kick = __pv_kick;
+ }
+}
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 6e944fc..c9f056e 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -547,6 +547,11 @@ static void __init pSeries_setup_arch(void)
"%ld\n", rc);
}
}
+
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+ pv_lock_init();
+#endif
+
}
static int __init pSeries_init_panel(void)
--
1.9.1
^ permalink raw reply related
* [PATCH v3 5/6] pv-qspinlock: use cmpxchg_release in __pv_queued_spin_unlock
From: Pan Xinhui @ 2016-05-25 8:18 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, virtualization
Cc: jeremy, Pan Xinhui, peterz, benh, chrisw, mingo, paulus, mpe,
akataria, paulmck
In-Reply-To: <1464164289-6124-1-git-send-email-xinhui.pan@linux.vnet.ibm.com>
cmpxchg_release is light-wight than cmpxchg, we can gain a better
performace then. On some arch like ppc, barrier impact the performace
too much.
Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
---
kernel/locking/qspinlock_paravirt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index a5b1248..2bbffe4 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -614,7 +614,7 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock)
* unhash. Otherwise it would be possible to have multiple @lock
* entries, which would be BAD.
*/
- locked = cmpxchg(&l->locked, _Q_LOCKED_VAL, 0);
+ locked = cmpxchg_release(&l->locked, _Q_LOCKED_VAL, 0);
if (likely(locked == _Q_LOCKED_VAL))
return;
--
1.9.1
^ permalink raw reply related
* [PATCH v3 6/6] powerpc: pseries: Add pv-qspinlock build config/make
From: Pan Xinhui @ 2016-05-25 8:18 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, virtualization
Cc: jeremy, Pan Xinhui, peterz, benh, chrisw, mingo, paulus, mpe,
akataria, paulmck
In-Reply-To: <1464164289-6124-1-git-send-email-xinhui.pan@linux.vnet.ibm.com>
pseries has PowerVM support, the default option is Y.
Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
---
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/platforms/pseries/Kconfig | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 2da380f..ae7c2f1 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_PPC_970_NAP) += idle_power4.o
obj-$(CONFIG_PPC_P7_NAP) += idle_power7.o
procfs-y := proc_powerpc.o
obj-$(CONFIG_PROC_FS) += $(procfs-y)
+obj-$(CONFIG_PARAVIRT_SPINLOCKS) += paravirt.o
rtaspci-$(CONFIG_PPC64)-$(CONFIG_PCI) := rtas_pci.o
obj-$(CONFIG_PPC_RTAS) += rtas.o rtas-rtc.o $(rtaspci-y-y)
obj-$(CONFIG_PPC_RTAS_DAEMON) += rtasd.o
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index f669323..46632e4 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -128,3 +128,11 @@ config HV_PERF_CTRS
systems. 24x7 is available on Power 8 systems.
If unsure, select Y.
+
+config PARAVIRT_SPINLOCKS
+ bool "Paravirtialization support for qspinlock"
+ depends on PPC_SPLPAR && QUEUED_SPINLOCKS
+ default y
+ help
+ If platform supports virtualization, for example PowerVM, this option
+ can let guest have a better performace.
--
1.9.1
^ permalink raw reply related
* Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Michael S. Tsirkin @ 2016-05-25 8:35 UTC (permalink / raw)
To: Li, Liang Z
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E041A6625@shsmsx102.ccr.corp.intel.com>
On Wed, May 25, 2016 at 01:00:27AM +0000, Li, Liang Z wrote:
> It should be changed if we are going to use a small page bitmap.
Exactly.
^ permalink raw reply
* RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Li, Liang Z @ 2016-05-25 8:48 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <20160524130041-mutt-send-email-mst@redhat.com>
> > > Suggestion to address all above comments:
> > > 1. allocate a bunch of pages and link them up,
> > > calculating the min and the max pfn.
> > > if max-min exceeds the allocated bitmap size,
> > > tell host.
> >
> > I am not sure if it works well in some cases, e.g. The allocated pages
> > are across a wide range and the max-min > limit is very frequently to be
> true.
> > Then, there will be many times of virtio transmission and it's bad for
> > performance improvement. Right?
>
> It's a tradeoff for sure. Measure it, see what the overhead is.
>
Hi MST,
I have measured the performance when using a 32K page bitmap, and inflate the balloon to 3GB
of an idle guest with 4GB RAM.
Now:
total inflating time: 338ms
the count of virtio data transmission: 373
the call count of madvise: 865
before:
total inflating time: 175ms
the count of virtio data transmission: 1
the call count of madvise: 42
Maybe the result will be worse if the guest is not idle, or the guest has more RAM.
Do you want more data?
Is it worth to do that?
Liang
> >
> > > 2. limit allocated bitmap size to something reasonable.
> > > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > > out to 1Giga bytes of memory in the balloon.
> >
> > So, even the VM has 1TB of RAM, the page bitmap will take 32MB of
> memory.
> > Maybe it's better to use a big page bitmap the save the pages
> > allocated by balloon, and split the big page bitmap to 32K bytes unit, then
> transfer one unit at a time.
>
> How is this different from what I said?
>
> >
> > Should we use a page bitmap to replace 'vb->pages' ?
> >
> > How about rolling back to use PFNs if the count of requested pages is a
> small number?
> >
> > Liang
>
> That's why we have start pfn. you can use that to pass even a single page
> without a lot of overhead.
>
> > > > --
> > > > 1.9.1
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > > the body of a message to majordomo@vger.kernel.org More majordomo
> > > info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Michael S. Tsirkin @ 2016-05-25 8:57 UTC (permalink / raw)
To: Li, Liang Z
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E041A6C67@shsmsx102.ccr.corp.intel.com>
On Wed, May 25, 2016 at 08:48:17AM +0000, Li, Liang Z wrote:
> > > > Suggestion to address all above comments:
> > > > 1. allocate a bunch of pages and link them up,
> > > > calculating the min and the max pfn.
> > > > if max-min exceeds the allocated bitmap size,
> > > > tell host.
> > >
> > > I am not sure if it works well in some cases, e.g. The allocated pages
> > > are across a wide range and the max-min > limit is very frequently to be
> > true.
> > > Then, there will be many times of virtio transmission and it's bad for
> > > performance improvement. Right?
> >
> > It's a tradeoff for sure. Measure it, see what the overhead is.
> >
>
> Hi MST,
>
> I have measured the performance when using a 32K page bitmap,
Just to make sure. Do you mean a 32Kbyte bitmap?
Covering 1Gbyte of memory?
> and inflate the balloon to 3GB
> of an idle guest with 4GB RAM.
Should take 3 requests then, right?
> Now:
> total inflating time: 338ms
> the count of virtio data transmission: 373
Why was this so high? I would expect 3 transmissions.
> the call count of madvise: 865
>
> before:
> total inflating time: 175ms
> the count of virtio data transmission: 1
> the call count of madvise: 42
>
> Maybe the result will be worse if the guest is not idle, or the guest has more RAM.
> Do you want more data?
>
> Is it worth to do that?
>
> Liang
Either my math is wrong or there's an implementation bug.
> > >
> > > > 2. limit allocated bitmap size to something reasonable.
> > > > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > > > out to 1Giga bytes of memory in the balloon.
> > >
> > > So, even the VM has 1TB of RAM, the page bitmap will take 32MB of
> > memory.
> > > Maybe it's better to use a big page bitmap the save the pages
> > > allocated by balloon, and split the big page bitmap to 32K bytes unit, then
> > transfer one unit at a time.
> >
> > How is this different from what I said?
> >
> > >
> > > Should we use a page bitmap to replace 'vb->pages' ?
> > >
> > > How about rolling back to use PFNs if the count of requested pages is a
> > small number?
> > >
> > > Liang
> >
> > That's why we have start pfn. you can use that to pass even a single page
> > without a lot of overhead.
> >
> > > > > --
> > > > > 1.9.1
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > > > the body of a message to majordomo@vger.kernel.org More majordomo
> > > > info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Li, Liang Z @ 2016-05-25 9:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <20160525115419-mutt-send-email-mst@redhat.com>
> On Wed, May 25, 2016 at 08:48:17AM +0000, Li, Liang Z wrote:
> > > > > Suggestion to address all above comments:
> > > > > 1. allocate a bunch of pages and link them up,
> > > > > calculating the min and the max pfn.
> > > > > if max-min exceeds the allocated bitmap size,
> > > > > tell host.
> > > >
> > > > I am not sure if it works well in some cases, e.g. The allocated
> > > > pages are across a wide range and the max-min > limit is very
> > > > frequently to be
> > > true.
> > > > Then, there will be many times of virtio transmission and it's bad
> > > > for performance improvement. Right?
> > >
> > > It's a tradeoff for sure. Measure it, see what the overhead is.
> > >
> >
> > Hi MST,
> >
> > I have measured the performance when using a 32K page bitmap,
>
> Just to make sure. Do you mean a 32Kbyte bitmap?
> Covering 1Gbyte of memory?
Yes.
>
> > and inflate the balloon to 3GB
> > of an idle guest with 4GB RAM.
>
> Should take 3 requests then, right?
>
No, we can't assign the PFN when allocating page in balloon driver,
So the PFNs of pages allocated may be across a large range, we will
tell the host once the pfn_max -pfn_min >= 0x40000(1GB range),
so the requests count is most likely to be more than 3.
> > Now:
> > total inflating time: 338ms
> > the count of virtio data transmission: 373
>
> Why was this so high? I would expect 3 transmissions.
I follow your suggestion:
------------------------------------------------------------------------------------
Suggestion to address all above comments:
1. allocate a bunch of pages and link them up,
calculating the min and the max pfn.
if max-min exceeds the allocated bitmap size,
tell host.
2. limit allocated bitmap size to something reasonable.
How about 32Kbytes? This is 256kilo bit in the map, which comes
out to 1Giga bytes of memory in the balloon.
-------------------------------------------------------------------------------------
Because the PFNs of the allocated pages are not linear increased, so 3 transmissions
are impossible.
Liang
>
> > the call count of madvise: 865
> >
> > before:
> > total inflating time: 175ms
> > the count of virtio data transmission: 1 the call count of madvise: 42
> >
> > Maybe the result will be worse if the guest is not idle, or the guest has
> more RAM.
> > Do you want more data?
> >
> > Is it worth to do that?
> >
> > Liang
>
> Either my math is wrong or there's an implementation bug.
>
> > > >
> > > > > 2. limit allocated bitmap size to something reasonable.
> > > > > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > > > > out to 1Giga bytes of memory in the balloon.
> > > >
> > > > So, even the VM has 1TB of RAM, the page bitmap will take 32MB of
> > > memory.
> > > > Maybe it's better to use a big page bitmap the save the pages
> > > > allocated by balloon, and split the big page bitmap to 32K bytes
> > > > unit, then
> > > transfer one unit at a time.
> > >
> > > How is this different from what I said?
> > >
> > > >
> > > > Should we use a page bitmap to replace 'vb->pages' ?
> > > >
> > > > How about rolling back to use PFNs if the count of requested pages
> > > > is a
> > > small number?
> > > >
> > > > Liang
> > >
> > > That's why we have start pfn. you can use that to pass even a single
> > > page without a lot of overhead.
> > >
> > > > > > --
> > > > > > 1.9.1
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe kvm"
> > > > > in the body of a message to majordomo@vger.kernel.org More
> > > > > majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the body of
> a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Michael S. Tsirkin @ 2016-05-25 9:40 UTC (permalink / raw)
To: Li, Liang Z
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E041A6CF9@shsmsx102.ccr.corp.intel.com>
On Wed, May 25, 2016 at 09:28:58AM +0000, Li, Liang Z wrote:
> > On Wed, May 25, 2016 at 08:48:17AM +0000, Li, Liang Z wrote:
> > > > > > Suggestion to address all above comments:
> > > > > > 1. allocate a bunch of pages and link them up,
> > > > > > calculating the min and the max pfn.
> > > > > > if max-min exceeds the allocated bitmap size,
> > > > > > tell host.
> > > > >
> > > > > I am not sure if it works well in some cases, e.g. The allocated
> > > > > pages are across a wide range and the max-min > limit is very
> > > > > frequently to be
> > > > true.
> > > > > Then, there will be many times of virtio transmission and it's bad
> > > > > for performance improvement. Right?
> > > >
> > > > It's a tradeoff for sure. Measure it, see what the overhead is.
> > > >
> > >
> > > Hi MST,
> > >
> > > I have measured the performance when using a 32K page bitmap,
> >
> > Just to make sure. Do you mean a 32Kbyte bitmap?
> > Covering 1Gbyte of memory?
> Yes.
>
> >
> > > and inflate the balloon to 3GB
> > > of an idle guest with 4GB RAM.
> >
> > Should take 3 requests then, right?
> >
>
> No, we can't assign the PFN when allocating page in balloon driver,
> So the PFNs of pages allocated may be across a large range, we will
> tell the host once the pfn_max -pfn_min >= 0x40000(1GB range),
> so the requests count is most likely to be more than 3.
>
> > > Now:
> > > total inflating time: 338ms
> > > the count of virtio data transmission: 373
> >
> > Why was this so high? I would expect 3 transmissions.
>
> I follow your suggestion:
> ------------------------------------------------------------------------------------
> Suggestion to address all above comments:
> 1. allocate a bunch of pages and link them up,
> calculating the min and the max pfn.
> if max-min exceeds the allocated bitmap size,
> tell host.
> 2. limit allocated bitmap size to something reasonable.
> How about 32Kbytes? This is 256kilo bit in the map, which comes
> out to 1Giga bytes of memory in the balloon.
> -------------------------------------------------------------------------------------
> Because the PFNs of the allocated pages are not linear increased, so 3 transmissions
> are impossible.
>
>
> Liang
Interesting. How about instead of tell host, we do multiple scans, each
time ignoring pages out of range?
for (pfn = min pfn; pfn < max pfn; pfn += 1G) {
foreach page
if page pfn < pfn || page pfn >= pfn + 1G
continue
set bit
tell host
}
>
> >
> > > the call count of madvise: 865
> > >
> > > before:
> > > total inflating time: 175ms
> > > the count of virtio data transmission: 1 the call count of madvise: 42
> > >
> > > Maybe the result will be worse if the guest is not idle, or the guest has
> > more RAM.
> > > Do you want more data?
> > >
> > > Is it worth to do that?
> > >
> > > Liang
> >
> > Either my math is wrong or there's an implementation bug.
> >
> > > > >
> > > > > > 2. limit allocated bitmap size to something reasonable.
> > > > > > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > > > > > out to 1Giga bytes of memory in the balloon.
> > > > >
> > > > > So, even the VM has 1TB of RAM, the page bitmap will take 32MB of
> > > > memory.
> > > > > Maybe it's better to use a big page bitmap the save the pages
> > > > > allocated by balloon, and split the big page bitmap to 32K bytes
> > > > > unit, then
> > > > transfer one unit at a time.
> > > >
> > > > How is this different from what I said?
> > > >
> > > > >
> > > > > Should we use a page bitmap to replace 'vb->pages' ?
> > > > >
> > > > > How about rolling back to use PFNs if the count of requested pages
> > > > > is a
> > > > small number?
> > > > >
> > > > > Liang
> > > >
> > > > That's why we have start pfn. you can use that to pass even a single
> > > > page without a lot of overhead.
> > > >
> > > > > > > --
> > > > > > > 1.9.1
> > > > > > --
> > > > > > To unsubscribe from this list: send the line "unsubscribe kvm"
> > > > > > in the body of a message to majordomo@vger.kernel.org More
> > > > > > majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in the body of
> > a message to majordomo@vger.kernel.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Li, Liang Z @ 2016-05-25 10:10 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <20160525123546-mutt-send-email-mst@redhat.com>
> > > >
> > > > Hi MST,
> > > >
> > > > I have measured the performance when using a 32K page bitmap,
> > >
> > > Just to make sure. Do you mean a 32Kbyte bitmap?
> > > Covering 1Gbyte of memory?
> > Yes.
> >
> > >
> > > > and inflate the balloon to 3GB
> > > > of an idle guest with 4GB RAM.
> > >
> > > Should take 3 requests then, right?
> > >
> >
> > No, we can't assign the PFN when allocating page in balloon driver,
> > So the PFNs of pages allocated may be across a large range, we will
> > tell the host once the pfn_max -pfn_min >= 0x40000(1GB range), so the
> > requests count is most likely to be more than 3.
> >
> > > > Now:
> > > > total inflating time: 338ms
> > > > the count of virtio data transmission: 373
> > >
> > > Why was this so high? I would expect 3 transmissions.
> >
> > I follow your suggestion:
> > ----------------------------------------------------------------------
> > -------------- Suggestion to address all above comments:
> > 1. allocate a bunch of pages and link them up,
> > calculating the min and the max pfn.
> > if max-min exceeds the allocated bitmap size,
> > tell host.
> > 2. limit allocated bitmap size to something reasonable.
> > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > out to 1Giga bytes of memory in the balloon.
> > ----------------------------------------------------------------------
> > --------------- Because the PFNs of the allocated pages are not linear
> > increased, so 3 transmissions are impossible.
> >
> >
> > Liang
>
> Interesting. How about instead of tell host, we do multiple scans, each time
> ignoring pages out of range?
>
> for (pfn = min pfn; pfn < max pfn; pfn += 1G) {
> foreach page
> if page pfn < pfn || page pfn >= pfn + 1G
> continue
> set bit
> tell host
> }
>
That means we have to allocate/free all the requested pages first, and then tell the host.
It works fine for inflating, but for deflating, because the page has been deleted from the vb-> vb_dev_info->pages,
so, we have to use a struct to save the dequeued pages before calling release_pages_balloon(),
I think a page bitmap is the best struct to save these pages, because it consumes less memory.
And that bitmap should be large enough to save pfn 0 to max_pfn.
If the above is true, then we are back to the square one. we really need a large page bitmap. Right?
Liang
^ permalink raw reply
* Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Michael S. Tsirkin @ 2016-05-25 10:37 UTC (permalink / raw)
To: Li, Liang Z
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E041A6D89@shsmsx102.ccr.corp.intel.com>
On Wed, May 25, 2016 at 10:10:47AM +0000, Li, Liang Z wrote:
> > > > >
> > > > > Hi MST,
> > > > >
> > > > > I have measured the performance when using a 32K page bitmap,
> > > >
> > > > Just to make sure. Do you mean a 32Kbyte bitmap?
> > > > Covering 1Gbyte of memory?
> > > Yes.
> > >
> > > >
> > > > > and inflate the balloon to 3GB
> > > > > of an idle guest with 4GB RAM.
> > > >
> > > > Should take 3 requests then, right?
> > > >
> > >
> > > No, we can't assign the PFN when allocating page in balloon driver,
> > > So the PFNs of pages allocated may be across a large range, we will
> > > tell the host once the pfn_max -pfn_min >= 0x40000(1GB range), so the
> > > requests count is most likely to be more than 3.
> > >
> > > > > Now:
> > > > > total inflating time: 338ms
> > > > > the count of virtio data transmission: 373
> > > >
> > > > Why was this so high? I would expect 3 transmissions.
> > >
> > > I follow your suggestion:
> > > ----------------------------------------------------------------------
> > > -------------- Suggestion to address all above comments:
> > > 1. allocate a bunch of pages and link them up,
> > > calculating the min and the max pfn.
> > > if max-min exceeds the allocated bitmap size,
> > > tell host.
> > > 2. limit allocated bitmap size to something reasonable.
> > > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > > out to 1Giga bytes of memory in the balloon.
> > > ----------------------------------------------------------------------
> > > --------------- Because the PFNs of the allocated pages are not linear
> > > increased, so 3 transmissions are impossible.
> > >
> > >
> > > Liang
> >
> > Interesting. How about instead of tell host, we do multiple scans, each time
> > ignoring pages out of range?
> >
> > for (pfn = min pfn; pfn < max pfn; pfn += 1G) {
> > foreach page
> > if page pfn < pfn || page pfn >= pfn + 1G
> > continue
> > set bit
> > tell host
> > }
> >
>
> That means we have to allocate/free all the requested pages first, and then tell the host.
> It works fine for inflating, but for deflating, because the page has been deleted from the vb-> vb_dev_info->pages,
> so, we have to use a struct to save the dequeued pages before calling release_pages_balloon(),
struct list_head? I think you can just replace set_page_pfns with
list_add(&page->lru, &page_list);
> I think a page bitmap is the best struct to save these pages, because it consumes less memory.
> And that bitmap should be large enough to save pfn 0 to max_pfn.
>
> If the above is true, then we are back to the square one. we really need a large page bitmap. Right?
>
> Liang
These look like implementation issues to me.
I think the below might be helpful (completely untested),
your work can go on top.
--->
virtio-balloon: rework deflate to add page to a tmp list
Will allow faster notifications using a bitmap down the road.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 476c0e3..44050a3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -195,8 +195,9 @@ static void release_pages_balloon(struct virtio_balloon *vb)
static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
{
unsigned num_freed_pages;
- struct page *page;
+ struct page *page, *next;
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
+ LIST_HEAD(pages); /* Pages dequeued for handing to Host */
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
@@ -207,10 +208,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
page = balloon_page_dequeue(vb_dev_info);
if (!page)
break;
- set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+ list_add(&page->lru, &pages);
vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
}
+ list_for_each_entry_safe(page, next, &pages, lru)
+ set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+
num_freed_pages = vb->num_pfns;
/*
* Note that if
--
MST
^ permalink raw reply related
* RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process
From: Li, Liang Z @ 2016-05-25 14:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, amit.shah@redhat.com,
pbonzini@redhat.com, akpm@linux-foundation.org,
dgilbert@redhat.com
In-Reply-To: <20160525131716-mutt-send-email-mst@redhat.com>
> > > Interesting. How about instead of tell host, we do multiple scans,
> > > each time ignoring pages out of range?
> > >
> > > for (pfn = min pfn; pfn < max pfn; pfn += 1G) {
> > > foreach page
> > > if page pfn < pfn || page pfn >= pfn + 1G
> > > continue
> > > set bit
> > > tell host
> > > }
> > >
> >
> > That means we have to allocate/free all the requested pages first, and then
> tell the host.
> > It works fine for inflating, but for deflating, because the page has
> > been deleted from the vb-> vb_dev_info->pages, so, we have to use a
> > struct to save the dequeued pages before calling
> > release_pages_balloon(),
>
> struct list_head? I think you can just replace set_page_pfns with
> list_add(&page->lru, &page_list);
>
>
That's is fine, I will retry and get back to you.
Thanks!
Liang
>
> > I think a page bitmap is the best struct to save these pages, because it
> consumes less memory.
> > And that bitmap should be large enough to save pfn 0 to max_pfn.
> >
> > If the above is true, then we are back to the square one. we really need a
> large page bitmap. Right?
> >
> > Liang
>
> These look like implementation issues to me.
>
> I think the below might be helpful (completely untested), your work can go
> on top.
>
> --->
>
> virtio-balloon: rework deflate to add page to a tmp list
>
> Will allow faster notifications using a bitmap down the road.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 476c0e3..44050a3 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -195,8 +195,9 @@ static void release_pages_balloon(struct
> virtio_balloon *vb) static unsigned leak_balloon(struct virtio_balloon *vb,
> size_t num) {
> unsigned num_freed_pages;
> - struct page *page;
> + struct page *page, *next;
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> + LIST_HEAD(pages); /* Pages dequeued for handing to
> Host */
>
> /* We can only do one array worth at a time. */
> num = min(num, ARRAY_SIZE(vb->pfns));
> @@ -207,10 +208,13 @@ static unsigned leak_balloon(struct virtio_balloon
> *vb, size_t num)
> page = balloon_page_dequeue(vb_dev_info);
> if (!page)
> break;
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + list_add(&page->lru, &pages);
> vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
>
> + list_for_each_entry_safe(page, next, &pages, lru)
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> +
> num_freed_pages = vb->num_pfns;
> /*
> * Note that if
> --
> MST
^ permalink raw reply
* Re: [PATCH v3 7/7] [wip] virtio-gpu: add page flip support
From: Daniel Vetter @ 2016-05-25 16:37 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: open list, dri-devel, open list:VIRTIO GPU DRIVER
In-Reply-To: <1443787104-24243-8-git-send-email-kraxel@redhat.com>
On Fri, Oct 2, 2015 at 1:58 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
So I entirely missed this, but this isn't really how to implement
page_flip for an atomic driver. Working on some stuff and will hack up
a likely totally broken patch, but should be enough as guideline.
-Daniel
> ---
> drivers/gpu/drm/virtio/virtgpu_display.c | 49 ++++++++++++++++++++++++++++++--
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
> index c9c1427..f545913 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -125,6 +125,51 @@ static int virtio_gpu_crtc_cursor_move(struct drm_crtc *crtc,
> return 0;
> }
>
> +static int virtio_gpu_page_flip(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t flags)
> +{
> + struct virtio_gpu_device *vgdev = crtc->dev->dev_private;
> + struct virtio_gpu_output *output =
> + container_of(crtc, struct virtio_gpu_output, crtc);
> + struct drm_plane *plane = crtc->primary;
> + struct virtio_gpu_framebuffer *vgfb;
> + struct virtio_gpu_object *bo;
> + unsigned long irqflags;
> + uint32_t handle;
> +
> + plane->fb = fb;
> + vgfb = to_virtio_gpu_framebuffer(plane->fb);
> + bo = gem_to_virtio_gpu_obj(vgfb->obj);
> + handle = bo->hw_res_handle;
> +
> + DRM_DEBUG("handle 0x%x%s, crtc %dx%d\n", handle,
> + bo->dumb ? ", dumb" : "",
> + crtc->mode.hdisplay, crtc->mode.vdisplay);
> + if (bo->dumb) {
> + virtio_gpu_cmd_transfer_to_host_2d
> + (vgdev, handle, 0,
> + cpu_to_le32(crtc->mode.hdisplay),
> + cpu_to_le32(crtc->mode.vdisplay),
> + 0, 0, NULL);
> + }
> + virtio_gpu_cmd_set_scanout(vgdev, output->index, handle,
> + crtc->mode.hdisplay,
> + crtc->mode.vdisplay, 0, 0);
> + virtio_gpu_cmd_resource_flush(vgdev, handle, 0, 0,
> + crtc->mode.hdisplay,
> + crtc->mode.vdisplay);
> +
> + if (event) {
> + spin_lock_irqsave(&crtc->dev->event_lock, irqflags);
> + drm_send_vblank_event(crtc->dev, -1, event);
> + spin_unlock_irqrestore(&crtc->dev->event_lock, irqflags);
> + }
> +
> + return 0;
> +}
> +
> static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
> .cursor_set2 = virtio_gpu_crtc_cursor_set,
> .cursor_move = virtio_gpu_crtc_cursor_move,
> @@ -132,9 +177,7 @@ static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
> .set_config = drm_atomic_helper_set_config,
> .destroy = drm_crtc_cleanup,
>
> -#if 0 /* not (yet) working without vblank support according to docs */
> - .page_flip = drm_atomic_helper_page_flip,
> -#endif
> + .page_flip = virtio_gpu_page_flip,
> .reset = drm_atomic_helper_crtc_reset,
> .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH] Add virtio gpu driver.
From: Daniel Vetter @ 2016-05-25 16:40 UTC (permalink / raw)
To: Gerd Hoffmann, virtio-dev, Michael S. Tsirkin, open list:ABI/API,
Rusty Russell, open list, open list:DRM DRIVERS,
open list:VIRTIO CORE, NET..., Dave Airlie
In-Reply-To: <20150330144927.GN23521@phenom.ffwll.local>
On Mon, Mar 30, 2015 at 4:49 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Mar 30, 2015 at 02:23:47PM +0200, Gerd Hoffmann wrote:
>> > > Signed-off-by: Dave Airlie <airlied@redhat.com>
>> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> >
>> > Standard request from my side for new drm drivers (especially if they're
>> > this simple): Can you please update the drivers to latest drm internal
>> > interfaces, i.e. using universal planes and atomic?
>>
>> Up'n'running. Incremental patch:
>>
>> https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2d&id=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a
>>
>> v2 coming, but I'll go over the other reviews first.
>
> Looking good. Wrt pageflip the current MO is to handroll it in your
> driver, common approach is to use the msm async commit implementation
> msm_atomic_commit. The issue is simply that right now there's still no
> useable generic vblank callback support (drm_irq.c is a mess) hence why
> the core helpers don't support async flips yet.
I guess I didn't do a good job at looking at your v2: Cursor is still
using legacy interfaces and not a proper plane. Would be awesome if
you could fix that up. Atomic drivers really shouldn't use the legacy
cursor interfaces any more at all.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH] Add virtio gpu driver.
From: Emil Velikov @ 2016-05-25 16:44 UTC (permalink / raw)
To: Daniel Vetter
Cc: virtio-dev, Michael S. Tsirkin, open list:ABI/API, open list,
open list:DRM DRIVERS, open list:VIRTIO CORE, NET..., Dave Airlie
In-Reply-To: <CAKMK7uFUNT193f5djZD6T8cgRxxnZL73yrqbupPHOd2P-+Y=pQ@mail.gmail.com>
On 25 May 2016 at 17:40, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Mar 30, 2015 at 4:49 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Mon, Mar 30, 2015 at 02:23:47PM +0200, Gerd Hoffmann wrote:
>>> > > Signed-off-by: Dave Airlie <airlied@redhat.com>
>>> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> >
>>> > Standard request from my side for new drm drivers (especially if they're
>>> > this simple): Can you please update the drivers to latest drm internal
>>> > interfaces, i.e. using universal planes and atomic?
>>>
>>> Up'n'running. Incremental patch:
>>>
>>> https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2d&id=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a
>>>
>>> v2 coming, but I'll go over the other reviews first.
>>
>> Looking good. Wrt pageflip the current MO is to handroll it in your
>> driver, common approach is to use the msm async commit implementation
>> msm_atomic_commit. The issue is simply that right now there's still no
>> useable generic vblank callback support (drm_irq.c is a mess) hence why
>> the core helpers don't support async flips yet.
>
> I guess I didn't do a good job at looking at your v2: Cursor is still
> using legacy interfaces and not a proper plane. Would be awesome if
> you could fix that up. Atomic drivers really shouldn't use the legacy
> cursor interfaces any more at all.
Wild idea:
Worth adding if (drm_core_check_feature(dev, DRIVER_ATOMIC) {
printf("abort abort"); return; }
style of checks for the legacy (preatomic) kms helpers ?
Or does it feel like an overkill ?
-Emil
^ permalink raw reply
* [PATCH] x86/paravirt: Do not trace _paravirt_ident_*() functions
From: Steven Rostedt @ 2016-05-25 17:47 UTC (permalink / raw)
To: LKML
Cc: Jeremy Fitzhardinge, Łukasz Daniluk, virtualization,
Chris Wright, Thomas Gleixner, H. Peter Anvin, Alok Kataria,
Ingo Molnar
Łukasz Daniluk reported that on a RHEL kernel that his machine would lock up
after enabling function tracer. I asked him to bisect the functions within
available_filter_functions, which he did and it came down to three:
_paravirt_nop(), _paravirt_ident_32() and _paravirt_ident_64()
It was found that this is only an issue when noreplace-paravirt is added to
the kernel command line.
This means that those functions are most likely called within critical
sections of the funtion tracer, and must not be traced.
In newer kenels _paravirt_nop() is defined within gcc asm(), and is no
longer an issue. But both _paravirt_ident_{32,64}() causes the following
splat when they are traced:
mm/pgtable-generic.c:33: bad pmd ffff8800d2435150(0000000001d00054)
mm/pgtable-generic.c:33: bad pmd ffff8800d3624190(0000000001d00070)
mm/pgtable-generic.c:33: bad pmd ffff8800d36a5110(0000000001d00054)
mm/pgtable-generic.c:33: bad pmd ffff880118eb1450(0000000001d00054)
NMI watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [systemd-journal:469]
Modules linked in: e1000e
CPU: 2 PID: 469 Comm: systemd-journal Not tainted 4.6.0-rc4-test+ #513
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 05/07/2012
task: ffff880118f740c0 ti: ffff8800d4aec000 task.ti: ffff8800d4aec000
RIP: 0010:[<ffffffff81134148>] [<ffffffff81134148>] queued_spin_lock_slowpath+0x118/0x1a0
RSP: 0018:ffff8800d4aefb90 EFLAGS: 00000246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88011eb16d40
RDX: ffffffff82485760 RSI: 000000001f288820 RDI: ffffea0000008030
RBP: ffff8800d4aefb90 R08: 00000000000c0000 R09: 0000000000000000
R10: ffffffff821c8e0e R11: 0000000000000000 R12: ffff880000200fb8
R13: 00007f7a4e3f7000 R14: ffffea000303f600 R15: ffff8800d4b562e0
FS: 00007f7a4e3d7840(0000) GS:ffff88011eb00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f7a4e3f7000 CR3: 00000000d3e71000 CR4: 00000000001406e0
Stack:
ffff8800d4aefba0 ffffffff81cc5f47 ffff8800d4aefc60 ffffffff8122c15b
ffff8800d4aefcb0 ffff8800d4aefbd0 ffffffff811bf4cb 0000000000000002
0000000000000015 ffff8800d2276050 80000000c0fd8867 ffffea0000008030
Call Trace:
[<ffffffff81cc5f47>] _raw_spin_lock+0x27/0x30
[<ffffffff8122c15b>] handle_pte_fault+0x13db/0x16b0
[<ffffffff811bf4cb>] ? function_trace_call+0x15b/0x180
[<ffffffff8122ad85>] ? handle_pte_fault+0x5/0x16b0
[<ffffffff8122e322>] handle_mm_fault+0x312/0x670
[<ffffffff81231068>] ? find_vma+0x68/0x70
[<ffffffff810ab741>] __do_page_fault+0x1b1/0x4e0
[<ffffffff810aba92>] do_page_fault+0x22/0x30
[<ffffffff81cc7f68>] page_fault+0x28/0x30
[<ffffffff81574af5>] ? copy_user_enhanced_fast_string+0x5/0x10
[<ffffffff8129dec5>] ? seq_read+0x305/0x370
[<ffffffff81279668>] __vfs_read+0x28/0xe0
[<ffffffff81279645>] ? __vfs_read+0x5/0xe0
[<ffffffff81279645>] ? __vfs_read+0x5/0xe0
[<ffffffff81279df6>] vfs_read+0x86/0x130
[<ffffffff8127b216>] SyS_read+0x46/0xa0
[<ffffffff81cc6176>] entry_SYSCALL_64_fastpath+0x1e/0xa8
Code: 12 48 c1 ea 0c 83 e8 01 83 e2 30 48 98 48 81 c2 40 6d 01 00 48 03 14
c5 80 6a 5d 82 48 89 0a 8b 41 08 85 c0 75 09 f3 90 8b 41 08 <85> c0 74 f7
4c 8b 09 4d 85 c9 74 08 41 0f 18 09 eb 02 f3 90 8b
Reported-by: Łukasz Daniluk <lukasz.daniluk@intel.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index f08ac28b8136..f975d226be6e 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -55,12 +55,12 @@ asm (".pushsection .entry.text, \"ax\"\n"
".popsection");
/* identity function, which can be inlined */
-u32 _paravirt_ident_32(u32 x)
+u32 notrace _paravirt_ident_32(u32 x)
{
return x;
}
-u64 _paravirt_ident_64(u64 x)
+u64 notrace _paravirt_ident_64(u64 x)
{
return x;
}
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* Re: [PATCH v3 5/6] pv-qspinlock: use cmpxchg_release in __pv_queued_spin_unlock
From: Peter Zijlstra @ 2016-05-26 16:47 UTC (permalink / raw)
To: Pan Xinhui
Cc: jeremy, mpe, linux-kernel, virtualization, chrisw, mingo, paulus,
benh, akataria, paulmck, linuxppc-dev
In-Reply-To: <1464164289-6124-6-git-send-email-xinhui.pan@linux.vnet.ibm.com>
On Wed, May 25, 2016 at 04:18:08PM +0800, Pan Xinhui wrote:
> cmpxchg_release is light-wight than cmpxchg, we can gain a better
> performace then. On some arch like ppc, barrier impact the performace
> too much.
>
> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
> ---
> kernel/locking/qspinlock_paravirt.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
> index a5b1248..2bbffe4 100644
> --- a/kernel/locking/qspinlock_paravirt.h
> +++ b/kernel/locking/qspinlock_paravirt.h
> @@ -614,7 +614,7 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock)
> * unhash. Otherwise it would be possible to have multiple @lock
> * entries, which would be BAD.
> */
> - locked = cmpxchg(&l->locked, _Q_LOCKED_VAL, 0);
> + locked = cmpxchg_release(&l->locked, _Q_LOCKED_VAL, 0);
> if (likely(locked == _Q_LOCKED_VAL))
> return;
This patch fails to explain _why_ it can be relaxed.
And seeing how this cmpxchg() can actually unlock the lock, I don't see
how this can possibly be correct. Maybe cmpxchg_release(), but relaxed
seems very wrong.
^ permalink raw reply
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