29.10.2021 19:38, Emanuele Giuseppe Esposito wrote:
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 patches 1-3-5-6-7, we split the job API in two headers:
job-driver.h and job-monitor.h.
As explained in job.c, job-monitor are the functions mainly used
by the monitor, and require consistency between the search of
a specific job (job_get) and the actual operation/action on it
(e.g. job_user_cancel). Therefore job-monitor API assume that
the job mutex lock is always held by the caller.
job-driver, on the other side, is the collection of functions
that are used by the job drivers or core block layer. These
functions are not aware of the job mutex, and delegate the
locking to the callee instead.
We also have job-common.h contains the job struct definition
and common functions that are not part of monitor or driver APIs.
job.h is left for legacy and to avoid changing all files that
include it.
Honestly, I don't really like the idea of splitting:
1. It's not a functional split: for some functions we have a locked version in
one header and unlocked in the other. But actually they are the same function.
I'd prefer such wrappers to live together. All the declarations in the headers
are about one thing: Job.
I think, splitting make sense when we really split things, split objects into
some separate entities. But here you just use different header to group
functions by some criteria not related to their action. I don't like it.
I think, it's enough to specify in a comment above the function, does it need locking or
not ("foo_locked" naming helps too), and different headers doesn't help to
understand code but make it more difficult.
2. I don't like file names:
"job-driver" for me sounds like something about JobDriver struct. "job-monitor"
- unclear. You define job-monitor as functions mainly used by the monitor. But actually they are
used by other code paths as well.. Also, jobs don't directly relate to monitor, they are abstract,
so no reason to establish such a relation in file names.
--
Best regards,
Vladimir
