| Message ID | 20200326155628.859862-2-s.reiter@proxmox.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix some AIO context locking in jobs | expand |
26.03.2020 18:56, Stefan Reiter wrote: > All code-paths leading to backup_clean (via job_clean) have the job's > context already acquired. The job's context is guaranteed to be the same > as the one used by backup_top via backup_job_create. As we already discussed, this is not quite right. So, may be this patch should be the last one... > > Since the previous logic effectively acquired the lock twice, this > broke cleanup of backups for disks using IO threads, since the BDRV_POLL_WHILE > in bdrv_backup_top_drop -> bdrv_do_drained_begin would only release the lock > once, thus deadlocking with the IO thread. > > This is a partial revert of 0abf2581717a19. > > Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> > --- > block/backup.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index 7430ca5883..a7a7dcaf4c 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -126,11 +126,7 @@ static void backup_abort(Job *job) > static void backup_clean(Job *job) > { > BackupBlockJob *s = container_of(job, BackupBlockJob, common.job); > - AioContext *aio_context = bdrv_get_aio_context(s->backup_top); > - > - aio_context_acquire(aio_context); > bdrv_backup_top_drop(s->backup_top); > - aio_context_release(aio_context); > } > > void backup_do_checkpoint(BlockJob *job, Error **errp) >
diff --git a/block/backup.c b/block/backup.c index 7430ca5883..a7a7dcaf4c 100644 --- a/block/backup.c +++ b/block/backup.c @@ -126,11 +126,7 @@ static void backup_abort(Job *job) static void backup_clean(Job *job) { BackupBlockJob *s = container_of(job, BackupBlockJob, common.job); - AioContext *aio_context = bdrv_get_aio_context(s->backup_top); - - aio_context_acquire(aio_context); bdrv_backup_top_drop(s->backup_top); - aio_context_release(aio_context); } void backup_do_checkpoint(BlockJob *job, Error **errp)
All code-paths leading to backup_clean (via job_clean) have the job's context already acquired. The job's context is guaranteed to be the same as the one used by backup_top via backup_job_create. Since the previous logic effectively acquired the lock twice, this broke cleanup of backups for disks using IO threads, since the BDRV_POLL_WHILE in bdrv_backup_top_drop -> bdrv_do_drained_begin would only release the lock once, thus deadlocking with the IO thread. This is a partial revert of 0abf2581717a19. Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> --- block/backup.c | 4 ---- 1 file changed, 4 deletions(-)