All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: Anton Nefedov <anton.nefedov@virtuozzo.com>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com
Subject: Re: [Qemu-devel] [Qemu-block] segfault in parallel blockjobs (iotest 30)
Date: Wed, 08 Nov 2017 15:45:38 +0100	[thread overview]
Message-ID: <w51inekzv59.fsf@maestria.local.igalia.com> (raw)
In-Reply-To: <dd350838-9132-e787-10b1-4cb200076ec9@virtuozzo.com>

On Tue 07 Nov 2017 05:19:41 PM CET, Anton Nefedov wrote:
> BlockBackend gets deleted by another job's stream_complete(), deferred
> to the main loop, so the fact that the job is put to sleep by
> bdrv_drain_all_begin() doesn't really stop it from execution.

I was debugging this a bit, and the block_job_defer_to_main_loop() call
happens _after_ all jobs have been paused, so I think that when the BDS
is drained then stream_run() finishes the last iteration without
checking if it's paused.

Without your patch (i.e. with a smaller STREAM_BUFFER_SIZE) then I
assume that the function would have to continue looping and
block_job_sleep_ns() would make the job coroutine yield, effectively
pausing the job and preventing the crash.

I can fix the crash by adding block_job_pause_point(&s->common) at the
end of stream_run() (where the 'out' label is).

I'm thinking that perhaps we should add the pause point directly to
block_job_defer_to_main_loop(), to prevent any block job from running
the exit function when it's paused.

Somehow I had the impression that we discussed this already in the past
(?) because I remember thinking about this very scenario.

Berto

  parent reply	other threads:[~2017-11-08 14:45 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-07 16:19 [Qemu-devel] segfault in parallel blockjobs (iotest 30) Anton Nefedov
2017-11-08 13:58 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2017-11-08 14:45 ` Alberto Garcia [this message]
2017-11-08 15:50   ` Anton Nefedov
2017-11-09  6:05     ` Fam Zheng
2017-11-09 10:03       ` Alberto Garcia
2017-11-09 16:26   ` Alberto Garcia
2017-11-10  3:02     ` Fam Zheng
2017-11-15 15:42       ` Alberto Garcia
2017-11-15 16:31         ` Anton Nefedov
2017-11-16 15:54           ` Alberto Garcia
2017-11-16 16:09             ` Anton Nefedov
2017-11-21 12:51               ` Alberto Garcia
2017-11-21 15:18                 ` Anton Nefedov
2017-11-21 15:31                   ` Alberto Garcia
2017-11-22  0:13                     ` John Snow
2017-11-22 12:55                   ` Alberto Garcia
2017-11-22 15:58                     ` John Snow
2017-11-16 21:56             ` John Snow
2017-11-17 16:16               ` Alberto Garcia
2017-11-22 15:05 ` Alberto Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=w51inekzv59.fsf@maestria.local.igalia.com \
    --to=berto@igalia.com \
    --cc=anton.nefedov@virtuozzo.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.