Cc'ing Mark for macio which seems to have the same issue.
On 5/1/22 12:13, Hanna Reitz wrote:
When we still have an AIOCB registered for DMA operations, we try to
settle the respective operation by draining the BlockBackend associated
with the IDE device.
However, this assumes that every DMA
When building on macOS 12 we get:
../block/file-posix.c:3335:18: warning: 'IOMasterPort' is deprecated: first
deprecated in macOS 12.0 [-Wdeprecated-declarations]
kernResult = IOMasterPort( MACH_PORT_NULL, );
^~~~
IOMainPort
Use IOMainPort
These functions assume that the job lock is held by the
caller, to avoid TOC/TOU conditions. Therefore, their
name must end with _locked.
Introduce also additional helpers that define _locked
functions (useful when the job_mutex is globally applied).
Note: at this stage, job_{lock/unlock} and
If the job->aio_context is accessed under job_mutex,
leave as it is. Otherwise use job_get_aio_context().
Signed-off-by: Emanuele Giuseppe Esposito
---
block/commit.c | 4 ++--
block/mirror.c | 2 +-
block/replication.c | 2 +-
blockjob.c
Change the job_{lock/unlock} and macros to use job_mutex.
Now that they are not nop anymore, remove the aiocontext
to avoid deadlocks.
Therefore:
- when possible, remove completely the aiocontext lock/unlock pair
- if it is used by some other functions too, reduce the locking
section as much as
In preparation to the job_lock/unlock patch, remove these
aiocontext locks.
The main reason these two locks are removed here is because
they are inside a loop iterating on the jobs list. Once the
job_lock is added, it will have to protect the whole loop,
wrapping also the aiocontext
Introduce the job locking mechanism through the whole job API,
following the comments and requirements of job-monitor (assume
lock is held) and job-driver (lock is not held).
job_{lock/unlock} is independent from real_job_{lock/unlock}.
Note: at this stage, job_{lock/unlock} and job lock guard
Instead of having the lock in job_tnx_apply, move it inside
in the callback. This will be helpful for next commits, when
we introduce job_lock/unlock pairs.
job_transition_to_pending() and job_needs_finalize() do not
need to be protected by the aiocontext lock.
No functional change intended.
Categorize the fields in struct Job to understand which ones
need to be protected by the job mutex and which don't.
Signed-off-by: Emanuele Giuseppe Esposito
---
include/qemu/job.h | 63 +++---
1 file changed, 37 insertions(+), 26 deletions(-)
diff --git
job_event_* functions can all be static, as they are not used
outside job.c.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Stefan Hajnoczi
---
include/qemu/job.h | 6 --
job.c | 12 ++--
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git
All these functions assume that the lock is not held, and acquire
it internally.
These functions will be useful when job_lock is globally applied,
as they will allow callers to access the job struct fields
without worrying about the job lock.
Update also the comments in blockjob.c (and move them
Both blockdev.c and job-qmp.c have TOC/TOU conditions, because
they first search for the job and then perform an action on it.
Therefore, we need to do the search + action under the same
job mutex critical section.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Add missing job synchronization in the unit tests, with
both explicit locks and helpers.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito
---
tests/unit/test-bdrv-drain.c | 40 +++---
Now that we added the job_lock/unlock pairs, we can also
rename all static functions in job.c that are called with
the job mutex held as _locked(), and add a little comment
on top.
No functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito
---
blockjob.c | 8 ++
job.c |
Not sure what the atomic here was supposed to do, since job.busy
is protected by the job lock. Since the whole function
is called under job_mutex, just remove the atomic.
Signed-off-by: Emanuele Giuseppe Esposito
---
blockjob.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff
On 05.01.22 12:13, Hanna Reitz wrote:
[...]
Perhaps for a lack of being aware of all the kinds of tests we have, I
found it impossible to write a reproducer in any of our current test
frameworks: From how I understand the issue, to reproduce it, you need
to issue a TRIM request and immediately
Same as AIO_WAIT_WHILE macro, but if we are in the Main loop
do not release and then acquire ctx_ 's aiocontext.
Once all Aiocontext locks go away, this macro will replace
AIO_WAIT_WHILE.
Signed-off-by: Emanuele Giuseppe Esposito
---
include/block/aio-wait.h | 15 +++
1 file
Once job lock is used and aiocontext is removed, mirror has
to perform job operations under the same critical section,
using the helpers prepared in previous commit.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito
---
job mutex will be used to protect the job struct elements and list,
replacing AioContext locks.
Right now use a shared lock for all jobs, in order to keep things
simple. Once the AioContext lock is gone, we can introduce per-job
locks.
To simplify the switch from aiocontext to job lock,
In this series, we want to remove the AioContext lock and instead
use the already existent job_mutex to protect the job structures
and list. This is part of the work to get rid of AioContext lock
usage in favour of smaller granularity locks.
In order to simplify reviewer's job, job lock/unlock
When we still have an AIOCB registered for DMA operations, we try to
settle the respective operation by draining the BlockBackend associated
with the IDE device.
However, this assumes that every DMA operation is associated with some
I/O operation on the BlockBackend, and so settling the latter
21 matches
Mail list logo
