Re: [PATCH v2] virtio-balloon: do not call blocking ops when !TASK_RUNNING
Thomas Huth th...@linux.vnet.ibm.com writes: On Wed, 25 Feb 2015 16:11:27 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Wed, 25 Feb 2015 15:36:02 +0100 Michael S. Tsirkin m...@redhat.com wrote: virtio balloon has this code: wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update || kthread_should_stop() || freezing(current)); Which is a problem because towards_target() call might block after wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing the task_struct::state collision typical of nesting of sleeping primitives See also http://lwn.net/Articles/628628/ or Thomas's bug report http://article.gmane.org/gmane.linux.kernel.virtualization/24846 for a fuller explanation. To fix, rewrite using wait_woken. Cc: sta...@vger.kernel.org Reported-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- changes from v1: remove wait_event_interruptible noticed by Cornelia Huck cornelia.h...@de.ibm.com drivers/virtio/virtio_balloon.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) I was able to reproduce Thomas' original problem and can confirm that it is gone with this patch. Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Right, I just applied the patch on my system, too, and the problem is indeed gone! Thanks for the quick fix! Tested-by: Thomas Huth th...@linux.vnet.ibm.com Applied. Thanks, Rusty. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
Cornelia Huck cornelia.h...@de.ibm.com writes: On Wed, 4 Mar 2015 11:25:56 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Mar 04, 2015 at 04:44:54PM +1030, Rusty Russell wrote: Michael S. Tsirkin m...@redhat.com writes: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Cheers, Rusty. I agree, but I'd like to point out that even without kmalloc, on s390 get_config is blocking - it's waiting for a hardware interrupt. And it makes sense: config is not data path, I don't think we should spin there. So I think besides these two parts, we still need my two patches: virtio-balloon: do not call blocking ops when !TASK_RUNNING I prefer to annotate, over trying to fix this. Because it's not important. We might spin a few times, but it's very unlikely, and it's certainly not performance critical. Thanks, Rusty. Subject: virtio_balloon: annotate possible sleep waiting for event. CCW (s390) does this. Reported-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Rusty Russell ru...@rustcorp.com.au diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 0413157f3b49..3f4d5acdbde0 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -340,6 +340,15 @@ static int balloon(void *_vballoon) s64 diff; try_to_freeze(); + + /* + * Reading the config on the ccw backend involves an + * allocation, so we may actually sleep and have an + * extra iteration. It's extremely unlikely, Hmm, this part of the comment seems wrong to me. Reading the config on the ccw backend always sleeps because it's interrupt driven. (...) So I suspect http://mid.gmane.org/1424874878-17155-1-git-send-email-...@redhat.com is better. What do you think? I'd prefer to fix this as well. While the I/O request completes instantly on current qemu (the ssch backend handles the start function immediately, not asynchronously as on real hardware), this (a) is an implementation detail that may change and (b) doesn't account for the need to deliver the interrupt to the guest - which might take non-zero time. Ah, I see. My mistake. I've thrown out my patch, applied that one. Thanks, Rusty. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, 2 Mar 2015 21:44:10 +0100 Michael S. Tsirkin m...@redhat.com wrote: Normally, hotunplug requires guest cooperation. IOW unplug request should send guest interrupt, then block until guest confirms it's not using the device anymore. virtio pci already handles that fine, can't ccw do something similar? Hotunplug for channel devices does not require guest feedback. (In fact, I was surprised to hear that there is somthing like guest cooperation on other platforms.) Consider a storage device. If you don't flush out caches before removing the disk, you might lose a bunch of data. Yes, that is a problem. But hotunplug is indistinguishable from a hw failure on s390, so there's not really much we can do here. Basically, the guest is simply presented with the fact that the device is gone and has to deal with it. It does not matter whether the device was removed by operator request or due to a hardware failure. (We do have support in the s390 channel device core to be able to deal with devices going away and coming back gracefully. ccw devices can be put into a special state where they retain their configuration so that they can be reactivated if they become available again. For example, dasd (disk) devices survive being detached and reattached just fine, even under I/O load. See the -notify() callback of the ccw driver for details.) How does guest distinguish between this and intentional permanent removal? It can't. It will get the same kind of notifications (and channel I/O failures) for both. Only the admin has a chance of knowing, and they may kill off a device in that state permanently (which, of course, triggers the flush problems etc. which have just been delayed from the initial detach). Given that this is what the architecture gives us on all hypervisors (LPAR and z/VM) and is for all I know decades old, it is what we have to implement in qemu/kvm as well. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Wed, 4 Mar 2015 11:25:56 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Mar 04, 2015 at 04:44:54PM +1030, Rusty Russell wrote: Michael S. Tsirkin m...@redhat.com writes: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Cheers, Rusty. I agree, but I'd like to point out that even without kmalloc, on s390 get_config is blocking - it's waiting for a hardware interrupt. And it makes sense: config is not data path, I don't think we should spin there. So I think besides these two parts, we still need my two patches: virtio-balloon: do not call blocking ops when !TASK_RUNNING I prefer to annotate, over trying to fix this. Because it's not important. We might spin a few times, but it's very unlikely, and it's certainly not performance critical. Thanks, Rusty. Subject: virtio_balloon: annotate possible sleep waiting for event. CCW (s390) does this. Reported-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Rusty Russell ru...@rustcorp.com.au diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 0413157f3b49..3f4d5acdbde0 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -340,6 +340,15 @@ static int balloon(void *_vballoon) s64 diff; try_to_freeze(); + + /* +* Reading the config on the ccw backend involves an +* allocation, so we may actually sleep and have an +* extra iteration. It's extremely unlikely, Hmm, this part of the comment seems wrong to me. Reading the config on the ccw backend always sleeps because it's interrupt driven. (...) So I suspect http://mid.gmane.org/1424874878-17155-1-git-send-email-...@redhat.com is better. What do you think? I'd prefer to fix this as well. While the I/O request completes instantly on current qemu (the ssch backend handles the start function immediately, not asynchronously as on real hardware), this (a) is an implementation detail that may change and (b) doesn't account for the need to deliver the interrupt to the guest - which might take non-zero time. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
Michael S. Tsirkin m...@redhat.com writes: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Cheers, Rusty. I agree, but I'd like to point out that even without kmalloc, on s390 get_config is blocking - it's waiting for a hardware interrupt. And it makes sense: config is not data path, I don't think we should spin there. So I think besides these two parts, we still need my two patches: virtio-balloon: do not call blocking ops when !TASK_RUNNING I prefer to annotate, over trying to fix this. Because it's not important. We might spin a few times, but it's very unlikely, and it's certainly not performance critical. Thanks, Rusty. Subject: virtio_balloon: annotate possible sleep waiting for event. CCW (s390) does this. Reported-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Rusty Russell ru...@rustcorp.com.au diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 0413157f3b49..3f4d5acdbde0 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -340,6 +340,15 @@ static int balloon(void *_vballoon) s64 diff; try_to_freeze(); + + /* +* Reading the config on the ccw backend involves an +* allocation, so we may actually sleep and have an +* extra iteration. It's extremely unlikely, and this +* isn't a fast path in any sense. +*/ + sched_annotate_sleep(); + wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Wed, Mar 04, 2015 at 04:44:54PM +1030, Rusty Russell wrote:
Michael S. Tsirkin m...@redhat.com writes:
On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote:
Thomas Huth th...@linux.vnet.ibm.com writes:
On Thu, 26 Feb 2015 11:50:42 +1030
Rusty Russell ru...@rustcorp.com.au wrote:
Thomas Huth th...@linux.vnet.ibm.com writes:
Hi all,
with the recent kernel 3.19, I get a kernel warning when I start my
KVM guest on s390 with virtio balloon enabled:
The deeper problem is that virtio_ccw_get_config just silently fails on
OOM.
Neither get_config nor set_config are expected to fail.
AFAIK this is currently not a problem. According to
http://lwn.net/Articles/627419/ these kmalloc calls never
fail because they allocate less than a page.
I strongly suggest you unlearn that fact.
The fix for this is in two parts:
1) Annotate using sched_annotate_sleep() and add a comment: we may spin
a few times in low memory situations, but this isn't a high
performance path.
2) Handle get_config (and other) failure in some more elegant way.
Cheers,
Rusty.
I agree, but I'd like to point out that even without kmalloc,
on s390 get_config is blocking - it's waiting
for a hardware interrupt.
And it makes sense: config is not data path, I don't think
we should spin there.
So I think besides these two parts, we still need my two patches:
virtio-balloon: do not call blocking ops when !TASK_RUNNING
I prefer to annotate, over trying to fix this.
Because it's not important. We might spin a few times, but it's very
unlikely, and it's certainly not performance critical.
Thanks,
Rusty.
Subject: virtio_balloon: annotate possible sleep waiting for event.
CCW (s390) does this.
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0413157f3b49..3f4d5acdbde0 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -340,6 +340,15 @@ static int balloon(void *_vballoon)
s64 diff;
try_to_freeze();
+
+ /*
+ * Reading the config on the ccw backend involves an
+ * allocation, so we may actually sleep and have an
+ * extra iteration. It's extremely unlikely,
Hmm, this part of the comment seems wrong to me.
Reading the config on the ccw backend always sleeps
because it's interrupt driven.
This is the relevant code:
static int ccw_io_helper(struct virtio_ccw_device *vcdev,
struct ccw1 *ccw, __u32 intparm)
{
int ret;
unsigned long flags;
int flag = intparm VIRTIO_CCW_INTPARM_MASK;
do {
spin_lock_irqsave(get_ccwdev_lock(vcdev-cdev), flags);
ret = ccw_device_start(vcdev-cdev, ccw, intparm, 0, 0);
if (!ret) {
if (!vcdev-curr_io)
vcdev-err = 0;
vcdev-curr_io |= flag;
}
spin_unlock_irqrestore(get_ccwdev_lock(vcdev-cdev), flags);
cpu_relax();
} while (ret == -EBUSY);
wait_event(vcdev-wait_q, doing_io(vcdev, flag) == 0);
return ret ? ret : vcdev-err;
}
and this
+ * isn't a fast path in any sense.
+ */
+ sched_annotate_sleep();
+
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
So the wait_event_interruptible always calls wait_event
which then becomes a busy wait on s390, which is not nice.
So I suspect
http://mid.gmane.org/1424874878-17155-1-git-send-email-...@redhat.com
is better.
What do you think?
--
MST
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Cheers, Rusty. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, Mar 02, 2015 at 01:11:02PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:46:57 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 12:31:06PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:13:58 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Do you mean we need to enable the caller to deal with get_config failures (and the transport to relay those failures)? I agree with that. We can certainly tweak code to bypass need to kmalloc on get_config. Why is it doing these allocs? What's wrong with using vcdev-config directly? We'd need to make sure that vcdev-config is allocated with GFP_DMA, as we need it to be under 2G. I see - and that's expensive when there are many devices? One simple solution is to have a global buffer that everyone reuses. It'll need a lock, naturally. And we need to be more careful wrt serialization, Why does passing in vcdev-config mean we need to be more careful than when we allocate a buffer and then memcpy into vcdev-config? especially if we want to reuse the ccw structure as well, for example. Nothing complicated, I'd just need some free time to do it :) The more likely reason for get_config to fail is a device hotunplug, however. We'll get a seperate notification about that (via machine check + channel report), but it would be nice if we could stop poking the device immediately, as there's no use trying to do something with it anymore. -- MST -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, Mar 02, 2015 at 01:35:05PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 13:19:43 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 01:11:02PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:46:57 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 12:31:06PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:13:58 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Do you mean we need to enable the caller to deal with get_config failures (and the transport to relay those failures)? I agree with that. We can certainly tweak code to bypass need to kmalloc on get_config. Why is it doing these allocs? What's wrong with using vcdev-config directly? We'd need to make sure that vcdev-config is allocated with GFP_DMA, as we need it to be under 2G. And we need to be more careful wrt serialization, especially if we want to reuse the ccw structure as well, for example. Nothing complicated, I'd just need some free time to do it :) The more likely reason for get_config to fail is a device hotunplug, however. We'll get a seperate notification about that (via machine check + channel report), but it would be nice if we could stop poking the device immediately, as there's no use trying to do something with it anymore. Normally, hotunplug requires guest cooperation. IOW unplug request should send guest interrupt, then block until guest confirms it's not using the device anymore. virtio pci already handles that fine, can't ccw do something similar? Hotunplug for channel devices does not require guest feedback. (In fact, I was surprised to hear that there is somthing like guest cooperation on other platforms.) Consider a storage device. If you don't flush out caches before removing the disk, you might lose a bunch of data. Basically, the guest is simply presented with the fact that the device is gone and has to deal with it. It does not matter whether the device was removed by operator request or due to a hardware failure. (We do have support in the s390 channel device core to be able to deal with devices going away and coming back gracefully. ccw devices can be put into a special state where they retain their configuration so that they can be reactivated if they become available again. For example, dasd (disk) devices survive being detached and reattached just fine, even under I/O load. See the -notify() callback of the ccw driver for details.) How does guest distinguish between this and intentional permanent removal? -- MST -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, 2 Mar 2015 12:46:57 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 12:31:06PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:13:58 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Do you mean we need to enable the caller to deal with get_config failures (and the transport to relay those failures)? I agree with that. We can certainly tweak code to bypass need to kmalloc on get_config. Why is it doing these allocs? What's wrong with using vcdev-config directly? We'd need to make sure that vcdev-config is allocated with GFP_DMA, as we need it to be under 2G. And we need to be more careful wrt serialization, especially if we want to reuse the ccw structure as well, for example. Nothing complicated, I'd just need some free time to do it :) The more likely reason for get_config to fail is a device hotunplug, however. We'll get a seperate notification about that (via machine check + channel report), but it would be nice if we could stop poking the device immediately, as there's no use trying to do something with it anymore. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Cheers, Rusty. I agree, but I'd like to point out that even without kmalloc, on s390 get_config is blocking - it's waiting for a hardware interrupt. And it makes sense: config is not data path, I don't think we should spin there. So I think besides these two parts, we still need my two patches: virtio-balloon: do not call blocking ops when !TASK_RUNNING virtio_console: avoid config access from irq in 4.0. agree? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, Mar 02, 2015 at 01:11:02PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:46:57 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 12:31:06PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:13:58 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Do you mean we need to enable the caller to deal with get_config failures (and the transport to relay those failures)? I agree with that. We can certainly tweak code to bypass need to kmalloc on get_config. Why is it doing these allocs? What's wrong with using vcdev-config directly? We'd need to make sure that vcdev-config is allocated with GFP_DMA, as we need it to be under 2G. And we need to be more careful wrt serialization, especially if we want to reuse the ccw structure as well, for example. Nothing complicated, I'd just need some free time to do it :) The more likely reason for get_config to fail is a device hotunplug, however. We'll get a seperate notification about that (via machine check + channel report), but it would be nice if we could stop poking the device immediately, as there's no use trying to do something with it anymore. Normally, hotunplug requires guest cooperation. IOW unplug request should send guest interrupt, then block until guest confirms it's not using the device anymore. virtio pci already handles that fine, can't ccw do something similar? -- MST -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, Mar 02, 2015 at 12:31:06PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:13:58 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Do you mean we need to enable the caller to deal with get_config failures (and the transport to relay those failures)? I agree with that. We can certainly tweak code to bypass need to kmalloc on get_config. Why is it doing these allocs? What's wrong with using vcdev-config directly? Cheers, Rusty. I agree, but I'd like to point out that even without kmalloc, on s390 get_config is blocking - it's waiting for a hardware interrupt. And it makes sense: config is not data path, I don't think we should spin there. So I think besides these two parts, we still need my two patches: virtio-balloon: do not call blocking ops when !TASK_RUNNING virtio_console: avoid config access from irq in 4.0. agree? I agree that we need those fixes as well. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, 2 Mar 2015 13:19:43 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 01:11:02PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:46:57 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 12:31:06PM +0100, Cornelia Huck wrote: On Mon, 2 Mar 2015 12:13:58 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Do you mean we need to enable the caller to deal with get_config failures (and the transport to relay those failures)? I agree with that. We can certainly tweak code to bypass need to kmalloc on get_config. Why is it doing these allocs? What's wrong with using vcdev-config directly? We'd need to make sure that vcdev-config is allocated with GFP_DMA, as we need it to be under 2G. And we need to be more careful wrt serialization, especially if we want to reuse the ccw structure as well, for example. Nothing complicated, I'd just need some free time to do it :) The more likely reason for get_config to fail is a device hotunplug, however. We'll get a seperate notification about that (via machine check + channel report), but it would be nice if we could stop poking the device immediately, as there's no use trying to do something with it anymore. Normally, hotunplug requires guest cooperation. IOW unplug request should send guest interrupt, then block until guest confirms it's not using the device anymore. virtio pci already handles that fine, can't ccw do something similar? Hotunplug for channel devices does not require guest feedback. (In fact, I was surprised to hear that there is somthing like guest cooperation on other platforms.) Basically, the guest is simply presented with the fact that the device is gone and has to deal with it. It does not matter whether the device was removed by operator request or due to a hardware failure. (We do have support in the s390 channel device core to be able to deal with devices going away and coming back gracefully. ccw devices can be put into a special state where they retain their configuration so that they can be reactivated if they become available again. For example, dasd (disk) devices survive being detached and reattached just fine, even under I/O load. See the -notify() callback of the ccw driver for details.) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Mon, 2 Mar 2015 12:13:58 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. I strongly suggest you unlearn that fact. The fix for this is in two parts: 1) Annotate using sched_annotate_sleep() and add a comment: we may spin a few times in low memory situations, but this isn't a high performance path. 2) Handle get_config (and other) failure in some more elegant way. Do you mean we need to enable the caller to deal with get_config failures (and the transport to relay those failures)? I agree with that. Cheers, Rusty. I agree, but I'd like to point out that even without kmalloc, on s390 get_config is blocking - it's waiting for a hardware interrupt. And it makes sense: config is not data path, I don't think we should spin there. So I think besides these two parts, we still need my two patches: virtio-balloon: do not call blocking ops when !TASK_RUNNING virtio_console: avoid config access from irq in 4.0. agree? I agree that we need those fixes as well. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, Feb 26, 2015 at 09:30:31AM +0100, Michael S. Tsirkin wrote: On Thu, Feb 26, 2015 at 11:50:42AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. Cornelia, I think ccw and config_area should be allocated inside vcdev. You could either use pointers, or simply allocate vcdev with GDP_DMA. This would avoid the kmalloc inside these calls. Thanks, Rusty. But it won't solve the problem of nested sleepers with ccw: ATM is invokes ccw_io_helper to execute commands, and that one calls wait_event to wait for an interrupt. Might be fixable but I think my patch looks like a safer solution for 4.0/3.19, no? I've no idea what your patch was since I'm not subscribed to any of the lists this discussion is had on. But you can annotate the warning away; _however_ with the annotation needs to be a big comment explaining why its safe to do so. Typically to involved talking about how its actually rare for the call to sleep. So occasional sleeps inside a wait_event() are ok-ish, we'll just get to go around once more. But once you consistently sleep inside a wait_event() things go a bit funny. So for instance; if in ccw_io_helper() we expect that wait_event(, !doing_io()) to be (mostly) true on first go, then we'll never get into __wait_event() and -state won't actually be mucked about with. The thing to avoid is not actually sleeping (much) but setting TASK_RUNNING and turning the entire thing into a giant poll loop. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, Feb 26, 2015 at 06:08:49PM +0100, Peter Zijlstra wrote:
On Thu, Feb 26, 2015 at 09:30:31AM +0100, Michael S. Tsirkin wrote:
On Thu, Feb 26, 2015 at 11:50:42AM +1030, Rusty Russell wrote:
Thomas Huth th...@linux.vnet.ibm.com writes:
Hi all,
with the recent kernel 3.19, I get a kernel warning when I start my
KVM guest on s390 with virtio balloon enabled:
The deeper problem is that virtio_ccw_get_config just silently fails on
OOM.
Neither get_config nor set_config are expected to fail.
Cornelia, I think ccw and config_area should be allocated inside vcdev.
You could either use pointers, or simply allocate vcdev with GDP_DMA.
This would avoid the kmalloc inside these calls.
Thanks,
Rusty.
But it won't solve the problem of nested sleepers
with ccw: ATM is invokes ccw_io_helper to execute
commands, and that one calls wait_event
to wait for an interrupt.
Might be fixable but I think my patch looks like a safer
solution for 4.0/3.19, no?
I've no idea what your patch was since I'm not subscribed to any of the
lists this discussion is had on.
Oh, sorry about that.
Here it is, below:
- Forwarded message from Michael S. Tsirkin m...@redhat.com -
Date: Wed, 25 Feb 2015 15:36:02 +0100
From: Michael S. Tsirkin m...@redhat.com
To: linux-ker...@vger.kernel.org
Cc: sta...@vger.kernel.org, Thomas Huth th...@linux.vnet.ibm.com, Rusty
Russell ru...@rustcorp.com.au,
virtualizat...@lists.linux-foundation.org, kvm@vger.kernel.org,
Cornelia Huck cornelia.h...@de.ibm.com
Subject: [PATCH v2] virtio-balloon: do not call blocking ops when !TASK_RUNNING
Message-ID: 1424874878-17155-1-git-send-email-...@redhat.com
virtio balloon has this code:
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
|| kthread_should_stop()
|| freezing(current));
Which is a problem because towards_target() call might block after
wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing
the task_struct::state collision typical of nesting of sleeping
primitives
See also http://lwn.net/Articles/628628/ or Thomas's
bug report
http://article.gmane.org/gmane.linux.kernel.virtualization/24846
for a fuller explanation.
To fix, rewrite using wait_woken.
Cc: sta...@vger.kernel.org
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
changes from v1:
remove wait_event_interruptible
noticed by Cornelia Huck cornelia.h...@de.ibm.com
drivers/virtio/virtio_balloon.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0413157..5a6ad6d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,6 +29,7 @@
#include linux/module.h
#include linux/balloon_compaction.h
#include linux/oom.h
+#include linux/wait.h
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -334,17 +335,25 @@ static int virtballoon_oom_notify(struct notifier_block
*self,
static int balloon(void *_vballoon)
{
struct virtio_balloon *vb = _vballoon;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
set_freezable();
while (!kthread_should_stop()) {
s64 diff;
try_to_freeze();
- wait_event_interruptible(vb-config_change,
-(diff = towards_target(vb)) != 0
-|| vb-need_stats_update
-|| kthread_should_stop()
-|| freezing(current));
+
+ add_wait_queue(vb-config_change, wait);
+ for (;;) {
+ if ((diff = towards_target(vb)) != 0 ||
+ vb-need_stats_update ||
+ kthread_should_stop() ||
+ freezing(current))
+ break;
+ wait_woken(wait, TASK_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
+ }
+ remove_wait_queue(vb-config_change, wait);
+
if (vb-need_stats_update)
stats_handle_request(vb);
if (diff 0)
--
MST
- End forwarded message -
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, Feb 26, 2015 at 06:27:43PM +0100, Michael S. Tsirkin wrote:
On Thu, Feb 26, 2015 at 06:08:49PM +0100, Peter Zijlstra wrote:
On Thu, Feb 26, 2015 at 09:30:31AM +0100, Michael S. Tsirkin wrote:
On Thu, Feb 26, 2015 at 11:50:42AM +1030, Rusty Russell wrote:
Thomas Huth th...@linux.vnet.ibm.com writes:
Hi all,
with the recent kernel 3.19, I get a kernel warning when I start my
KVM guest on s390 with virtio balloon enabled:
The deeper problem is that virtio_ccw_get_config just silently fails on
OOM.
Neither get_config nor set_config are expected to fail.
Cornelia, I think ccw and config_area should be allocated inside vcdev.
You could either use pointers, or simply allocate vcdev with GDP_DMA.
This would avoid the kmalloc inside these calls.
Thanks,
Rusty.
But it won't solve the problem of nested sleepers
with ccw: ATM is invokes ccw_io_helper to execute
commands, and that one calls wait_event
to wait for an interrupt.
Might be fixable but I think my patch looks like a safer
solution for 4.0/3.19, no?
I've no idea what your patch was since I'm not subscribed to any of the
lists this discussion is had on.
Oh, sorry about that.
Here it is, below:
- Forwarded message from Michael S. Tsirkin m...@redhat.com -
Date: Wed, 25 Feb 2015 15:36:02 +0100
From: Michael S. Tsirkin m...@redhat.com
To: linux-ker...@vger.kernel.org
Cc: sta...@vger.kernel.org, Thomas Huth th...@linux.vnet.ibm.com, Rusty
Russell ru...@rustcorp.com.au,
virtualizat...@lists.linux-foundation.org, kvm@vger.kernel.org,
Cornelia Huck cornelia.h...@de.ibm.com
Subject: [PATCH v2] virtio-balloon: do not call blocking ops when
!TASK_RUNNING
Message-ID: 1424874878-17155-1-git-send-email-...@redhat.com
virtio balloon has this code:
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
|| kthread_should_stop()
|| freezing(current));
Which is a problem because towards_target() call might block after
wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing
the task_struct::state collision typical of nesting of sleeping
primitives
See also http://lwn.net/Articles/628628/ or Thomas's
bug report
http://article.gmane.org/gmane.linux.kernel.virtualization/24846
for a fuller explanation.
To fix, rewrite using wait_woken.
Cc: sta...@vger.kernel.org
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
changes from v1:
remove wait_event_interruptible
noticed by Cornelia Huck cornelia.h...@de.ibm.com
drivers/virtio/virtio_balloon.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0413157..5a6ad6d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,6 +29,7 @@
#include linux/module.h
#include linux/balloon_compaction.h
#include linux/oom.h
+#include linux/wait.h
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -334,17 +335,25 @@ static int virtballoon_oom_notify(struct notifier_block
*self,
static int balloon(void *_vballoon)
{
struct virtio_balloon *vb = _vballoon;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
set_freezable();
while (!kthread_should_stop()) {
s64 diff;
try_to_freeze();
- wait_event_interruptible(vb-config_change,
- (diff = towards_target(vb)) != 0
- || vb-need_stats_update
- || kthread_should_stop()
- || freezing(current));
+
+ add_wait_queue(vb-config_change, wait);
+ for (;;) {
+ if ((diff = towards_target(vb)) != 0 ||
+ vb-need_stats_update ||
+ kthread_should_stop() ||
+ freezing(current))
+ break;
+ wait_woken(wait, TASK_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
+ }
+ remove_wait_queue(vb-config_change, wait);
+
if (vb-need_stats_update)
stats_handle_request(vb);
if (diff 0)
--
MST
WRT which, I have a question. IIUC it's OK for towards_target
in this code to call wait_event in its turn, assuming that
*that* wait_event is not not calling blocking ops.
Right?
- End forwarded message -
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, Feb 26, 2015 at 11:50:42AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Same problem with virtio_ccw_reset. But avoiding kmalloc calls in virtio_ccw_get_config isn't enough I think, it might still sleep. Neither get_config nor set_config are expected to fail. Cornelia, I think ccw and config_area should be allocated inside vcdev. You could either use pointers, or simply allocate vcdev with GDP_DMA. This would avoid the kmalloc inside these calls. Thanks, Rusty. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. It is a problem that we cannot relay failures back to the caller: not only for the memory allocations. We need to do channel I/O, and any channel I/O can fail. For our virtio case, we don't have to deal with the failures that may happen on real hardware (like path failures), but what can happen is a hotunplug, which means we cannot talk to the device anymore from one moment to the other. Cornelia, I think ccw and config_area should be allocated inside vcdev. You could either use pointers, or simply allocate vcdev with GDP_DMA. This would avoid the kmalloc inside these calls. I can certainly look into that, but I'm not sure it's worth it. We still have to deal with possible failures from doing channel I/O. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, Feb 26, 2015 at 11:50:42AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. Cornelia, I think ccw and config_area should be allocated inside vcdev. You could either use pointers, or simply allocate vcdev with GDP_DMA. This would avoid the kmalloc inside these calls. Thanks, Rusty. But it won't solve the problem of nested sleepers with ccw: ATM is invokes ccw_io_helper to execute commands, and that one calls wait_event to wait for an interrupt. Might be fixable but I think my patch looks like a safer solution for 4.0/3.19, no? -- MST -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, 26 Feb 2015 09:45:29 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 26, 2015 at 11:50:42AM +1030, Rusty Russell wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Same problem with virtio_ccw_reset. But avoiding kmalloc calls in virtio_ccw_get_config isn't enough I think, it might still sleep. It is probably a problem with all calls into the transport that assume an implementation that cannot fail: If we have a channel I/O backing, we need to be able to handle things not working. The only case we need to care about for virtio is probably a -ENODEV triggered by a hotunplug, though. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] virtio-balloon: do not call blocking ops when !TASK_RUNNING
On Wed, Feb 25, 2015 at 03:32:08PM +0100, Cornelia Huck wrote:
On Wed, 25 Feb 2015 15:14:36 +0100
Michael S. Tsirkin m...@redhat.com wrote:
virtio balloon has this code:
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
|| kthread_should_stop()
|| freezing(current));
Which is a problem because towards_target() call might block after
wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing
the task_struct::state collision typical of nesting of sleeping
primitives
See also http://lwn.net/Articles/628628/ or Thomas's
bug report
http://article.gmane.org/gmane.linux.kernel.virtualization/24846
for a fuller explanation.
To fix, rewrite using wait_woken.
Cc: sta...@vger.kernel.org
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/virtio/virtio_balloon.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
index 0413157..2f19f65 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,6 +29,7 @@
#include linux/module.h
#include linux/balloon_compaction.h
#include linux/oom.h
+#include linux/wait.h
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -334,12 +335,25 @@ static int virtballoon_oom_notify(struct
notifier_block *self,
static int balloon(void *_vballoon)
{
struct virtio_balloon *vb = _vballoon;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
set_freezable();
while (!kthread_should_stop()) {
s64 diff;
try_to_freeze();
+
+ add_wait_queue(vb-config_change, wait);
+ for (;;) {
+ if ((diff = towards_target(vb)) != 0 ||
+ vb-need_stats_update ||
+ kthread_should_stop() ||
+ freezing(current))
+ break;
+ wait_woken(wait, TASK_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
+ }
+ remove_wait_queue(vb-config_change, wait);
+
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
Forgot to remove the wait_event_interruptible()?
Ugh. Forgot to commit :(
Will resend.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] virtio-balloon: do not call blocking ops when !TASK_RUNNING
On Wed, 25 Feb 2015 15:14:36 +0100
Michael S. Tsirkin m...@redhat.com wrote:
virtio balloon has this code:
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
|| kthread_should_stop()
|| freezing(current));
Which is a problem because towards_target() call might block after
wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing
the task_struct::state collision typical of nesting of sleeping
primitives
See also http://lwn.net/Articles/628628/ or Thomas's
bug report
http://article.gmane.org/gmane.linux.kernel.virtualization/24846
for a fuller explanation.
To fix, rewrite using wait_woken.
Cc: sta...@vger.kernel.org
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/virtio/virtio_balloon.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0413157..2f19f65 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,6 +29,7 @@
#include linux/module.h
#include linux/balloon_compaction.h
#include linux/oom.h
+#include linux/wait.h
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -334,12 +335,25 @@ static int virtballoon_oom_notify(struct notifier_block
*self,
static int balloon(void *_vballoon)
{
struct virtio_balloon *vb = _vballoon;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
set_freezable();
while (!kthread_should_stop()) {
s64 diff;
try_to_freeze();
+
+ add_wait_queue(vb-config_change, wait);
+ for (;;) {
+ if ((diff = towards_target(vb)) != 0 ||
+ vb-need_stats_update ||
+ kthread_should_stop() ||
+ freezing(current))
+ break;
+ wait_woken(wait, TASK_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
+ }
+ remove_wait_queue(vb-config_change, wait);
+
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
Forgot to remove the wait_event_interruptible()?
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] virtio-balloon: do not call blocking ops when !TASK_RUNNING
virtio balloon has this code:
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
|| kthread_should_stop()
|| freezing(current));
Which is a problem because towards_target() call might block after
wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing
the task_struct::state collision typical of nesting of sleeping
primitives
See also http://lwn.net/Articles/628628/ or Thomas's
bug report
http://article.gmane.org/gmane.linux.kernel.virtualization/24846
for a fuller explanation.
To fix, rewrite using wait_woken.
Cc: sta...@vger.kernel.org
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
changes from v1:
remove wait_event_interruptible
noticed by Cornelia Huck cornelia.h...@de.ibm.com
drivers/virtio/virtio_balloon.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0413157..5a6ad6d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,6 +29,7 @@
#include linux/module.h
#include linux/balloon_compaction.h
#include linux/oom.h
+#include linux/wait.h
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -334,17 +335,25 @@ static int virtballoon_oom_notify(struct notifier_block
*self,
static int balloon(void *_vballoon)
{
struct virtio_balloon *vb = _vballoon;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
set_freezable();
while (!kthread_should_stop()) {
s64 diff;
try_to_freeze();
- wait_event_interruptible(vb-config_change,
-(diff = towards_target(vb)) != 0
-|| vb-need_stats_update
-|| kthread_should_stop()
-|| freezing(current));
+
+ add_wait_queue(vb-config_change, wait);
+ for (;;) {
+ if ((diff = towards_target(vb)) != 0 ||
+ vb-need_stats_update ||
+ kthread_should_stop() ||
+ freezing(current))
+ break;
+ wait_woken(wait, TASK_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
+ }
+ remove_wait_queue(vb-config_change, wait);
+
if (vb-need_stats_update)
stats_handle_request(vb);
if (diff 0)
--
MST
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] virtio-balloon: do not call blocking ops when !TASK_RUNNING
virtio balloon has this code:
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
|| kthread_should_stop()
|| freezing(current));
Which is a problem because towards_target() call might block after
wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing
the task_struct::state collision typical of nesting of sleeping
primitives
See also http://lwn.net/Articles/628628/ or Thomas's
bug report
http://article.gmane.org/gmane.linux.kernel.virtualization/24846
for a fuller explanation.
To fix, rewrite using wait_woken.
Cc: sta...@vger.kernel.org
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/virtio/virtio_balloon.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0413157..2f19f65 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,6 +29,7 @@
#include linux/module.h
#include linux/balloon_compaction.h
#include linux/oom.h
+#include linux/wait.h
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -334,12 +335,25 @@ static int virtballoon_oom_notify(struct notifier_block
*self,
static int balloon(void *_vballoon)
{
struct virtio_balloon *vb = _vballoon;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
set_freezable();
while (!kthread_should_stop()) {
s64 diff;
try_to_freeze();
+
+ add_wait_queue(vb-config_change, wait);
+ for (;;) {
+ if ((diff = towards_target(vb)) != 0 ||
+ vb-need_stats_update ||
+ kthread_should_stop() ||
+ freezing(current))
+ break;
+ wait_woken(wait, TASK_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
+ }
+ remove_wait_queue(vb-config_change, wait);
+
wait_event_interruptible(vb-config_change,
(diff = towards_target(vb)) != 0
|| vb-need_stats_update
--
MST
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Wed, Feb 25, 2015 at 11:13:18AM +0100, Thomas Huth wrote: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: [0.839687] do not call blocking ops when !TASK_RUNNING; state=1 set at [00174a1e] prepare_to_wait_event+0x7e/0x108 [0.839694] [ cut here ] [0.839697] WARNING: at kernel/sched/core.c:7326 [0.839698] Modules linked in: [0.839702] CPU: 0 PID: 46 Comm: vballoon Not tainted 3.19.0 #233 [0.839705] task: 021d ti: 021d8000 task.ti: 021d8000 [0.839707] Krnl PSW : 0704c0018000 0015bf8e (__might_sleep+0x8e/0x98) [0.839713]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 EA:3 Krnl GPRS: 000d 021d 0071 0001 [0.839718]00675ace 01998c50 [0.839720]00982134 0058f824 00a008a8 [0.839722]04d9 007ea992 0015bf8a 021dbc28 [0.839731] Krnl Code: 0015bf7e: c0200033e838 larl %r2,7d8fee 0015bf84: c0e50028cd62 brasl %r14,675a48 #0015bf8a: a7f40001 brc 15,15bf8c 0015bf8e: 9201a000 mvi 0(%r10),1 0015bf92: a7f4ffe2 brc 15,15bf56 0015bf96: 0707 bcr 0,%r7 0015bf98: ebdff0800024 stmg%r13,%r15,128(%r15) 0015bf9e: a7f13fe0 tmll%r15,16352 [0.839749] Call Trace: [0.839751] ([0015bf8a] __might_sleep+0x8a/0x98) [0.839756] [0028a562] __kmalloc+0x272/0x350 [0.839759] [0058f824] virtio_ccw_get_config+0x3c/0x100 [0.839762] [0049fcb0] balloon+0x1b8/0x330 [0.839765] [001529c8] kthread+0x120/0x138 [0.839767] [00683c22] kernel_thread_starter+0x6/0xc [0.839770] [00683c1c] kernel_thread_starter+0x0/0xc [0.839772] no locks held by vballoon/46. [0.839773] Last Breaking-Event-Address: [0.839776] [0015bf8a] __might_sleep+0x8a/0x98 [0.839778] ---[ end trace d27fcdfa27273d7c ]--- The problem seems to be this code in balloon() in drivers/virtio/virtio_balloon.c: wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update || kthread_should_stop() || freezing(current)); wait_event_interruptible() sets the state of the current task to TASK_INTERRUPTIBLE, then checks the condition. The condition contains towards_target() which reads the virtio config space via virtio_cread(). On s390, this then triggers virtio_ccw_get_config() - and this function calls some other functions again that might sleep (e.g. kzalloc or wait_event in ccw_io_helper) ... and this causes the new kernel warning message with kernel 3.19. I think it would be quite difficult or at least ugly to rewrite virtio_ccw_get_config() so that it does not call sleepable functions anymore. So would it be feasible to rewrite the balloon() function that it does not call the towards_target() in its wait_event condition anymore? I am unfortunately not that familiar with the balloon code semantics, so any help is very appreciated here! Thanks, Thomas Thanks for finding this! I just sent a patch that should fix this problem: http://article.gmane.org/gmane.linux.kernel.virtualization/24851 Testing would be appreciated. Thanks again! -- MST -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] virtio-balloon: do not call blocking ops when !TASK_RUNNING
On Wed, 25 Feb 2015 16:11:27 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Wed, 25 Feb 2015 15:36:02 +0100 Michael S. Tsirkin m...@redhat.com wrote: virtio balloon has this code: wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update || kthread_should_stop() || freezing(current)); Which is a problem because towards_target() call might block after wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing the task_struct::state collision typical of nesting of sleeping primitives See also http://lwn.net/Articles/628628/ or Thomas's bug report http://article.gmane.org/gmane.linux.kernel.virtualization/24846 for a fuller explanation. To fix, rewrite using wait_woken. Cc: sta...@vger.kernel.org Reported-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- changes from v1: remove wait_event_interruptible noticed by Cornelia Huck cornelia.h...@de.ibm.com drivers/virtio/virtio_balloon.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) I was able to reproduce Thomas' original problem and can confirm that it is gone with this patch. Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Right, I just applied the patch on my system, too, and the problem is indeed gone! Thanks for the quick fix! Tested-by: Thomas Huth th...@linux.vnet.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] virtio-balloon: do not call blocking ops when !TASK_RUNNING
On Wed, 25 Feb 2015 15:36:02 +0100 Michael S. Tsirkin m...@redhat.com wrote: virtio balloon has this code: wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update || kthread_should_stop() || freezing(current)); Which is a problem because towards_target() call might block after wait_event_interruptible sets task state to TAST_INTERRUPTIBLE, causing the task_struct::state collision typical of nesting of sleeping primitives See also http://lwn.net/Articles/628628/ or Thomas's bug report http://article.gmane.org/gmane.linux.kernel.virtualization/24846 for a fuller explanation. To fix, rewrite using wait_woken. Cc: sta...@vger.kernel.org Reported-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- changes from v1: remove wait_event_interruptible noticed by Cornelia Huck cornelia.h...@de.ibm.com drivers/virtio/virtio_balloon.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) I was able to reproduce Thomas' original problem and can confirm that it is gone with this patch. Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. Cornelia, I think ccw and config_area should be allocated inside vcdev. You could either use pointers, or simply allocate vcdev with GDP_DMA. This would avoid the kmalloc inside these calls. Thanks, Rusty. [0.839687] do not call blocking ops when !TASK_RUNNING; state=1 set at [00174a1e] prepare_to_wait_event+0x7e/0x108 [0.839694] [ cut here ] [0.839697] WARNING: at kernel/sched/core.c:7326 [0.839698] Modules linked in: [0.839702] CPU: 0 PID: 46 Comm: vballoon Not tainted 3.19.0 #233 [0.839705] task: 021d ti: 021d8000 task.ti: 021d8000 [0.839707] Krnl PSW : 0704c0018000 0015bf8e (__might_sleep+0x8e/0x98) [0.839713]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 EA:3 Krnl GPRS: 000d 021d 0071 0001 [0.839718]00675ace 01998c50 [0.839720]00982134 0058f824 00a008a8 [0.839722]04d9 007ea992 0015bf8a 021dbc28 [0.839731] Krnl Code: 0015bf7e: c0200033e838 larl %r2,7d8fee 0015bf84: c0e50028cd62 brasl %r14,675a48 #0015bf8a: a7f40001 brc 15,15bf8c 0015bf8e: 9201a000 mvi 0(%r10),1 0015bf92: a7f4ffe2 brc 15,15bf56 0015bf96: 0707 bcr 0,%r7 0015bf98: ebdff0800024 stmg%r13,%r15,128(%r15) 0015bf9e: a7f13fe0 tmll%r15,16352 [0.839749] Call Trace: [0.839751] ([0015bf8a] __might_sleep+0x8a/0x98) [0.839756] [0028a562] __kmalloc+0x272/0x350 [0.839759] [0058f824] virtio_ccw_get_config+0x3c/0x100 [0.839762] [0049fcb0] balloon+0x1b8/0x330 [0.839765] [001529c8] kthread+0x120/0x138 [0.839767] [00683c22] kernel_thread_starter+0x6/0xc [0.839770] [00683c1c] kernel_thread_starter+0x0/0xc [0.839772] no locks held by vballoon/46. [0.839773] Last Breaking-Event-Address: [0.839776] [0015bf8a] __might_sleep+0x8a/0x98 [0.839778] ---[ end trace d27fcdfa27273d7c ]--- The problem seems to be this code in balloon() in drivers/virtio/virtio_balloon.c: wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update || kthread_should_stop() || freezing(current)); wait_event_interruptible() sets the state of the current task to TASK_INTERRUPTIBLE, then checks the condition. The condition contains towards_target() which reads the virtio config space via virtio_cread(). On s390, this then triggers virtio_ccw_get_config() - and this function calls some other functions again that might sleep (e.g. kzalloc or wait_event in ccw_io_helper) ... and this causes the new kernel warning message with kernel 3.19. I think it would be quite difficult or at least ugly to rewrite virtio_ccw_get_config() so that it does not call sleepable functions anymore. So would it be feasible to rewrite the balloon() function that it does not call the towards_target() in its wait_event condition anymore? I am unfortunately not that familiar with the balloon code semantics, so any help is very appreciated here! Thanks, Thomas -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Thu, 26 Feb 2015 11:50:42 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Thomas Huth th...@linux.vnet.ibm.com writes: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: The deeper problem is that virtio_ccw_get_config just silently fails on OOM. Neither get_config nor set_config are expected to fail. AFAIK this is currently not a problem. According to http://lwn.net/Articles/627419/ these kmalloc calls never fail because they allocate less than a page. Thomas -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
virtio balloon: do not call blocking ops when !TASK_RUNNING
Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: [0.839687] do not call blocking ops when !TASK_RUNNING; state=1 set at [00174a1e] prepare_to_wait_event+0x7e/0x108 [0.839694] [ cut here ] [0.839697] WARNING: at kernel/sched/core.c:7326 [0.839698] Modules linked in: [0.839702] CPU: 0 PID: 46 Comm: vballoon Not tainted 3.19.0 #233 [0.839705] task: 021d ti: 021d8000 task.ti: 021d8000 [0.839707] Krnl PSW : 0704c0018000 0015bf8e (__might_sleep+0x8e/0x98) [0.839713]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 EA:3 Krnl GPRS: 000d 021d 0071 0001 [0.839718]00675ace 01998c50 [0.839720]00982134 0058f824 00a008a8 [0.839722]04d9 007ea992 0015bf8a 021dbc28 [0.839731] Krnl Code: 0015bf7e: c0200033e838larl %r2,7d8fee 0015bf84: c0e50028cd62 brasl %r14,675a48 #0015bf8a: a7f40001 brc 15,15bf8c 0015bf8e: 9201a000 mvi 0(%r10),1 0015bf92: a7f4ffe2 brc 15,15bf56 0015bf96: 0707 bcr 0,%r7 0015bf98: ebdff0800024 stmg%r13,%r15,128(%r15) 0015bf9e: a7f13fe0 tmll%r15,16352 [0.839749] Call Trace: [0.839751] ([0015bf8a] __might_sleep+0x8a/0x98) [0.839756] [0028a562] __kmalloc+0x272/0x350 [0.839759] [0058f824] virtio_ccw_get_config+0x3c/0x100 [0.839762] [0049fcb0] balloon+0x1b8/0x330 [0.839765] [001529c8] kthread+0x120/0x138 [0.839767] [00683c22] kernel_thread_starter+0x6/0xc [0.839770] [00683c1c] kernel_thread_starter+0x0/0xc [0.839772] no locks held by vballoon/46. [0.839773] Last Breaking-Event-Address: [0.839776] [0015bf8a] __might_sleep+0x8a/0x98 [0.839778] ---[ end trace d27fcdfa27273d7c ]--- The problem seems to be this code in balloon() in drivers/virtio/virtio_balloon.c: wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update || kthread_should_stop() || freezing(current)); wait_event_interruptible() sets the state of the current task to TASK_INTERRUPTIBLE, then checks the condition. The condition contains towards_target() which reads the virtio config space via virtio_cread(). On s390, this then triggers virtio_ccw_get_config() - and this function calls some other functions again that might sleep (e.g. kzalloc or wait_event in ccw_io_helper) ... and this causes the new kernel warning message with kernel 3.19. I think it would be quite difficult or at least ugly to rewrite virtio_ccw_get_config() so that it does not call sleepable functions anymore. So would it be feasible to rewrite the balloon() function that it does not call the towards_target() in its wait_event condition anymore? I am unfortunately not that familiar with the balloon code semantics, so any help is very appreciated here! Thanks, Thomas -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio balloon: do not call blocking ops when !TASK_RUNNING
On Wed, 25 Feb 2015 11:13:18 +0100 Thomas Huth th...@linux.vnet.ibm.com wrote: Hi all, with the recent kernel 3.19, I get a kernel warning when I start my KVM guest on s390 with virtio balloon enabled: [0.839687] do not call blocking ops when !TASK_RUNNING; state=1 set at [00174a1e] prepare_to_wait_event+0x7e/0x108 [0.839694] [ cut here ] [0.839697] WARNING: at kernel/sched/core.c:7326 [0.839698] Modules linked in: [0.839702] CPU: 0 PID: 46 Comm: vballoon Not tainted 3.19.0 #233 [0.839705] task: 021d ti: 021d8000 task.ti: 021d8000 [0.839707] Krnl PSW : 0704c0018000 0015bf8e (__might_sleep+0x8e/0x98) [0.839713]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 EA:3 Krnl GPRS: 000d 021d 0071 0001 [0.839718]00675ace 01998c50 [0.839720]00982134 0058f824 00a008a8 [0.839722]04d9 007ea992 0015bf8a 021dbc28 [0.839731] Krnl Code: 0015bf7e: c0200033e838 larl %r2,7d8fee 0015bf84: c0e50028cd62 brasl %r14,675a48 #0015bf8a: a7f40001 brc 15,15bf8c 0015bf8e: 9201a000 mvi 0(%r10),1 0015bf92: a7f4ffe2 brc 15,15bf56 0015bf96: 0707 bcr 0,%r7 0015bf98: ebdff0800024 stmg%r13,%r15,128(%r15) 0015bf9e: a7f13fe0 tmll%r15,16352 [0.839749] Call Trace: [0.839751] ([0015bf8a] __might_sleep+0x8a/0x98) [0.839756] [0028a562] __kmalloc+0x272/0x350 [0.839759] [0058f824] virtio_ccw_get_config+0x3c/0x100 [0.839762] [0049fcb0] balloon+0x1b8/0x330 [0.839765] [001529c8] kthread+0x120/0x138 [0.839767] [00683c22] kernel_thread_starter+0x6/0xc [0.839770] [00683c1c] kernel_thread_starter+0x0/0xc [0.839772] no locks held by vballoon/46. [0.839773] Last Breaking-Event-Address: [0.839776] [0015bf8a] __might_sleep+0x8a/0x98 [0.839778] ---[ end trace d27fcdfa27273d7c ]--- The problem seems to be this code in balloon() in drivers/virtio/virtio_balloon.c: wait_event_interruptible(vb-config_change, (diff = towards_target(vb)) != 0 || vb-need_stats_update || kthread_should_stop() || freezing(current)); wait_event_interruptible() sets the state of the current task to TASK_INTERRUPTIBLE, then checks the condition. The condition contains towards_target() which reads the virtio config space via virtio_cread(). On s390, this then triggers virtio_ccw_get_config() - and this function calls some other functions again that might sleep (e.g. kzalloc or wait_event in ccw_io_helper) ... and this causes the new kernel warning message with kernel 3.19. I think it would be quite difficult or at least ugly to rewrite virtio_ccw_get_config() so that it does not call sleepable functions anymore. Yes: The config-space interacting functions for virtio-ccw trigger channel I/O, which is by nature asynchronous. No way to get this non-sleeping without really ugly hacks. So would it be feasible to rewrite the balloon() function that it does not call the towards_target() in its wait_event condition anymore? I am unfortunately not that familiar with the balloon code semantics, so any help is very appreciated here! It might be possible to use nested wait event functions like wake_woken(), but I haven't looked into that deeply. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
