blockjob: Remove the job from the list earlier in block_job_unref()

[Qemu-devel] [PATCH for-2.11] blockjob: Remove the job from the list earlier in block_job_unref()

Posted by Alberto Garcia 6 years, 4 months ago

When destroying a block job in block_job_unref() we should remove it
from the job list before calling block_job_remove_all_bdrv().

This is because removing the BDSs can trigger an aio_poll() and wake
up other jobs that might attempt to use the block job list. If that
happens the job we're currently destroying should not be in that list
anymore.
---
 blockjob.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blockjob.c b/blockjob.c
index ff9a614531..2f0cc1528b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -152,6 +152,7 @@ void block_job_unref(BlockJob *job)
 {
     if (--job->refcnt == 0) {
         BlockDriverState *bs = blk_bs(job->blk);
+        QLIST_REMOVE(job, job_list);
         bs->job = NULL;
         block_job_remove_all_bdrv(job);
         blk_remove_aio_context_notifier(job->blk,
@@ -160,7 +161,6 @@ void block_job_unref(BlockJob *job)
         blk_unref(job->blk);
         error_free(job->blocker);
         g_free(job->id);
-        QLIST_REMOVE(job, job_list);
         g_free(job);
     }
 }
-- 
2.11.0

Re: [Qemu-devel] [PATCH for-2.11] blockjob: Remove the job from the list earlier in block_job_unref()

Posted by Alberto Garcia 6 years, 4 months ago

On Tue 28 Nov 2017 03:53:27 PM CET, Alberto Garcia wrote:
> When destroying a block job in block_job_unref() we should remove it
> from the job list before calling block_job_remove_all_bdrv().
>
> This is because removing the BDSs can trigger an aio_poll() and wake
> up other jobs that might attempt to use the block job list. If that
> happens the job we're currently destroying should not be in that list
> anymore.
> ---
>  blockjob.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I forgot this

Signed-off-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH for-2.11] blockjob: Remove the job from the list earlier in block_job_unref()

Posted by Kevin Wolf 6 years, 4 months ago

Am 28.11.2017 um 16:53 hat Alberto Garcia geschrieben:
> On Tue 28 Nov 2017 03:53:27 PM CET, Alberto Garcia wrote:
> > When destroying a block job in block_job_unref() we should remove it
> > from the job list before calling block_job_remove_all_bdrv().
> >
> > This is because removing the BDSs can trigger an aio_poll() and wake
> > up other jobs that might attempt to use the block job list. If that
> > happens the job we're currently destroying should not be in that list
> > anymore.
> > ---
> >  blockjob.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> I forgot this
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Thanks, applied to the block branch.

Kevin