date:20120601

Re: [PATCH v2 3/3] virtio-blk: Use block layer provided spinlock

2012-06-01 Thread Michael S. Tsirkin

On Fri, May 25, 2012 at 04:03:27PM +0800, Asias He wrote:
 Block layer will allocate a spinlock for the queue if the driver does
 not provide one in blk_init_queue().
 
 The reason to use the internal spinlock is that blk_cleanup_queue() will
 switch to use the internal spinlock in the cleanup code path.
 
 if (q-queue_lock != q-__queue_lock)
 q-queue_lock = q-__queue_lock;
 
 However, processes which are in D state might have taken the driver
 provided spinlock, when the processes wake up, they would release the
 block provided spinlock.
 
 =
 [ BUG: bad unlock balance detected! ]
 3.4.0-rc7+ #238 Not tainted
 -
 fio/3587 is trying to release lock ((q-__queue_lock)-rlock) at:
 [813274d2] blk_queue_bio+0x2a2/0x380
 but there are no more locks to release!
 
 other info that might help us debug this:
 1 lock held by fio/3587:
  #0:  ((vblk-lock)-rlock){..}, at:
 [8132661a] get_request_wait+0x19a/0x250
 
 Other drivers use block layer provided spinlock as well, e.g. SCSI.
 
 Switching to the block layer provided spinlock saves a bit of memory and
 does not increase lock contention. Performance test shows no real
 difference is observed before and after this patch.
 
 Changes in v2: Improve commit log as Michael suggested.
 
 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: k...@vger.kernel.org
 Signed-off-by: Asias He as...@redhat.com

It may also be worth pointing out that external lock
is inherently broken - it's kept around for historical reasons
only.

See also this discussion
https://lkml.org/lkml/2012/5/28/72

Note: a bugfix so 3.5 material I think.

Acked-by: Michael S. Tsirkin m...@redhat.com


 ---
  drivers/block/virtio_blk.c |9 +++--
  1 file changed, 3 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
 index b4fa2d7..774c31d 100644
 --- a/drivers/block/virtio_blk.c
 +++ b/drivers/block/virtio_blk.c
 @@ -21,8 +21,6 @@ struct workqueue_struct *virtblk_wq;
  
  struct virtio_blk
  {
 - spinlock_t lock;
 -
   struct virtio_device *vdev;
   struct virtqueue *vq;
  
 @@ -65,7 +63,7 @@ static void blk_done(struct virtqueue *vq)
   unsigned int len;
   unsigned long flags;
  
 - spin_lock_irqsave(vblk-lock, flags);
 + spin_lock_irqsave(vblk-disk-queue-queue_lock, flags);
   while ((vbr = virtqueue_get_buf(vblk-vq, len)) != NULL) {
   int error;
  
 @@ -99,7 +97,7 @@ static void blk_done(struct virtqueue *vq)
   }
   /* In case queue is stopped waiting for more buffers. */
   blk_start_queue(vblk-disk-queue);
 - spin_unlock_irqrestore(vblk-lock, flags);
 + spin_unlock_irqrestore(vblk-disk-queue-queue_lock, flags);
  }
  
  static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
 @@ -431,7 +429,6 @@ static int __devinit virtblk_probe(struct virtio_device 
 *vdev)
   goto out_free_index;
   }
  
 - spin_lock_init(vblk-lock);
   vblk-vdev = vdev;
   vblk-sg_elems = sg_elems;
   sg_init_table(vblk-sg, vblk-sg_elems);
 @@ -456,7 +453,7 @@ static int __devinit virtblk_probe(struct virtio_device 
 *vdev)
   goto out_mempool;
   }
  
 - q = vblk-disk-queue = blk_init_queue(do_virtblk_request, vblk-lock);
 + q = vblk-disk-queue = blk_init_queue(do_virtblk_request, NULL);
   if (!q) {
   err = -ENOMEM;
   goto out_put_disk;
 -- 
 1.7.10.2
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

2012-06-01 Thread Srivatsa S. Bhat

xen_play_dead calls cpu_bringup() which looks weird, because xen_play_dead()
is invoked in the cpu down path, whereas cpu_bringup() (as the name suggests)
is useful in the cpu bringup path.

Getting rid of xen_play_dead()'s dependency on cpu_bringup() helps in hooking
on to the generic SMP booting framework.

Also remove the extra call to preempt_enable() added by commit 41bd956
(xen/smp: Fix CPU online/offline bug triggering a BUG: scheduling while
atomic) because it becomes unnecessary after this change.

Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Cc: Jeremy Fitzhardinge jer...@goop.org
Cc: Thomas Gleixner t...@linutronix.de
Cc: Ingo Molnar mi...@redhat.com
Cc: H. Peter Anvin h...@zytor.com
Cc: x...@kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
---

 arch/x86/xen/smp.c |8 
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 09a7199..602d6b7 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -417,14 +417,6 @@ static void __cpuinit xen_play_dead(void) /* used only 
with HOTPLUG_CPU */
 {
play_dead_common();
HYPERVISOR_vcpu_op(VCPUOP_down, smp_processor_id(), NULL);
-   cpu_bringup();
-   /*
-* Balance out the preempt calls - as we are running in cpu_idle
-* loop which has been called at bootup from cpu_bringup_and_idle.
-* The cpucpu_bringup_and_idle called cpu_bringup which made a
-* preempt_disable() So this preempt_enable will balance it out.
-*/
-   preempt_enable();
 }
 
 #else /* !CONFIG_HOTPLUG_CPU */

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

[PATCH 06/27] xen, smpboot: Use generic SMP booting infrastructure

2012-06-01 Thread Srivatsa S. Bhat

Convert xen to use the generic framework to boot secondary CPUs.

Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Cc: Jeremy Fitzhardinge jer...@goop.org
Cc: Thomas Gleixner t...@linutronix.de
Cc: Ingo Molnar mi...@redhat.com
Cc: H. Peter Anvin h...@zytor.com
Cc: x...@kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
---

 arch/x86/xen/smp.c |   21 -
 1 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 602d6b7..46c96f9 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -58,13 +58,12 @@ static irqreturn_t xen_reschedule_interrupt(int irq, void 
*dev_id)
return IRQ_HANDLED;
 }
 
-static void __cpuinit cpu_bringup(void)
+void __cpuinit xen_cpu_pre_starting(void *unused)
 {
int cpu;
 
cpu_init();
touch_softlockup_watchdog();
-   preempt_disable();
 
xen_enable_sysenter();
xen_enable_syscall();
@@ -75,25 +74,11 @@ static void __cpuinit cpu_bringup(void)
set_cpu_sibling_map(cpu);
 
xen_setup_cpu_clockevents();
-
-   notify_cpu_starting(cpu);
-
-   set_cpu_online(cpu, true);
-
-   this_cpu_write(cpu_state, CPU_ONLINE);
-
-   wmb();
-
-   /* We can take interrupts now: we're officially up. */
-   local_irq_enable();
-
-   wmb();  /* make sure everything is out */
 }
 
 static void __cpuinit cpu_bringup_and_idle(void)
 {
-   cpu_bringup();
-   cpu_idle();
+   smpboot_start_secondary(NULL);
 }
 
 static int xen_smp_intr_init(unsigned int cpu)
@@ -515,6 +500,8 @@ static const struct smp_ops xen_smp_ops __initconst = {
.smp_prepare_cpus = xen_smp_prepare_cpus,
.smp_cpus_done = xen_smp_cpus_done,
 
+   .cpu_pre_starting = xen_cpu_pre_starting,
+
.cpu_up = xen_cpu_up,
.cpu_die = xen_cpu_die,
.cpu_disable = xen_cpu_disable,

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

[PATCH v3] virtio_blk: unlock vblk-lock during kick

2012-06-01 Thread Stefan Hajnoczi

Holding the vblk-lock across kick causes poor scalability in SMP
guests.  If one CPU is doing virtqueue kick and another CPU touches the
vblk-lock it will have to spin until virtqueue kick completes.

This patch reduces system% CPU utilization in SMP guests that are
running multithreaded I/O-bound workloads.  The improvements are small
but show as iops and SMP are increased.

Khoa Huynh k...@us.ibm.com provided initial performance data that
indicates this optimization is worthwhile at high iops.

Asias He as...@redhat.com reports the following fio results:

Host: Linux 3.4.0+ #302 SMP x86_64 GNU/Linux
Guest: same as host kernel

Average 3 runs:
with locked kick
readiops=119907.50 bw=59954.00 runt=35018.50 io=2048.00
write   iops=217187.00 bw=108594.00 runt=19312.00 io=2048.00
readiops=33948.00 bw=16974.50 runt=186820.50 io=3095.70
write   iops=35014.00 bw=17507.50 runt=181151.00 io=3095.70
clat (usec) max=3484.10 avg=121085.38 stdev=174416.11 min=0.00
clat (usec) max=3438.30 avg=59863.35 stdev=116607.69 min=0.00
clat (usec) max=3745.65 avg=454501.30 stdev=332699.00 min=0.00
clat (usec) max=4089.75 avg=442374.99 stdev=304874.62 min=0.00
cpu sys=615.12 majf=24080.50 ctx=64253616.50 usr=68.08 minf=17907363.00
cpu sys=1235.95 majf=23389.00 ctx=59788148.00 usr=98.34 minf=20020008.50
cpu sys=764.96 majf=28414.00 ctx=848279274.00 usr=36.39 minf=19737254.00
cpu sys=714.13 majf=21853.50 ctx=854608972.00 usr=33.56 minf=18256760.50

with unlocked kick
readiops=118559.00 bw=59279.66 runt=35400.66 io=2048.00
write   iops=227560.00 bw=113780.33 runt=18440.00 io=2048.00
readiops=34567.66 bw=17284.00 runt=183497.33 io=3095.70
write   iops=34589.33 bw=17295.00 runt=183355.00 io=3095.70
clat (usec) max=3485.56 avg=121989.58 stdev=197355.15 min=0.00
clat (usec) max=3222.33 avg=57784.11 stdev=141002.89 min=0.00
clat (usec) max=4060.93 avg=447098.65 stdev=315734.33 min=0.00
clat (usec) max=3656.30 avg=447281.70 stdev=314051.33 min=0.00
cpu sys=683.78 majf=24501.33 ctx=64435364.66 usr=68.91 minf=17907893.33
cpu sys=1218.24 majf=25000.33 ctx=60451475.00 usr=101.04 minf=19757720.00
cpu sys=740.39 majf=24809.00 ctx=845290443.66 usr=37.25 minf=19349958.33
cpu sys=723.63 majf=27597.33 ctx=850199927.33 usr=35.35 minf=19092343.00

FIO config file:

[global]
exec_prerun=echo 3  /proc/sys/vm/drop_caches
group_reporting
norandommap
ioscheduler=noop
thread
bs=512
size=4MB
direct=1
filename=/dev/vdb
numjobs=256
ioengine=aio
iodepth=64
loops=3

Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
Other block drivers (cciss, rbd, nbd) use spin_unlock_irq() so I followed that.
To me this seems wrong: blk_run_queue() uses spin_lock_irqsave() but we enable
irqs with spin_unlock_irq().  If the caller of blk_run_queue() had irqs
disabled and we enable them again this could be a problem, right?  Can someone
more familiar with kernel locking comment?

 drivers/block/virtio_blk.c |   10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 774c31d..d674977 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -199,8 +199,14 @@ static void do_virtblk_request(struct request_queue *q)
issued++;
}
 
-   if (issued)
-   virtqueue_kick(vblk-vq);
+   if (!issued)
+   return;
+
+   if (virtqueue_kick_prepare(vblk-vq)) {
+   spin_unlock_irq(vblk-disk-queue-queue_lock);
+   virtqueue_notify(vblk-vq);
+   spin_lock_irq(vblk-disk-queue-queue_lock);
+   }
 }
 
 /* return id (s/n) string for *disk to *id_str
-- 
1.7.10

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

[PATCH 03/27] smpboot: Define and use cpu_state per-cpu variable in generic code

2012-06-01 Thread Srivatsa S. Bhat

The per-cpu variable cpu_state is used in x86 and also used in other
architectures, to track the state of the cpu during bringup and hotplug.
Pull it out into generic code.

Cc: Tony Luck tony.l...@intel.com
Cc: Fenghua Yu fenghua...@intel.com
Cc: Ralf Baechle r...@linux-mips.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Paul Mundt let...@linux-sh.org
Cc: Chris Metcalf cmetc...@tilera.com
Cc: Thomas Gleixner t...@linutronix.de
Cc: Ingo Molnar mi...@redhat.com
Cc: H. Peter Anvin h...@zytor.com
Cc: x...@kernel.org
Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Cc: Jeremy Fitzhardinge jer...@goop.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Andrew Morton a...@linux-foundation.org
Cc: Mike Frysinger vap...@gentoo.org
Cc: Yong Zhang yong.zha...@gmail.com
Cc: Venkatesh Pallipadi ve...@google.com
Cc: Suresh Siddha suresh.b.sid...@intel.com
Cc: linux-i...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux...@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
---

 arch/ia64/include/asm/cpu.h   |2 --
 arch/ia64/kernel/process.c|1 +
 arch/ia64/kernel/smpboot.c|6 +-
 arch/mips/cavium-octeon/smp.c |4 +---
 arch/powerpc/kernel/smp.c |6 +-
 arch/sh/include/asm/smp.h |2 --
 arch/sh/kernel/smp.c  |4 +---
 arch/tile/kernel/smpboot.c|4 +---
 arch/x86/include/asm/cpu.h|2 --
 arch/x86/kernel/smpboot.c |4 +---
 arch/x86/xen/smp.c|1 +
 include/linux/smpboot.h   |1 +
 kernel/smpboot.c  |4 
 13 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/arch/ia64/include/asm/cpu.h b/arch/ia64/include/asm/cpu.h
index fcca30b..1c3acac 100644
--- a/arch/ia64/include/asm/cpu.h
+++ b/arch/ia64/include/asm/cpu.h
@@ -12,8 +12,6 @@ struct ia64_cpu {
 
 DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
 
-DECLARE_PER_CPU(int, cpu_state);
-
 #ifdef CONFIG_HOTPLUG_CPU
 extern int arch_register_cpu(int num);
 extern void arch_unregister_cpu(int);
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 5e0e86d..32566c7 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -29,6 +29,7 @@
 #include linux/kdebug.h
 #include linux/utsname.h
 #include linux/tracehook.h
+#include linux/smpboot.h
 
 #include asm/cpu.h
 #include asm/delay.h
diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index 963d2db..df00a3c 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -39,6 +39,7 @@
 #include linux/efi.h
 #include linux/percpu.h
 #include linux/bitops.h
+#include linux/smpboot.h
 
 #include linux/atomic.h
 #include asm/cache.h
@@ -111,11 +112,6 @@ extern unsigned long ia64_iobase;
 
 struct task_struct *task_for_booting_cpu;
 
-/*
- * State for each CPU
- */
-DEFINE_PER_CPU(int, cpu_state);
-
 cpumask_t cpu_core_map[NR_CPUS] __cacheline_aligned;
 EXPORT_SYMBOL(cpu_core_map);
 DEFINE_PER_CPU_SHARED_ALIGNED(cpumask_t, cpu_sibling_map);
diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 97e7ce9..93cd4b0 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -13,6 +13,7 @@
 #include linux/kernel_stat.h
 #include linux/sched.h
 #include linux/module.h
+#include linux/smpboot.h
 
 #include asm/mmu_context.h
 #include asm/time.h
@@ -252,9 +253,6 @@ static void octeon_cpus_done(void)
 
 #ifdef CONFIG_HOTPLUG_CPU
 
-/* State of each CPU. */
-DEFINE_PER_CPU(int, cpu_state);
-
 extern void fixup_irqs(void);
 
 static DEFINE_SPINLOCK(smp_reserve_lock);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index e1417c4..1928058a 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -31,6 +31,7 @@
 #include linux/cpu.h
 #include linux/notifier.h
 #include linux/topology.h
+#include linux/smpboot.h
 
 #include asm/ptrace.h
 #include linux/atomic.h
@@ -57,11 +58,6 @@
 #define DBG(fmt...)
 #endif
 
-#ifdef CONFIG_HOTPLUG_CPU
-/* State of each CPU during hotplug phases */
-static DEFINE_PER_CPU(int, cpu_state) = { 0 };
-#endif
-
 struct thread_info *secondary_ti;
 
 DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
diff --git a/arch/sh/include/asm/smp.h b/arch/sh/include/asm/smp.h
index 78b0d0f4..bda041e 100644
--- a/arch/sh/include/asm/smp.h
+++ b/arch/sh/include/asm/smp.h
@@ -31,8 +31,6 @@ enum {
SMP_MSG_NR, /* must be last */
 };
 
-DECLARE_PER_CPU(int, cpu_state);
-
 void smp_message_recv(unsigned int msg);
 void smp_timer_broadcast(const struct cpumask *mask);
 
diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c
index b86e9ca..8e0fde0 100644
--- a/arch/sh/kernel/smp.c
+++ b/arch/sh/kernel/smp.c
@@ -22,6 +22,7 @@
 #include linux/interrupt.h
 #include linux/sched.h
 #include linux/atomic.h
+#include linux/smpboot.h
 #include asm/processor.h
 #include asm/mmu_context.h
 #include asm/smp.h

Re: [Xen-devel] [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

2012-06-01 Thread Jan Beulich

 On 01.06.12 at 11:11, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
wrote:
 xen_play_dead calls cpu_bringup() which looks weird, because xen_play_dead()
 is invoked in the cpu down path, whereas cpu_bringup() (as the name 
 suggests) is useful in the cpu bringup path.

This might not be correct - the code as it is without this change is
safe even when the vCPU gets onlined back later by an external
entity (e.g. the Xen tool stack), and it would in that case resume
at the return point of the VCPUOP_down hypercall. That might
be a heritage from the original XenoLinux tree though, and be
meaningless in pv-ops context - Jeremy, Konrad?

Possibly it was bogus/unused even in that original tree - Keir?

Jan

 Getting rid of xen_play_dead()'s dependency on cpu_bringup() helps in 
 hooking on to the generic SMP booting framework.
 
 Also remove the extra call to preempt_enable() added by commit 41bd956
 (xen/smp: Fix CPU online/offline bug triggering a BUG: scheduling while
 atomic) because it becomes unnecessary after this change.
 
 Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
 Cc: Jeremy Fitzhardinge jer...@goop.org
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Ingo Molnar mi...@redhat.com
 Cc: H. Peter Anvin h...@zytor.com
 Cc: x...@kernel.org 
 Cc: xen-de...@lists.xensource.com 
 Cc: virtualization@lists.linux-foundation.org 
 Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
 ---
 
  arch/x86/xen/smp.c |8 
  1 files changed, 0 insertions(+), 8 deletions(-)
 
 diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
 index 09a7199..602d6b7 100644
 --- a/arch/x86/xen/smp.c
 +++ b/arch/x86/xen/smp.c
 @@ -417,14 +417,6 @@ static void __cpuinit xen_play_dead(void) /* used only 
 with HOTPLUG_CPU */
  {
   play_dead_common();
   HYPERVISOR_vcpu_op(VCPUOP_down, smp_processor_id(), NULL);
 - cpu_bringup();
 - /*
 -  * Balance out the preempt calls - as we are running in cpu_idle
 -  * loop which has been called at bootup from cpu_bringup_and_idle.
 -  * The cpucpu_bringup_and_idle called cpu_bringup which made a
 -  * preempt_disable() So this preempt_enable will balance it out.
 -  */
 - preempt_enable();
  }
  
  #else /* !CONFIG_HOTPLUG_CPU */
 
 
 ___
 Xen-devel mailing list
 xen-de...@lists.xen.org 
 http://lists.xen.org/xen-devel 



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

Re: [Xen-devel] [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

2012-06-01 Thread Srivatsa S. Bhat

On 06/01/2012 06:29 PM, Jan Beulich wrote:

 On 01.06.12 at 11:11, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
 wrote:
 xen_play_dead calls cpu_bringup() which looks weird, because xen_play_dead()
 is invoked in the cpu down path, whereas cpu_bringup() (as the name 
 suggests) is useful in the cpu bringup path.
 
 This might not be correct - the code as it is without this change is
 safe even when the vCPU gets onlined back later by an external
 entity (e.g. the Xen tool stack), and it would in that case resume
 at the return point of the VCPUOP_down hypercall. That might
 be a heritage from the original XenoLinux tree though, and be
 meaningless in pv-ops context - Jeremy, Konrad?
 
 Possibly it was bogus/unused even in that original tree - Keir?



Thanks for your comments Jan!

In case this change is wrong, the other method I had in mind was to call
cpu_bringup_and_idle() in xen_play_dead(). (Even ARM does something similar,
in the sense that it runs the cpu bringup code including cpu_idle(), in the
cpu offline path, namely the cpu_die() function). Would that approach work
for xen as well? If yes, then we wouldn't have any issues to convert xen to
generic code.

Regards,
Srivatsa S. Bhat

 
 Getting rid of xen_play_dead()'s dependency on cpu_bringup() helps in 
 hooking on to the generic SMP booting framework.

 Also remove the extra call to preempt_enable() added by commit 41bd956
 (xen/smp: Fix CPU online/offline bug triggering a BUG: scheduling while
 atomic) because it becomes unnecessary after this change.

 Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
 Cc: Jeremy Fitzhardinge jer...@goop.org
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Ingo Molnar mi...@redhat.com
 Cc: H. Peter Anvin h...@zytor.com
 Cc: x...@kernel.org 
 Cc: xen-de...@lists.xensource.com 
 Cc: virtualization@lists.linux-foundation.org 
 Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
 ---

  arch/x86/xen/smp.c |8 
  1 files changed, 0 insertions(+), 8 deletions(-)

 diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
 index 09a7199..602d6b7 100644
 --- a/arch/x86/xen/smp.c
 +++ b/arch/x86/xen/smp.c
 @@ -417,14 +417,6 @@ static void __cpuinit xen_play_dead(void) /* used only 
 with HOTPLUG_CPU */
  {
  play_dead_common();
  HYPERVISOR_vcpu_op(VCPUOP_down, smp_processor_id(), NULL);
 -cpu_bringup();
 -/*
 - * Balance out the preempt calls - as we are running in cpu_idle
 - * loop which has been called at bootup from cpu_bringup_and_idle.
 - * The cpucpu_bringup_and_idle called cpu_bringup which made a
 - * preempt_disable() So this preempt_enable will balance it out.
 - */
 -preempt_enable();
  }
  
  #else /* !CONFIG_HOTPLUG_CPU */



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

Re: [Xen-devel] [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

2012-06-01 Thread Jan Beulich

 On 01.06.12 at 17:13, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
wrote:
 On 06/01/2012 06:29 PM, Jan Beulich wrote:
 
 On 01.06.12 at 11:11, Srivatsa S. Bhat 
 srivatsa.b...@linux.vnet.ibm.com
 wrote:
 xen_play_dead calls cpu_bringup() which looks weird, because xen_play_dead()
 is invoked in the cpu down path, whereas cpu_bringup() (as the name 
 suggests) is useful in the cpu bringup path.
 
 This might not be correct - the code as it is without this change is
 safe even when the vCPU gets onlined back later by an external
 entity (e.g. the Xen tool stack), and it would in that case resume
 at the return point of the VCPUOP_down hypercall. That might
 be a heritage from the original XenoLinux tree though, and be
 meaningless in pv-ops context - Jeremy, Konrad?
 
 Possibly it was bogus/unused even in that original tree - Keir?

 
 
 Thanks for your comments Jan!
 
 In case this change is wrong, the other method I had in mind was to call
 cpu_bringup_and_idle() in xen_play_dead(). (Even ARM does something similar,
 in the sense that it runs the cpu bringup code including cpu_idle(), in the
 cpu offline path, namely the cpu_die() function). Would that approach work
 for xen as well? If yes, then we wouldn't have any issues to convert xen to
 generic code.

No, that wouldn't work either afaict - the function is expected
to return.

Jan

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

Question regarding Virtio Console and Remoteproc

2012-06-01 Thread Sjur BRENDELAND

Hi Amit and Rusty,

I've been looking into the possibility of using the Virtio Console
Driver together with the remoteproc framework to communicate with
ST-Ericsson modem over shared memory.

It seems like Virtio Console would be a good fit, except for a issue
with buffer allocation. Due to HW limitations the STE-Modem cannot
access kernel memory (no IOMMU and limited address range). Instead
we have a designated shared memory region used for IPC. 

Due to this I cannot use kmalloc() for buffer allocation, but I
have to allocate buffers from the memory region shared with the
modem.

In remoteproc this is solved by using dma_alloc_coherent() for all
memory to be shared with the modem. This works fine for me, because
I can pass the IPC memory region to dma_declare_coherent_memory() 
so dma_alloc_coherent() will allocate from this memory region.

I think I can solve this issue in Virtio Console by changing calls
to kmalloc() to something like:

if (virtio_has_feature(vdev, VIRTIO_CONSOLE_USE_DMA_MEM)) {
dma_addr_t dma;
buf = dma_alloc_coherent(dev, size, dma, GFP_KERNEL);
} else
buf = kmalloc(count, GFP_KERNEL);

I'd like to get the opinion from you virtualization folks on this!
If you think it looks reasonable I might start cooking some patches...

Regards,
Sjur



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

Re: [vmw_vmci RFC 00/11] VMCI for Linux

2012-06-01 Thread Andy King

Greg,

Thanks so much for the comments and apologies for the delayed response.

 Don't we have something like this already for KVM and maybe Xen?
 virtio?  Can't you use that code instead of a new block of code that
 is only used by vmware users?  It has virtual pci devices which
 should give you what you want/need here, right?

 If not, why doesn't that work for you?  Would it be easier to just
 extend it?

The VMCI virtual device for which this driver is intended has been
around a lot longer than this submission might suggest.  The virtual
hardware was released in a product before Rusty sent his RFC and
quite a bit before it made it to mainline; there was, regrettably,
no virtio then.

As such, it was designed to be its own transport, and it's something
that is now very much fixed at the hardware level (enhancements
not withstanding), and which we have to support all the way back.

In addition to that, our hypervisor endpoints are written using
the existing device backend; virtio doesn't currently make a lot of
sense for them, and would require a lot of additional work.

All of this is unfortunate.  While I agree that virtio is certainly
the right approach, and we need to avoid this proliferation, I think
at this point we'd really like to try and upstream this in its current
form.  There's certainly the possibility going forwards that we could
add a glue layer, such that other clients could use virtio if they're
willing to write their own hypervisor endpoints.

Does that sound reasonable?

Again, many thanks for taking the time to review this.

Kind regards,
- Andy King
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 03/27] smpboot: Define and use cpu_state per-cpu variable in generic code

2012-06-01 Thread David Daney


On 06/01/2012 02:10 AM, Srivatsa S. Bhat wrote:

The per-cpu variable cpu_state is used in x86 and also used in other
architectures, to track the state of the cpu during bringup and hotplug.
Pull it out into generic code.

Cc: Tony Lucktony.l...@intel.com
Cc: Fenghua Yufenghua...@intel.com
Cc: Ralf Baechler...@linux-mips.org
Cc: Benjamin Herrenschmidtb...@kernel.crashing.org
Cc: Paul Mundtlet...@linux-sh.org
Cc: Chris Metcalfcmetc...@tilera.com
Cc: Thomas Gleixnert...@linutronix.de
Cc: Ingo Molnarmi...@redhat.com
Cc: H. Peter Anvinh...@zytor.com
Cc: x...@kernel.org
Cc: Konrad Rzeszutek Wilkkonrad.w...@oracle.com
Cc: Jeremy Fitzhardingejer...@goop.org
Cc: Peter Zijlstraa.p.zijls...@chello.nl
Cc: Andrew Mortona...@linux-foundation.org
Cc: Mike Frysingervap...@gentoo.org
Cc: Yong Zhangyong.zha...@gmail.com
Cc: Venkatesh Pallipadive...@google.com
Cc: Suresh Siddhasuresh.b.sid...@intel.com
Cc: linux-i...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux...@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Srivatsa S. Bhatsrivatsa.b...@linux.vnet.ibm.com
---

  arch/ia64/include/asm/cpu.h   |2 --
  arch/ia64/kernel/process.c|1 +
  arch/ia64/kernel/smpboot.c|6 +-
  arch/mips/cavium-octeon/smp.c |4 +---
  arch/powerpc/kernel/smp.c |6 +-
  arch/sh/include/asm/smp.h |2 --
  arch/sh/kernel/smp.c  |4 +---
  arch/tile/kernel/smpboot.c|4 +---
  arch/x86/include/asm/cpu.h|2 --
  arch/x86/kernel/smpboot.c |4 +---
  arch/x86/xen/smp.c|1 +
  include/linux/smpboot.h   |1 +
  kernel/smpboot.c  |4 
  13 files changed, 13 insertions(+), 28 deletions(-)


[...]

diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 97e7ce9..93cd4b0 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -13,6 +13,7 @@
  #includelinux/kernel_stat.h
  #includelinux/sched.h
  #includelinux/module.h
+#includelinux/smpboot.h

  #includeasm/mmu_context.h
  #includeasm/time.h
@@ -252,9 +253,6 @@ static void octeon_cpus_done(void)

  #ifdef CONFIG_HOTPLUG_CPU

-/* State of each CPU. */
-DEFINE_PER_CPU(int, cpu_state);
-
  extern void fixup_irqs(void);

  static DEFINE_SPINLOCK(smp_reserve_lock);


The Octeon bit:

Acked-by: David Daney david.da...@cavium.com


FWIW, the rest looks good too.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 3/3] virtio-blk: Use block layer provided spinlock

[PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

[PATCH 06/27] xen, smpboot: Use generic SMP booting infrastructure

[PATCH v3] virtio_blk: unlock vblk-lock during kick

[PATCH 03/27] smpboot: Define and use cpu_state per-cpu variable in generic code

Re: [Xen-devel] [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

Re: [Xen-devel] [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

Re: [Xen-devel] [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

Question regarding Virtio Console and Remoteproc

Re: [vmw_vmci RFC 00/11] VMCI for Linux

Re: [PATCH 03/27] smpboot: Define and use cpu_state per-cpu variable in generic code

11 matches

Site Navigation

Mail list logo

Footer information