All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: Anton Nefedov <anton.nefedov@virtuozzo.com>, Fam Zheng <famz@redhat.com>
Cc: qemu-devel@nongnu.org, kwolf@redhat.com, qemu-block@nongnu.org,
	mreitz@redhat.com, John Snow <jsnow@redhat.com>
Subject: Re: [Qemu-devel] [Qemu-block] segfault in parallel blockjobs (iotest 30)
Date: Wed, 22 Nov 2017 13:55:17 +0100	[thread overview]
Message-ID: <w51tvxmscay.fsf@maestria.local.igalia.com> (raw)
In-Reply-To: <160cac7d-7dc6-2daf-5299-82a57bffe14c@virtuozzo.com>

On Tue 21 Nov 2017 04:18:13 PM CET, Anton Nefedov wrote:
>  >> keep BlockJob referenced while it is
>  >> paused (by block_job_pause/resume_all()). That should prevent it from
>  >> deleting the BB.
>
> looks kind of hacky; maybe referencing in block_job_pause() (and not
> just pause_all) seems more correct? I think it didn't work for me
> right away though. But I can look more.

This fixes one crash for me (but only the test_stream_commit, it doesn't
fix iotest 030 completely), but I agree it looks kind of hacky.

Peharps replacing block_job_next() with QLIST_FOREACH_SAFE() is a good
idea, though, I guess resuming a block job can theoretically lead to its
destruction?

Berto

diff --git a/blockjob.c b/blockjob.c
index ff9a614531..53cffc7980 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -699,6 +699,7 @@ void block_job_pause_all(void)
         AioContext *aio_context = blk_get_aio_context(job->blk);
 
         aio_context_acquire(aio_context);
+        block_job_ref(job);
         block_job_pause(job);
         aio_context_release(aio_context);
     }
@@ -759,12 +760,14 @@ void coroutine_fn block_job_pause_point(BlockJob *job)
 
 void block_job_resume_all(void)
 {
-    BlockJob *job = NULL;
-    while ((job = block_job_next(job))) {
+    BlockJob *job, *next;
+
+    QLIST_FOREACH_SAFE(job, &block_jobs, job_list, next) {
         AioContext *aio_context = blk_get_aio_context(job->blk);
 
         aio_context_acquire(aio_context);
         block_job_resume(job);
+        block_job_unref(job);
         aio_context_release(aio_context);
     }
 }

  parent reply	other threads:[~2017-11-22 12:55 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
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 [this message]
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=w51tvxmscay.fsf@maestria.local.igalia.com \
    --to=berto@igalia.com \
    --cc=anton.nefedov@virtuozzo.com \
    --cc=famz@redhat.com \
    --cc=jsnow@redhat.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.