* Re: [PATCH RFC V4 0/5] kvm : Paravirt-spinlock support for KVM guests
From: Dave Hansen @ 2012-01-17 21:57 UTC (permalink / raw)
To: Raghavendra K T
Cc: Jeremy Fitzhardinge, Greg Kroah-Hartman, linux-doc,
Peter Zijlstra, Jan Kiszka, Srivatsa Vaddagiri, Paul Mackerras,
H. Peter Anvin, Stefano Stabellini, Xen, Dave Jiang, KVM,
Glauber Costa, X86, Ingo Molnar, Avi Kivity, Rik van Riel,
Konrad Rzeszutek Wilk, Sasha Levin, Sedat Dilek, Thomas Gleixner,
Virtualization, LKML, Suzuki Poulose <suzuk>
In-Reply-To: <4F15BFAE.7060500@linux.vnet.ibm.com>
On 01/17/2012 10:36 AM, Raghavendra K T wrote:
> It was a quick test. two iteration of kernbench (=6runs) and had
> ensured cache is cleared.
>
> echo "1" > /proc/sys/vm/drop_caches
> ccache -C. Yes may be I can run test as you mentioned..
echo 3 > /proc/sys/vm/drop_caches
is better. 1 will only do page cache, but 3 also does dentries fwiw.
^ permalink raw reply
* Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
From: Jeremy Fitzhardinge @ 2012-01-18 1:34 UTC (permalink / raw)
To: Srivatsa Vaddagiri
Cc: X86, linux-doc, Peter Zijlstra, Jan Kiszka, Virtualization,
Paul Mackerras, H. Peter Anvin, Stefano Stabellini, Xen,
Dave Jiang, KVM, Glauber Costa, Raghavendra K T, Ingo Molnar,
Avi Kivity, Rik van Riel, Konrad Rzeszutek Wilk,
Greg Kroah-Hartman, Sasha Levin, Sedat Dilek, Thomas Gleixner,
LKML, Dave Hansen
In-Reply-To: <20120117113312.GB30398@linux.vnet.ibm.com>
On 01/17/2012 10:33 PM, Srivatsa Vaddagiri wrote:
> * Marcelo Tosatti <mtosatti@redhat.com> [2012-01-17 09:02:11]:
>
>>> +/* Kick vcpu waiting on @lock->head to reach value @ticket */
>>> +static void kvm_unlock_kick(struct arch_spinlock *lock, __ticket_t ticket)
>>> +{
>>> + int cpu;
>>> + int apicid;
>>> +
>>> + add_stats(RELEASED_SLOW, 1);
>>> +
>>> + for_each_cpu(cpu, &waiting_cpus) {
>>> + const struct kvm_lock_waiting *w = &per_cpu(lock_waiting, cpu);
>>> + if (ACCESS_ONCE(w->lock) == lock &&
>>> + ACCESS_ONCE(w->want) == ticket) {
>>> + add_stats(RELEASED_SLOW_KICKED, 1);
>>> + apicid = per_cpu(x86_cpu_to_apicid, cpu);
>>> + kvm_kick_cpu(apicid);
>>> + break;
>>> + }
>>> + }
>> What prevents a kick from being lost here, if say, the waiter is at
>> local_irq_save in kvm_lock_spinning, before the lock/want assignments?
> The waiter does check for lock becoming available before actually
> sleeping:
>
> + /*
> + * check again make sure it didn't become free while
> + * we weren't looking.
> + */
> + if (ACCESS_ONCE(lock->tickets.head) == want) {
> + add_stats(TAKEN_SLOW_PICKUP, 1);
> + goto out;
> + }
That logic relies on the "kick" being level triggered, so that "kick"
before "block" will cause the block to fall out immediately. If you're
using "hlt" as the block and it has the usual edge-triggered behaviour,
what stops a "kick-before-hlt" from losing the kick?
J
^ permalink raw reply
* Re: [PATCH RFC V4 0/5] kvm : Paravirt-spinlock support for KVM guests
From: Raghavendra K T @ 2012-01-18 2:27 UTC (permalink / raw)
To: Dave Hansen
Cc: Jeremy Fitzhardinge, Greg Kroah-Hartman, linux-doc,
Peter Zijlstra, Jan Kiszka, Srivatsa Vaddagiri, Paul Mackerras,
H. Peter Anvin, Stefano Stabellini, Xen, Dave Jiang, KVM,
Glauber Costa, X86, Ingo Molnar, Avi Kivity, Rik van Riel,
Konrad Rzeszutek Wilk, Sasha Levin, Sedat Dilek, Thomas Gleixner,
Virtualization, LKML, Suzuki Poulose <suzuk>
In-Reply-To: <4F15EEE6.5020207@linux.vnet.ibm.com>
On 01/18/2012 03:27 AM, Dave Hansen wrote:
> On 01/17/2012 10:36 AM, Raghavendra K T wrote:
>> It was a quick test. two iteration of kernbench (=6runs) and had
>> ensured cache is cleared.
>>
>> echo "1"> /proc/sys/vm/drop_caches
>> ccache -C. Yes may be I can run test as you mentioned..
>
> echo 3> /proc/sys/vm/drop_caches
>
> is better. 1 will only do page cache, but 3 also does dentries fwiw.
>
yes. that needs to be used.
^ permalink raw reply
* Re: [RFC 7/11] virtio_pci: new, capability-aware driver.
From: Rusty Russell @ 2012-01-18 4:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Pawel Moll, Benjamin Herrenschmidt, virtualization,
Christian Borntraeger, Sasha Levin, Anthony Liguori
In-Reply-To: <20120113021930.GA15379@redhat.com>
On Fri, 13 Jan 2012 04:19:30 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Jan 12, 2012 at 12:12:17PM +1030, Rusty Russell wrote:
> > On Thu, 12 Jan 2012 00:02:33 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > Look, we have a race currently. Let us not tie a bug fix to a huge
> > > rewrite with unclear performance benefits, please.
> >
> > In theory, yes. In practice, we bandaid it.
> >
> > I think in the short term we change ->get to get the entire sequence
> > twice, and check it's the same. Theoretically, still racy, but it does
> > cut the window. And we haven't seen the bug yet, either.
>
> I thought about this some more. Since we always get
> an interrupt on config changes, it seems that a rather
> robust method would be to just synchronize against that.
> Something like the below (warning - completely untested).
> Still need to think about memory barriers, overflow etc.
> What do you think?
Ben's point about arbitrary delays in irq delivery still applies. If
qemu changes config space the injects, there's a window there, too.
But in practice, like my suggested double-read, it probably slows things
down enough that it would be hard to hit. Testing required...
Obviously, I'd like to fix this properly, but for legacy this might work
fine.
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCH RFC V4 0/5] kvm : Paravirt-spinlock support for KVM guests
From: Raghavendra K T @ 2012-01-18 10:23 UTC (permalink / raw)
To: Jeremy Fitzhardinge, Alexander Graf
Cc: Greg Kroah-Hartman, linux-doc, Peter Zijlstra, Jan Kiszka,
Virtualization, Paul Mackerras, Raghavendra, H. Peter Anvin,
Stefano Stabellini, Xen, Dave Jiang, KVM, Rob Landley, X86,
Ingo Molnar, Avi Kivity, Rik van Riel, Konrad Rzeszutek Wilk,
Srivatsa Vaddagiri, Sasha Levin, Sedat Dilek, Thomas Gleixner,
LKML
In-Reply-To: <4F14C10C.3010303@goop.org>
On 01/17/2012 06:00 AM, Jeremy Fitzhardinge wrote:
> On 01/16/2012 09:24 PM, Alexander Graf wrote:
>> This is true in case you're spinning. If on overcommit spinlocks would
>> instead of spin just yield(), we wouldn't have any vcpu running that's
>> just waiting for a late ticket.
>
> Yes, but the reality is that most spinlocks are held for a short period
> of time and there's a low likelihood of being preempted while within a
> spinlock critical section. Therefore if someone else tries to get the
> spinlock and there's contention, it's always worth spinning for a little
> while because the lock will likely become free soon.
I too believe that lock-holder-preemption forms small part of the
problem. Non - PLE machine results seem to support that for the patch.
>
> At least that's the case if the lock has low contention (shallow queue
> depth and not in slow state). Again, maybe it makes sense to never spin
> for deep queues or already slowstate locks.
>
>> We still have an issue finding the point in time when a vcpu could run again, which is what this whole series is about. My point above was that instead of doing a count loop, we could just do the normal spin dance and set the threshold to when we enable the magic to have another spin lock notify us in the CPU. That way we
>>
>> * don't change the uncontended case
>
> I don't follow you. What do you mean by "the normal spin dance"? What
> do you mean by "have another spinlock notify us in the CPU"? Don't
> change which uncontended case? Do you mean in the locking path? Or the
> unlock path? Or both?
>
>> * can set the threshold on the host, which knows how contended the system is
>
> Hm, I'm not convinced that knowing how contended the system is is all
> that useful overall. What's important is how contended a particular
> lock is, and what state the current holder is in. If it's not currently
> running, then knowing the overall system contention would give you some
> idea about how long you need to wait for it to be rescheduled, but
> that's getting pretty indirect.
>
> I think the "slowpath if preempted while spinning" idea I mentioned in
> the other mail is probably worth following up, since that give specific
> actionable information to the guest from the hypervisor. But lots of
> caveats.
>
> [[
> A possible mechanism:
>
> * register ranges of [er]ips with the hypervisor
> * each range is paired with a "resched handler block"
> * if vcpu is preempted within such a range, make sure it is
> rescheduled in the resched handler block
>
> This is obviously akin to the exception mechanism, but it is partially
> implemented by the hypervisor. It allows the spinlock code to be
> unchanged from native, but make use of a resched rather than an explicit
> counter to determine when to slowpath the lock. And it's a nice general
> mechanism that could be potentially useful elsewhere.
>
> Unfortunately, it doesn't change the unlock path at all; it still needs
> to explicitly test if a VCPU needs to be kicked on unlock.
> ]]
>
>
>> And since we control what spin locks look like, we can for example always keep the pointer to it in a specific register so that we can handle pv_lock_ops.lock_spinning() inside there and fetch all the information we need from our pt_regs.
>
> You've left a pile of parts of an idea lying around, but I'm not sure
> what shape you intend it to be.
Interesting option But, Is it a feasible option to have specific
registers ? Considering we can have nested locks [ which means we need
a table ] and also considering the fact that "normal" spinlock
acquisition path should have less overhead.
>
> J
>
>
^ permalink raw reply
* Re: [PATCH RFC V4 0/5] kvm : Paravirt-spinlock support for KVM guests
From: Raghavendra K T @ 2012-01-18 10:48 UTC (permalink / raw)
To: Jeremy Fitzhardinge, Avi Kivity
Cc: Greg Kroah-Hartman, linux-doc, Peter Zijlstra, Jan Kiszka,
Virtualization, Paul Mackerras, Raghavendra, H. Peter Anvin,
Stefano Stabellini, Xen, Dave Jiang, KVM, Rob Landley, X86,
Ingo Molnar, Rik van Riel, Konrad Rzeszutek Wilk,
Srivatsa Vaddagiri, Sasha Levin, Sedat Dilek, Thomas Gleixner,
LKML, Dave Hansen
In-Reply-To: <4F14B9C7.9090709@goop.org>
On 01/17/2012 05:29 AM, Jeremy Fitzhardinge wrote:
> On 01/16/2012 07:55 PM, Avi Kivity wrote:
>> On 01/16/2012 08:40 AM, Jeremy Fitzhardinge wrote:
>>>> That means we're spinning for n cycles, then notify the spinlock holder that we'd like to get kicked and go sleeping. While I'm pretty sure that it improves the situation, it doesn't solve all of the issues we have.
>>>>
>>>> Imagine we have an idle host. All vcpus can freely run and everyone can fetch the lock as fast as on real machines. We don't need to / want to go to sleep here. Locks that take too long are bugs that need to be solved on real hw just as well, so all we do is possibly incur overhead.
>>> I'm not quite sure what your concern is. The lock is under contention, so there's nothing to do except spin; all this patch adds is a variable decrement/test to the spin loop, but that's not going to waste any more CPU than the non-counting case. And once it falls into the blocking path, its a win because the VCPU isn't burning CPU any more.
>> The wakeup path is slower though. The previous lock holder has to
>> hypercall, and the new lock holder has to be scheduled, and transition
>> from halted state to running (a vmentry). So it's only a clear win if
>> we can do something with the cpu other than go into the idle loop.
>
> Not burning power is a win too.
>
> Actually what you want is something like "if you preempt a VCPU while
> its spinning in a lock, then push it into the slowpath and don't
> reschedule it without a kick". But I think that interface would have a
> lot of fiddly corners.
>
Yes wakeup path is little slower but better than burning cpu. no?
Suppose we have 16 vcpu,
vcpu 1- lockholder (preempted).
vcpu 2-8 - in slowpath.
If scheduler schedules vcpu-1 that is most favourable for lock progress,
But if vcpu-9 - vcpu-16 OR something else scheduled, then also it's a
win right (we are doing some useful work), but yes lock progress is
again little slower though.
The optimization areas of interests are perhaps:
(1) suppose vcpu-1 is running and is about to release lock and next
vcpu in queue just goes to halt(). so this makes us to tune
SPIN_THRESHOLD rightly and have a mechanism to determine if lock-holder
is running and do continue spin. Identifying whether lock-holder is
running would be easier task and can be next step of optimization.
(2) Much talked, identifying lockholder-preemption (vcpu) and do
yield_to().
But I am not sure how complicated is yield_to() implementation once we
have identified the exact preempted vcpu (lock-holder).
> J
>
>
^ permalink raw reply
* Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
From: Srivatsa Vaddagiri @ 2012-01-18 13:54 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: X86, linux-doc, Peter Zijlstra, Jan Kiszka, Virtualization,
Paul Mackerras, H. Peter Anvin, Stefano Stabellini, Xen,
Dave Jiang, KVM, Glauber Costa, Raghavendra K T, Ingo Molnar,
Avi Kivity, Rik van Riel, Konrad Rzeszutek Wilk,
Greg Kroah-Hartman, Sasha Levin, Sedat Dilek, Thomas Gleixner,
LKML, Dave Hansen
In-Reply-To: <4F1621B2.3020203@goop.org>
* Jeremy Fitzhardinge <jeremy@goop.org> [2012-01-18 12:34:42]:
> >> What prevents a kick from being lost here, if say, the waiter is at
> >> local_irq_save in kvm_lock_spinning, before the lock/want assignments?
> > The waiter does check for lock becoming available before actually
> > sleeping:
> >
> > + /*
> > + * check again make sure it didn't become free while
> > + * we weren't looking.
> > + */
> > + if (ACCESS_ONCE(lock->tickets.head) == want) {
> > + add_stats(TAKEN_SLOW_PICKUP, 1);
> > + goto out;
> > + }
>
> That logic relies on the "kick" being level triggered, so that "kick"
> before "block" will cause the block to fall out immediately. If you're
> using "hlt" as the block and it has the usual edge-triggered behaviour,
> what stops a "kick-before-hlt" from losing the kick?
Hmm ..'hlt' should result in a check for kick request (in hypervisor
context) before vcpu is put to sleep. IOW vcpu1 that is attempting to kick vcpu0
will set a 'somebody_tried_kicking_vcpu0' flag, which hypervisor should check
before it puts vcpu0 to sleep because of trapped 'hlt' instruction.
Won't that trap the 'kick-before-hlt' case? What am I missing here?
- vatsa
^ permalink raw reply
* Re: [RFC 7/11] virtio_pci: new, capability-aware driver.
From: Michael S. Tsirkin @ 2012-01-18 15:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Pawel Moll, virtualization, Christian Borntraeger, Sasha Levin,
Anthony Liguori
In-Reply-To: <1326421962.23910.228.camel@pasglop>
On Fri, Jan 13, 2012 at 01:32:42PM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2012-01-13 at 04:19 +0200, Michael S. Tsirkin wrote:
> > On Thu, Jan 12, 2012 at 12:12:17PM +1030, Rusty Russell wrote:
> > > On Thu, 12 Jan 2012 00:02:33 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > Look, we have a race currently. Let us not tie a bug fix to a huge
> > > > rewrite with unclear performance benefits, please.
> > >
> > > In theory, yes. In practice, we bandaid it.
> > >
> > > I think in the short term we change ->get to get the entire sequence
> > > twice, and check it's the same. Theoretically, still racy, but it does
> > > cut the window. And we haven't seen the bug yet, either.
> >
> > I thought about this some more. Since we always get
> > an interrupt on config changes, it seems that a rather
> > robust method would be to just synchronize against that.
> > Something like the below (warning - completely untested).
> > Still need to think about memory barriers, overflow etc.
> > What do you think?
>
> Your interrupt may take an unpredictable amount of time to arrive, I
> don't see how you can use that as a guarantee.
>
> Cheers,
> Ben.
In theory yes.
Practically, under current KVM, what would delay it?
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> > index 03d1984..b5df385 100644
> > --- a/drivers/virtio/virtio_pci.c
> > +++ b/drivers/virtio/virtio_pci.c
> > @@ -57,6 +57,7 @@ struct virtio_pci_device
> > unsigned msix_used_vectors;
> > /* Whether we have vector per vq */
> > bool per_vq_vectors;
> > + atomic_t config_changes;
> > };
> >
> > /* Constants for MSI-X */
> > @@ -125,6 +126,19 @@ static void vp_finalize_features(struct virtio_device *vdev)
> > iowrite32(vdev->features[0], vp_dev->ioaddr+VIRTIO_PCI_GUEST_FEATURES);
> > }
> >
> > +/* wait for pending irq handlers */
> > +static void vp_synchronize_vectors(struct virtio_device *vdev)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + int i;
> > +
> > + if (vp_dev->intx_enabled)
> > + synchronize_irq(vp_dev->pci_dev->irq);
> > +
> > + for (i = 0; i < vp_dev->msix_vectors; ++i)
> > + synchronize_irq(vp_dev->msix_entries[i].vector);
> > +}
> > +
> > /* virtio config->get() implementation */
> > static void vp_get(struct virtio_device *vdev, unsigned offset,
> > void *buf, unsigned len)
> > @@ -134,9 +148,20 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
> > VIRTIO_PCI_CONFIG(vp_dev) + offset;
> > u8 *ptr = buf;
> > int i;
> > -
> > - for (i = 0; i < len; i++)
> > - ptr[i] = ioread8(ioaddr + i);
> > + int uninitialized_var(c);
> > + c = atomic_read(&vp_dev->config_changes);
> > + /* Make sure read is done before we get the first config byte */
> > + rmb();
> > + do {
> > + for (i = 0; i < len; i++)
> > + ptr[i] = ioread8(ioaddr + i);
> > + /* Synchronize with config interrupt */
> > + vp_synchronize_vectors(vdev);
> > + /*
> > + * For multi-byte fields, we might get a config change interrupt
> > + * between byte reads. If this happens, retry the read.
> > + */
> > + } while (c != atomic_read(&vp_dev->config_changes))
> > }
> >
> > /* the config->set() implementation. it's symmetric to the config->get()
> > @@ -169,19 +194,6 @@ static void vp_set_status(struct virtio_device *vdev, u8 status)
> > iowrite8(status, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
> > }
> >
> > -/* wait for pending irq handlers */
> > -static void vp_synchronize_vectors(struct virtio_device *vdev)
> > -{
> > - struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > - int i;
> > -
> > - if (vp_dev->intx_enabled)
> > - synchronize_irq(vp_dev->pci_dev->irq);
> > -
> > - for (i = 0; i < vp_dev->msix_vectors; ++i)
> > - synchronize_irq(vp_dev->msix_entries[i].vector);
> > -}
> > -
> > static void vp_reset(struct virtio_device *vdev)
> > {
> > struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > @@ -213,6 +225,8 @@ static irqreturn_t vp_config_changed(int irq, void *opaque)
> > drv = container_of(vp_dev->vdev.dev.driver,
> > struct virtio_driver, driver);
> >
> > + atomic_inc(&vp_dev->config_changes);
> > +
> > if (drv && drv->config_changed)
> > drv->config_changed(&vp_dev->vdev);
> > return IRQ_HANDLED;
> > @@ -646,6 +660,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> > vp_dev->vdev.config = &virtio_pci_config_ops;
> > vp_dev->pci_dev = pci_dev;
> > INIT_LIST_HEAD(&vp_dev->virtqueues);
> > + atomic_set(&vp_dev->config_changes, 0);
> > spin_lock_init(&vp_dev->lock);
> >
> > /* Disable MSI/MSIX to bring device to a known good state. */
>
^ permalink raw reply
* [PATCH 1/1] drivers: hid: hid-hyperv: Properly disconnect the input device
From: K. Y. Srinivasan @ 2012-01-18 16:57 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization, ohering, joe,
dmitry.torokhov, jkosina
Cc: K. Y. Srinivasan, Haiyang Zhang, stable
When we unload the mouse driver, properly disconnect the input device.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reported-by: Fuzhou Chen <fuzhouch@microsoft.com>
Cc: stable <stable@vger.kernel.org>
---
drivers/hid/hid-hyperv.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index 0c33ae9..4066324 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -548,6 +548,7 @@ static int mousevsc_remove(struct hv_device *dev)
struct mousevsc_dev *input_dev = hv_get_drvdata(dev);
vmbus_close(dev->channel);
+ hid_hw_stop(input_dev->hid_device);
hid_destroy_device(input_dev->hid_device);
mousevsc_free_device(input_dev);
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
From: Jeremy Fitzhardinge @ 2012-01-18 21:52 UTC (permalink / raw)
To: Srivatsa Vaddagiri
Cc: X86, linux-doc, Peter Zijlstra, Jan Kiszka, Virtualization,
Paul Mackerras, H. Peter Anvin, Stefano Stabellini, Xen,
Dave Jiang, KVM, Glauber Costa, Raghavendra K T, Ingo Molnar,
Avi Kivity, Rik van Riel, Konrad Rzeszutek Wilk,
Greg Kroah-Hartman, Sasha Levin, Sedat Dilek, Thomas Gleixner,
LKML, Dave Hansen
In-Reply-To: <20120118135445.GB25711@linux.vnet.ibm.com>
On 01/19/2012 12:54 AM, Srivatsa Vaddagiri wrote:
>
>> That logic relies on the "kick" being level triggered, so that "kick"
>> before "block" will cause the block to fall out immediately. If you're
>> using "hlt" as the block and it has the usual edge-triggered behaviour,
>> what stops a "kick-before-hlt" from losing the kick?
> Hmm ..'hlt' should result in a check for kick request (in hypervisor
> context) before vcpu is put to sleep. IOW vcpu1 that is attempting to kick vcpu0
> will set a 'somebody_tried_kicking_vcpu0' flag, which hypervisor should check
> before it puts vcpu0 to sleep because of trapped 'hlt' instruction.
>
> Won't that trap the 'kick-before-hlt' case? What am I missing here?
Nothing, that sounds fine. It wasn't clear to me that your kick
operation left persistent state, and so has a level-triggered effect on hlt.
J
^ permalink raw reply
* [PATCH 1/2] virtio: fix typos of memory barriers
From: Jason Wang @ 2012-01-20 8:16 UTC (permalink / raw)
To: rusty, virtualization, linux-kernel, mst; +Cc: netdev, kvm
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_ring.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 79e1b29..78428a8 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -35,7 +35,7 @@
#define virtio_rmb(vq) \
do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
#define virtio_wmb(vq) \
- do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
+ do { if ((vq)->weak_barriers) smp_wmb(); else wmb(); } while(0)
#else
/* We must force memory ordering even if guest is UP since host could be
* running on another CPU, but SMP barriers are defined to barrier() in that
^ permalink raw reply related
* [PATCH 2/2] virtio: correct the memory barrier in virtqueue_kick_prepare()
From: Jason Wang @ 2012-01-20 8:17 UTC (permalink / raw)
To: rusty, virtualization, linux-kernel, mst; +Cc: netdev, kvm
In-Reply-To: <20120120081658.50747.10625.stgit@amd-6168-8-1.englab.nay.redhat.com>
Use virtio_mb() to make sure the available index to be exposed before
checking the the avail event. Otherwise we may get stale value of
avail event in guest and never kick the host after.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_ring.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 78428a8..07d9360 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -308,9 +308,9 @@ bool virtqueue_kick_prepare(struct virtqueue *_vq)
bool needs_kick;
START_USE(vq);
- /* Descriptors and available array need to be set before we expose the
- * new available array entries. */
- virtio_wmb(vq);
+ /* We need expose available array entries before checking avail
+ * event. */
+ virtio_mb(vq);
old = vq->vring.avail->idx - vq->num_added;
new = vq->vring.avail->idx;
^ permalink raw reply related
* Re: [PATCH RFC V4 5/5] Documentation/kvm : Add documentation on Hypercalls and features used for PV spinlock
From: Srivatsa Vaddagiri @ 2012-01-20 15:09 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: Jeremy Fitzhardinge, Raghavendra K T, KVM, linux-doc,
Peter Zijlstra, Jan Kiszka, Virtualization, Anthony Liguori,
Paul Mackerras, H. Peter Anvin, Stefano Stabellini, Xen,
Dave Jiang, Glauber Costa, X86, Ingo Molnar, Avi Kivity,
Rik van Riel, Konrad Rzeszutek Wilk, Sasha Levin, Sedat Dilek,
Thomas Gleixner, Anthony Liguori, Greg
In-Reply-To: <20120117155303.GA28904@amt.cnet>
* Marcelo Tosatti <mtosatti@redhat.com> [2012-01-17 13:53:03]:
> on tue, jan 17, 2012 at 05:32:33pm +0200, gleb natapov wrote:
> > on tue, jan 17, 2012 at 07:58:18pm +0530, srivatsa vaddagiri wrote:
> > > * gleb natapov <gleb@redhat.com> [2012-01-17 15:20:51]:
> > >
> > > > > having the hypercall makes the intent of vcpu (to sleep on a kick) clear to
> > > > > hypervisor vs assuming that because of a trapped hlt instruction (which
> > > > > will anyway won't work when yield_on_hlt=0).
> > > > >
> > > > the purpose of yield_on_hlt=0 is to allow vcpu to occupy cpu for the
> > > > entire time slice no mater what. i do not think disabling yield on hlt
> > > > is even make sense in cpu oversubscribe scenario.
> > >
> > > Yes, so is there any real use for yield_on_hlt=0? I believe Anthony
> > > initially added it as a way to implement CPU bandwidth capping for VMs,
> > > which would ensure that busy VMs don't eat into cycles meant for a idle
> > > VM. Now that we have proper support in scheduler for CPU bandwidth capping, is
> > > there any real world use for yield_on_hlt=0? If not, deprecate it?
> > >
> > I was against adding it in the first place, so if IBM no longer needs it
> > I am for removing it ASAP.
>
> +1.
>
> Anthony?
CCing Anthony.
Anthony, could you ACK removal of yield_on_hlt (as keeping it around will
require unnecessary complications in pv-spinlock patches)?
- vatsa
^ permalink raw reply
* [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Akinobu Mita @ 2012-01-20 15:15 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: xen-devel, Jeremy Fitzhardinge, virtualization, Akinobu Mita,
Konrad Rzeszutek Wilk
In-Reply-To: <1327072528-8479-1-git-send-email-akinobu.mita@gmail.com>
Use bitmap_set and bitmap_clear rather than modifying individual bits
in a memory region.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: xen-devel@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
---
drivers/block/xen-blkfront.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2f22874..619868d 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -43,6 +43,7 @@
#include <linux/slab.h>
#include <linux/mutex.h>
#include <linux/scatterlist.h>
+#include <linux/bitmap.h>
#include <xen/xen.h>
#include <xen/xenbus.h>
@@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
spin_lock(&minor_lock);
if (find_next_bit(minors, end, minor) >= end) {
- for (; minor < end; ++minor)
- __set_bit(minor, minors);
+ bitmap_set(minors, minor, nr);
rc = 0;
} else
rc = -EBUSY;
@@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
BUG_ON(end > nr_minors);
spin_lock(&minor_lock);
- for (; minor < end; ++minor)
- __clear_bit(minor, minors);
+ bitmap_clear(minors, minor, nr);
spin_unlock(&minor_lock);
}
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Akinobu Mita @ 2012-01-20 15:27 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: Jeremy Fitzhardinge, xen-devel, virtualization, Akinobu Mita,
Konrad Rzeszutek Wilk
In-Reply-To: <1327072528-8479-4-git-send-email-akinobu.mita@gmail.com>
I used Jeremy Fitzhardinge's old email address...
2012/1/21 Akinobu Mita <akinobu.mita@gmail.com>:
> Use bitmap_set and bitmap_clear rather than modifying individual bits
> in a memory region.
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: xen-devel@lists.xensource.com
> Cc: virtualization@lists.linux-foundation.org
> ---
> drivers/block/xen-blkfront.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2f22874..619868d 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -43,6 +43,7 @@
> #include <linux/slab.h>
> #include <linux/mutex.h>
> #include <linux/scatterlist.h>
> +#include <linux/bitmap.h>
>
> #include <xen/xen.h>
> #include <xen/xenbus.h>
> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
>
> spin_lock(&minor_lock);
> if (find_next_bit(minors, end, minor) >= end) {
> - for (; minor < end; ++minor)
> - __set_bit(minor, minors);
> + bitmap_set(minors, minor, nr);
> rc = 0;
> } else
> rc = -EBUSY;
> @@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
>
> BUG_ON(end > nr_minors);
> spin_lock(&minor_lock);
> - for (; minor < end; ++minor)
> - __clear_bit(minor, minors);
> + bitmap_clear(minors, minor, nr);
> spin_unlock(&minor_lock);
> }
>
> --
> 1.7.4.4
>
^ permalink raw reply
* Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Konrad Rzeszutek Wilk @ 2012-01-20 15:28 UTC (permalink / raw)
To: Akinobu Mita
Cc: xen-devel, akpm, Jeremy Fitzhardinge, linux-kernel,
virtualization
In-Reply-To: <1327072528-8479-4-git-send-email-akinobu.mita@gmail.com>
On Sat, Jan 21, 2012 at 12:15:26AM +0900, Akinobu Mita wrote:
> Use bitmap_set and bitmap_clear rather than modifying individual bits
> in a memory region.
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: xen-devel@lists.xensource.com
> Cc: virtualization@lists.linux-foundation.org
> ---
> drivers/block/xen-blkfront.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2f22874..619868d 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -43,6 +43,7 @@
> #include <linux/slab.h>
> #include <linux/mutex.h>
> #include <linux/scatterlist.h>
> +#include <linux/bitmap.h>
>
> #include <xen/xen.h>
> #include <xen/xenbus.h>
> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
>
> spin_lock(&minor_lock);
> if (find_next_bit(minors, end, minor) >= end) {
> - for (; minor < end; ++minor)
> - __set_bit(minor, minors);
> + bitmap_set(minors, minor, nr);
Hm, I would have thought the last argument should have been 'end'?
Did you test this patch with a large amount of minors?
> rc = 0;
> } else
> rc = -EBUSY;
> @@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
>
> BUG_ON(end > nr_minors);
> spin_lock(&minor_lock);
> - for (; minor < end; ++minor)
> - __clear_bit(minor, minors);
> + bitmap_clear(minors, minor, nr);
Ditto.
> spin_unlock(&minor_lock);
> }
>
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Akinobu Mita @ 2012-01-20 15:41 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: akpm, xen-devel, linux-kernel,
Jeremy Fitzhardinge Jeremy Fitzhardinge, virtualization
In-Reply-To: <20120120152819.GA3959@phenom.dumpdata.com>
2012/1/21 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> On Sat, Jan 21, 2012 at 12:15:26AM +0900, Akinobu Mita wrote:
>> Use bitmap_set and bitmap_clear rather than modifying individual bits
>> in a memory region.
>>
>> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: xen-devel@lists.xensource.com
>> Cc: virtualization@lists.linux-foundation.org
>> ---
>> drivers/block/xen-blkfront.c | 7 +++----
>> 1 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
>> index 2f22874..619868d 100644
>> --- a/drivers/block/xen-blkfront.c
>> +++ b/drivers/block/xen-blkfront.c
>> @@ -43,6 +43,7 @@
>> #include <linux/slab.h>
>> #include <linux/mutex.h>
>> #include <linux/scatterlist.h>
>> +#include <linux/bitmap.h>
>>
>> #include <xen/xen.h>
>> #include <xen/xenbus.h>
>> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
>>
>> spin_lock(&minor_lock);
>> if (find_next_bit(minors, end, minor) >= end) {
>> - for (; minor < end; ++minor)
>> - __set_bit(minor, minors);
>> + bitmap_set(minors, minor, nr);
>
> Hm, I would have thought the last argument should have been 'end'?
'end' is the index of the last bit to clear. But the last argument of
bitmap_clear() is the number of bits to clear. So I think 'nr' is correct.
> Did you test this patch with a large amount of minors?
Sorry I didn't do runtime test.
>> rc = 0;
>> } else
>> rc = -EBUSY;
>> @@ -193,8 +193,7 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
>>
>> BUG_ON(end > nr_minors);
>> spin_lock(&minor_lock);
>> - for (; minor < end; ++minor)
>> - __clear_bit(minor, minors);
>> + bitmap_clear(minors, minor, nr);
>
> Ditto.
>> spin_unlock(&minor_lock);
>> }
>>
>> --
>> 1.7.4.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Konrad Rzeszutek Wilk @ 2012-01-20 16:09 UTC (permalink / raw)
To: Akinobu Mita
Cc: akpm, xen-devel, linux-kernel,
Jeremy Fitzhardinge Jeremy Fitzhardinge, virtualization
In-Reply-To: <CAC5umyi9vuOh9Bx_HoB8ieXcVjZsisKOG6mCEM2ZGL7zXCrVTQ@mail.gmail.com>
On Sat, Jan 21, 2012 at 12:41:56AM +0900, Akinobu Mita wrote:
> 2012/1/21 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> > On Sat, Jan 21, 2012 at 12:15:26AM +0900, Akinobu Mita wrote:
> >> Use bitmap_set and bitmap_clear rather than modifying individual bits
> >> in a memory region.
> >>
> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> Cc: xen-devel@lists.xensource.com
> >> Cc: virtualization@lists.linux-foundation.org
> >> ---
> >> drivers/block/xen-blkfront.c | 7 +++----
> >> 1 files changed, 3 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >> index 2f22874..619868d 100644
> >> --- a/drivers/block/xen-blkfront.c
> >> +++ b/drivers/block/xen-blkfront.c
> >> @@ -43,6 +43,7 @@
> >> #include <linux/slab.h>
> >> #include <linux/mutex.h>
> >> #include <linux/scatterlist.h>
> >> +#include <linux/bitmap.h>
> >>
> >> #include <xen/xen.h>
> >> #include <xen/xenbus.h>
> >> @@ -177,8 +178,7 @@ static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
> >>
> >> spin_lock(&minor_lock);
> >> if (find_next_bit(minors, end, minor) >= end) {
> >> - for (; minor < end; ++minor)
> >> - __set_bit(minor, minors);
> >> + bitmap_set(minors, minor, nr);
> >
> > Hm, I would have thought the last argument should have been 'end'?
>
> 'end' is the index of the last bit to clear. But the last argument of
> bitmap_clear() is the number of bits to clear. So I think 'nr' is correct.
Ah, I see it.
>
> > Did you test this patch with a large amount of minors?
>
> Sorry I didn't do runtime test.
Please do.
^ permalink raw reply
* Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Andrew Morton @ 2012-01-20 23:11 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Jeremy Fitzhardinge Jeremy Fitzhardinge, xen-devel,
virtualization, Akinobu Mita, linux-kernel
In-Reply-To: <20120120160938.GC3959@phenom.dumpdata.com>
On Fri, 20 Jan 2012 11:09:38 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> >
> > > Did you test this patch with a large amount of minors?
> >
> > Sorry I didn't do runtime test.
>
> Please do.
The poor guy probably doesn't know how to test it and surely it would
be quite a lot of work for him to do so.
Overall, it would be much more efficient if the tester of this code is
someone who is set up to easily apply the patch and test it. ie: the
code maintainer(s).
^ permalink raw reply
* Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Konrad Rzeszutek Wilk @ 2012-01-20 23:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Jeremy Fitzhardinge Jeremy Fitzhardinge, xen-devel,
virtualization, Akinobu Mita, linux-kernel
In-Reply-To: <20120120151149.04352aac.akpm@linux-foundation.org>
On Fri, Jan 20, 2012 at 03:11:49PM -0800, Andrew Morton wrote:
> On Fri, 20 Jan 2012 11:09:38 -0500
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>
> > >
> > > > Did you test this patch with a large amount of minors?
> > >
> > > Sorry I didn't do runtime test.
> >
> > Please do.
>
> The poor guy probably doesn't know how to test it and surely it would
> be quite a lot of work for him to do so.
>
> Overall, it would be much more efficient if the tester of this code is
> someone who is set up to easily apply the patch and test it. ie: the
> code maintainer(s).
<grins>
This patch aside - Andrew how do you deal with a large amount of patches
and make sure they are tested? Is there a weekly Test Tuesday where you
kick off your automated tests and dilligently look for any variations?
Or is it more of compile the kernel with X features and run it for a week
doing normal (and abnormal!) things to see if it falls over?
Thanks!
^ permalink raw reply
* [PATCH] include/checkpatch: Prefer __scanf to __attribute__((format(scanf, ...)
From: Joe Perches @ 2012-01-21 0:01 UTC (permalink / raw)
To: Andrew Morton, Konrad Rzeszutek Wilk, Jeremy Fitzhardinge,
Andy Whitcroft
Cc: xen-devel, linux-kernel, virtualization
It's equivalent to __printf, so prefer __scanf.
Signed-off-by: Joe Perches <joe@perches.com>
---
include/linux/compiler-gcc.h | 3 ++-
include/linux/kernel.h | 8 ++++----
include/xen/xenbus.h | 4 ++--
scripts/checkpatch.pl | 6 ++++++
4 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 3fd17c2..e5834aa 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -87,7 +87,8 @@
*/
#define __pure __attribute__((pure))
#define __aligned(x) __attribute__((aligned(x)))
-#define __printf(a,b) __attribute__((format(printf,a,b)))
+#define __printf(a, b) __attribute__((format(printf, a, b)))
+#define __scanf(a, b) __attribute__((format(scanf, a, b)))
#define noinline __attribute__((noinline))
#define __attribute_const__ __attribute__((__const__))
#define __maybe_unused __attribute__((unused))
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index e834342..30f21ec 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -315,10 +315,10 @@ extern __printf(2, 3)
char *kasprintf(gfp_t gfp, const char *fmt, ...);
extern char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
-extern int sscanf(const char *, const char *, ...)
- __attribute__ ((format (scanf, 2, 3)));
-extern int vsscanf(const char *, const char *, va_list)
- __attribute__ ((format (scanf, 2, 0)));
+extern __scanf(2, 3)
+int sscanf(const char *, const char *, ...);
+extern __scanf(2, 0)
+int vsscanf(const char *, const char *, va_list);
extern int get_option(char **str, int *pint);
extern char *get_options(const char *str, int nints, int *ints);
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index e8c599b..0a7515c 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -139,9 +139,9 @@ int xenbus_transaction_start(struct xenbus_transaction *t);
int xenbus_transaction_end(struct xenbus_transaction t, int abort);
/* Single read and scanf: returns -errno or num scanned if > 0. */
+__scanf(4, 5)
int xenbus_scanf(struct xenbus_transaction t,
- const char *dir, const char *node, const char *fmt, ...)
- __attribute__((format(scanf, 4, 5)));
+ const char *dir, const char *node, const char *fmt, ...);
/* Single printf and write: returns -errno or 0. */
__printf(4, 5)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e3bfcbe..2b52aeb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3117,6 +3117,12 @@ sub process {
"__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr);
}
+# Check for __attribute__ format(scanf, prefer __scanf
+ if ($line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\b/) {
+ WARN("PREFER_SCANF",
+ "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr);
+ }
+
# check for sizeof(&)
if ($line =~ /\bsizeof\s*\(\s*\&/) {
WARN("SIZEOF_ADDRESS",
--
1.7.8.111.gad25c.dirty
^ permalink raw reply related
* Re: [PATCH] xen-blkfront: use bitmap_set() and bitmap_clear()
From: Andrew Morton @ 2012-01-21 0:04 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Jeremy Fitzhardinge Jeremy Fitzhardinge, xen-devel,
virtualization, Akinobu Mita, linux-kernel
In-Reply-To: <20120120235508.GA17260@phenom.dumpdata.com>
On Fri, 20 Jan 2012 18:55:08 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Fri, Jan 20, 2012 at 03:11:49PM -0800, Andrew Morton wrote:
> > On Fri, 20 Jan 2012 11:09:38 -0500
> > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> >
> > > >
> > > > > Did you test this patch with a large amount of minors?
> > > >
> > > > Sorry I didn't do runtime test.
> > >
> > > Please do.
> >
> > The poor guy probably doesn't know how to test it and surely it would
> > be quite a lot of work for him to do so.
> >
> > Overall, it would be much more efficient if the tester of this code is
> > someone who is set up to easily apply the patch and test it. ie: the
> > code maintainer(s).
>
> <grins>
>
> This patch aside - Andrew how do you deal with a large amount of patches
> and make sure they are tested?
Not very well :(
> Is there a weekly Test Tuesday where you
> kick off your automated tests and dilligently look for any variations?
> Or is it more of compile the kernel with X features and run it for a week
> doing normal (and abnormal!) things to see if it falls over?
I build all the patches I have along with all of linux-next and boot
the thing, then use the resulting kernel for a few hours compilation
testing, then shove it all into linux-next where I naively hope that
someone else is testing things.
The coverage which is obtained this way is pretty poor.
^ permalink raw reply
* Re: [PATCH 1/2] virtio: fix typos of memory barriers
From: Michael S. Tsirkin @ 2012-01-22 11:47 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20120120081658.50747.10625.stgit@amd-6168-8-1.englab.nay.redhat.com>
On Fri, Jan 20, 2012 at 04:16:59PM +0800, Jason Wang wrote:
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Good catch.
Note: this fixes a bug introduced by
7b21e34fd1c272e3a8c3846168f2f6287a4cd72b.
It's probably a good idea to mention
this is the commit log.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/virtio/virtio_ring.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 79e1b29..78428a8 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -35,7 +35,7 @@
> #define virtio_rmb(vq) \
> do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
> #define virtio_wmb(vq) \
> - do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
> + do { if ((vq)->weak_barriers) smp_wmb(); else wmb(); } while(0)
> #else
> /* We must force memory ordering even if guest is UP since host could be
> * running on another CPU, but SMP barriers are defined to barrier() in that
^ permalink raw reply
* Re: [PATCH 2/2] virtio: correct the memory barrier in virtqueue_kick_prepare()
From: Michael S. Tsirkin @ 2012-01-22 11:47 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20120120081708.50747.60241.stgit@amd-6168-8-1.englab.nay.redhat.com>
On Fri, Jan 20, 2012 at 04:17:08PM +0800, Jason Wang wrote:
> Use virtio_mb() to make sure the available index to be exposed before
> checking the the avail event. Otherwise we may get stale value of
> avail event in guest and never kick the host after.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Good catch.
Note: this fixes a bug introduced by ee7cd8981e15bcb365fc762afe3fc47b8242f630.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/virtio/virtio_ring.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 78428a8..07d9360 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -308,9 +308,9 @@ bool virtqueue_kick_prepare(struct virtqueue *_vq)
> bool needs_kick;
>
> START_USE(vq);
> - /* Descriptors and available array need to be set before we expose the
> - * new available array entries. */
> - virtio_wmb(vq);
> + /* We need expose available array entries before checking avail
Nit:
We need expose -> Need to expose
> + * event. */
> + virtio_mb(vq);
>
> old = vq->vring.avail->idx - vq->num_added;
> new = vq->vring.avail->idx;
^ permalink raw reply
* Re: [RFC 7/11] virtio_pci: new, capability-aware driver.
From: Rusty Russell @ 2012-01-22 22:53 UTC (permalink / raw)
To: Michael S. Tsirkin, Benjamin Herrenschmidt
Cc: Christian Borntraeger, Pawel Moll, Sasha Levin, Anthony Liguori,
virtualization
In-Reply-To: <20120118152949.GA2665@redhat.com>
On Wed, 18 Jan 2012 17:29:49 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Jan 13, 2012 at 01:32:42PM +1100, Benjamin Herrenschmidt wrote:
> > Your interrupt may take an unpredictable amount of time to arrive, I
> > don't see how you can use that as a guarantee.
>
> In theory yes.
> Practically, under current KVM, what would delay it?
Indeed, if we're talking about a bandaid, this is fine. For the config
change, I do think we need something more robust.
Thanks,
Rusty.
^ 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