[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] segfault in parallel blockjobs (iotest 30)

From: Anton Nefedov
Subject: Re: [Qemu-devel] [Qemu-block] segfault in parallel blockjobs (iotest 30)
Date: Tue, 21 Nov 2017 18:18:13 +0300
User-agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 21/11/2017 3:51 PM, Alberto Garcia wrote:
On Thu 16 Nov 2017 05:09:59 PM CET, Anton Nefedov wrote:
I have the impression that one major source of headaches is the
fact that the reopen queue contains nodes that don't need to be
reopened at all. Ideally this should be detected early on in
bdrv_reopen_queue(), so there's no chance that the queue contains
nodes used by a different block job. If we had that then op
blockers should be enough to prevent these things. Or am I missing

After applying Max's patch I tried the similar approach; that is
keep BDSes referenced while they are in the reopen queue.  Now I get
the stream job hanging. Somehow one blk_root_drained_begin() is not
paired with blk_root_drained_end(). So the job stays paused.

I can see this if I apply Max's patch and keep refs to BDSs in the
reopen queue:

#0  block_job_pause (...) at blockjob.c:130
#1  0x000055c143cb586d in block_job_drained_begin (...) at blockjob.c:227
#2  0x000055c143d08067 in blk_set_dev_ops (...) at block/block-backend.c:887
#3  0x000055c143cb69db in block_job_create (...) at blockjob.c:678
#4  0x000055c143d17c0c in mirror_start_job (...) at block/mirror.c:1177

There's a ops->drained_begin(opaque) call in blk_set_dev_ops() that
doesn't seem to be paired.

My understanding for now is

    1. in bdrv_drain_all_begin(), BDS gets drained with
       bdrv_parent_drained_begin(), all the parents get
       blk_root_drained_begin(), pause their jobs.
    2. one of the jobs finishes and deletes BB.
    3. in bdrv_drain_all_end(), bdrv_parent_drained_end() is never
       called because even though BDS still exists (referenced in the
       queue), it cannot be accessed as bdrv_next() takes BDS from the
       global BB list (and the corresponding BB is deleted).

Yes, I was debugging this and I got to a similar conclusion. With
test_stream_commit from iotest 030 I can see that

    1. the block-stream job is created first, then stream_run begins and
       starts copying the data.
    2. block-commit starts and tries to reopen the top image in
       read-write mode. This pauses the stream block job and drains all
    3. The draining causes the stream job to finish, it is deferred to
       the main loop, then stream_complete finishes and unrefs the block
       job, deleting it. At the point of deletion the pause count was
       still > 0 (because of step (2))

Max's patch v1 could have helped:

This doesn't solve the problem.

Or, perhaps another approach, keep BlockJob referenced while it is
paused (by block_job_pause/resume_all()). That should prevent it from
deleting the BB.

Yes, I tried this and it actually solves the issue. But I still think
that the problem is that block jobs are allowed to finish when they are

Agree, but

Adding block_job_pause_point(&s->common) at the end of stream_run()
fixes the problem too.

would be a nice fix, but it only works unless the job is already
deferred, right? And the BH can't be held off while job->paused because
with mirror job it is always the case.
So we cover most of the cases but also lose the stable reproduction.


>> 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]