[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 09/21] jobs: use job locks also in the unit tests
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v9 09/21] jobs: use job locks also in the unit tests |
Date: |
Mon, 11 Jul 2022 16:08:40 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 |
On 7/6/22 23:15, Emanuele Giuseppe Esposito wrote:
Add missing job synchronization in the unit tests, with
explicit locks.
We are deliberately using _locked functions wrapped by a guard
instead of a normal call because the normal call will be removed
in future, as the only usage is limited to the tests.
In other words, if a function like job_pause() is/will be only used
in tests to avoid:
WITH_JOB_LOCK_GUARD(){
job_pause_locked();
}
then it is not worth keeping job_pause(), and just use the guard.
I'm not sure. Why not worth keeping simpler code in tests? But I don't insist.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[..]
--- a/tests/unit/test-blockjob-txn.c
+++ b/tests/unit/test-blockjob-txn.c
@@ -116,8 +116,10 @@ static void test_single_job(int expected)
job = test_block_job_start(1, true, expected, &result, txn);
job_start(&job->job);
- if (expected == -ECANCELED) {
- job_cancel(&job->job, false);
+ WITH_JOB_LOCK_GUARD() {
+ if (expected == -ECANCELED) {
+ job_cancel_locked(&job->job, false);
+ }
}
"if (expected == ..)" may be kept around LOCK_GUARD section.
while (result == -EINPROGRESS) {
@@ -160,13 +162,15 @@ static void test_pair_jobs(int expected1, int expected2)
/* Release our reference now to trigger as many nice
* use-after-free bugs as possible.
*/
- job_txn_unref(txn);
[..]
cancel_common(s);
}
+/*
+ * This test always runs in the main loop, so there is no
+ * need to protect job->status.
+ */
That made me ask:
1. Are all tests always run in main loop? If yes, why to protect status reading
in test_complete_in_standby() ?
2. Maybe, we don't need to protect anything here? Why to protect other things
if we run everything in main loop?
--
Best regards,
Vladimir
- Re: [PATCH v9 11/21] jobs: group together API calls under the same job lock, (continued)
[PATCH v9 03/21] job.c: API functions not used outside should be static, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 06/21] job: move and update comments from blockjob.c, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 17/21] blockjob.h: categorize fields in struct BlockJob, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 01/21] job.c: make job_mutex and job_lock/unlock() public, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 09/21] jobs: use job locks also in the unit tests, Emanuele Giuseppe Esposito, 2022/07/06
- Re: [PATCH v9 09/21] jobs: use job locks also in the unit tests,
Vladimir Sementsov-Ogievskiy <=
[PATCH v9 20/21] blockjob: remove unused functions, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 10/21] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 15/21] job.c: enable job lock/unlock and remove Aiocontext locks, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 21/21] job: remove unused functions, Emanuele Giuseppe Esposito, 2022/07/06