"Michael S. Tsirkin" <mst@xxxxxxxxxx> writes: > On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: >> Thomas Huth <thuth@xxxxxxxxxxxxxxxxxx> writes: >> > On Thu, 26 Feb 2015 11:50:42 +1030 >> > Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote: >> > >> >> Thomas Huth <thuth@xxxxxxxxxxxxxxxxxx> 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 <thuth@xxxxxxxxxxxxxxxxxx> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
