From patchwork Fri Oct 29 16:39:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548084 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=aHDOkNK8; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpB06B1nz9sX3 for ; Sat, 30 Oct 2021 03:42:16 +1100 (AEDT) Received: from localhost ([::1]:44384 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgUxK-0000J3-Ik for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:42:14 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48510) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUur-0006cY-Mj for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:41 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:48207) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUuk-0002vD-PC for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:41 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525574; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6EWbEdDizuzRhdfh6MYRjs0bUL6SJ7uHkWqEPubIj88=; b=aHDOkNK8IjPYzkKYUWs9LLIbPuJh30Ld1BfeyaATQKjH/dI4MqYrevepOPcR4cAnATwDJo OSZT3m8+d7aQYxp/CTt4LtiUWKCAet4teoU0liG9vHZoE2Lvzlv/KeUcB02dXROXjtpWTM U246GE39VZj8iJyM0mB+vMvdO8pC03E= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-48-7ApLgTcEPPSvjEmpy6KpPQ-1; Fri, 29 Oct 2021 12:39:32 -0400 X-MC-Unique: 7ApLgTcEPPSvjEmpy6KpPQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C1DE5802B78; Fri, 29 Oct 2021 16:39:31 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4A25B69C83; Fri, 29 Oct 2021 16:39:28 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 01/15] jobs: add job-common.h Date: Fri, 29 Oct 2021 12:39:00 -0400 Message-Id: <20211029163914.4044794-2-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" job-common.h contains all struct and common function that currently are in job.h and will be shared by job-monitor and job-driver in the next commits. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- include/qemu/job-common.h | 300 ++++++++++++++++++++++++++++++++++++++ include/qemu/job.h | 271 +--------------------------------- 2 files changed, 301 insertions(+), 270 deletions(-) create mode 100644 include/qemu/job-common.h diff --git a/include/qemu/job-common.h b/include/qemu/job-common.h new file mode 100644 index 0000000000..c115028e33 --- /dev/null +++ b/include/qemu/job-common.h @@ -0,0 +1,300 @@ +/* + * Declarations for background jobs + * + * Copyright (c) 2011 IBM Corp. + * Copyright (c) 2012, 2018 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef JOB_COMMON_H +#define JOB_COMMON_H + +#include "qapi/qapi-types-job.h" +#include "qemu/queue.h" +#include "qemu/progress_meter.h" +#include "qemu/coroutine.h" +#include "block/aio.h" + +typedef struct JobDriver JobDriver; +typedef struct JobTxn JobTxn; + + +/** + * Long-running operation. + */ +typedef struct Job { + /** The ID of the job. May be NULL for internal jobs. */ + char *id; + + /** The type of this job. */ + const JobDriver *driver; + + /** Reference count of the block job */ + int refcnt; + + /** Current state; See @JobStatus for details. */ + JobStatus status; + + /** AioContext to run the job coroutine in */ + AioContext *aio_context; + + /** + * The coroutine that executes the job. If not NULL, it is reentered when + * busy is false and the job is cancelled. + */ + Coroutine *co; + + /** + * Timer that is used by @job_sleep_ns. Accessed under job_mutex (in + * job.c). + */ + QEMUTimer sleep_timer; + + /** + * Counter for pause request. If non-zero, the block job is either paused, + * or if busy == true will pause itself as soon as possible. + */ + int pause_count; + + /** + * Set to false by the job while the coroutine has yielded and may be + * re-entered by job_enter(). There may still be I/O or event loop activity + * pending. Accessed under block_job_mutex (in blockjob.c). + * + * When the job is deferred to the main loop, busy is true as long as the + * bottom half is still pending. + */ + bool busy; + + /** + * Set to true by the job while it is in a quiescent state, where + * no I/O or event loop activity is pending. + */ + bool paused; + + /** + * Set to true if the job is paused by user. Can be unpaused with the + * block-job-resume QMP command. + */ + bool user_paused; + + /** + * Set to true if the job should cancel itself. The flag must + * always be tested just before toggling the busy flag from false + * to true. After a job has been cancelled, it should only yield + * if #aio_poll will ("sooner or later") reenter the coroutine. + */ + bool cancelled; + + /** + * Set to true if the job should abort immediately without waiting + * for data to be in sync. + */ + bool force_cancel; + + /** Set to true when the job has deferred work to the main loop. */ + bool deferred_to_main_loop; + + /** True if this job should automatically finalize itself */ + bool auto_finalize; + + /** True if this job should automatically dismiss itself */ + bool auto_dismiss; + + ProgressMeter progress; + + /** + * Return code from @run and/or @prepare callback(s). + * Not final until the job has reached the CONCLUDED status. + * 0 on success, -errno on failure. + */ + int ret; + + /** + * Error object for a failed job. + * If job->ret is nonzero and an error object was not set, it will be set + * to strerror(-job->ret) during job_completed. + */ + Error *err; + + /** The completion function that will be called when the job completes. */ + BlockCompletionFunc *cb; + + /** The opaque value that is passed to the completion function. */ + void *opaque; + + /** Notifiers called when a cancelled job is finalised */ + NotifierList on_finalize_cancelled; + + /** Notifiers called when a successfully completed job is finalised */ + NotifierList on_finalize_completed; + + /** Notifiers called when the job transitions to PENDING */ + NotifierList on_pending; + + /** Notifiers called when the job transitions to READY */ + NotifierList on_ready; + + /** Notifiers called when the job coroutine yields or terminates */ + NotifierList on_idle; + + /** Element of the list of jobs */ + QLIST_ENTRY(Job) job_list; + + /** Transaction this job is part of */ + JobTxn *txn; + + /** Element of the list of jobs in a job transaction */ + QLIST_ENTRY(Job) txn_list; +} Job; + +/** + * Callbacks and other information about a Job driver. + */ +struct JobDriver { + + /* Fields initialized in struct definition and never changed. */ + + /** Derived Job struct size */ + size_t instance_size; + + /** Enum describing the operation */ + JobType job_type; + + /* + * Functions run without regard to the BQL and may run in any + * arbitrary thread. These functions do not need to be thread-safe + * because the caller ensures that are invoked from one thread at time. + */ + + /** + * Mandatory: Entrypoint for the Coroutine. + * + * This callback will be invoked when moving from CREATED to RUNNING. + * + * If this callback returns nonzero, the job transaction it is part of is + * aborted. If it returns zero, the job moves into the WAITING state. If it + * is the last job to complete in its transaction, all jobs in the + * transaction move from WAITING to PENDING. + */ + int coroutine_fn (*run)(Job *job, Error **errp); + + /** + * If the callback is not NULL, it will be invoked when the job transitions + * into the paused state. Paused jobs must not perform any asynchronous + * I/O or event loop activity. This callback is used to quiesce jobs. + */ + void coroutine_fn (*pause)(Job *job); + + /** + * If the callback is not NULL, it will be invoked when the job transitions + * out of the paused state. Any asynchronous I/O or event loop activity + * should be restarted from this callback. + */ + void coroutine_fn (*resume)(Job *job); + + /* + * Global state (GS) API. These functions run under the BQL lock. + * + * See include/block/block-global-state.h for more information about + * the GS API. + */ + + /** + * Called when the job is resumed by the user (i.e. user_paused becomes + * false). .user_resume is called before .resume. + */ + void (*user_resume)(Job *job); + + /** + * Optional callback for job types whose completion must be triggered + * manually. + */ + void (*complete)(Job *job, Error **errp); + + /** + * If the callback is not NULL, prepare will be invoked when all the jobs + * belonging to the same transaction complete; or upon this job's completion + * if it is not in a transaction. + * + * This callback will not be invoked if the job has already failed. + * If it fails, abort and then clean will be called. + */ + int (*prepare)(Job *job); + + /** + * If the callback is not NULL, it will be invoked when all the jobs + * belonging to the same transaction complete; or upon this job's + * completion if it is not in a transaction. Skipped if NULL. + * + * All jobs will complete with a call to either .commit() or .abort() but + * never both. + */ + void (*commit)(Job *job); + + /** + * If the callback is not NULL, it will be invoked when any job in the + * same transaction fails; or upon this job's failure (due to error or + * cancellation) if it is not in a transaction. Skipped if NULL. + * + * All jobs will complete with a call to either .commit() or .abort() but + * never both. + */ + void (*abort)(Job *job); + + /** + * If the callback is not NULL, it will be invoked after a call to either + * .commit() or .abort(). Regardless of which callback is invoked after + * completion, .clean() will always be called, even if the job does not + * belong to a transaction group. + */ + void (*clean)(Job *job); + + /** + * If the callback is not NULL, it will be invoked in job_cancel_async + * + * This function must return true if the job will be cancelled + * immediately without any further I/O (mandatory if @force is + * true), and false otherwise. This lets the generic job layer + * know whether a job has been truly (force-)cancelled, or whether + * it is just in a special completion mode (like mirror after + * READY). + * (If the callback is NULL, the job is assumed to terminate + * without I/O.) + */ + bool (*cancel)(Job *job, bool force); + + + /** Called when the job is freed */ + void (*free)(Job *job); +}; + +typedef enum JobCreateFlags { + /* Default behavior */ + JOB_DEFAULT = 0x00, + /* Job is not QMP-created and should not send QMP events */ + JOB_INTERNAL = 0x01, + /* Job requires manual finalize step */ + JOB_MANUAL_FINALIZE = 0x02, + /* Job requires manual dismiss step */ + JOB_MANUAL_DISMISS = 0x04, +} JobCreateFlags; + +#endif diff --git a/include/qemu/job.h b/include/qemu/job.h index 7e9e59f4b8..3cfd79088c 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -26,276 +26,7 @@ #ifndef JOB_H #define JOB_H -#include "qapi/qapi-types-job.h" -#include "qemu/queue.h" -#include "qemu/progress_meter.h" -#include "qemu/coroutine.h" -#include "block/aio.h" - -typedef struct JobDriver JobDriver; -typedef struct JobTxn JobTxn; - - -/** - * Long-running operation. - */ -typedef struct Job { - /** The ID of the job. May be NULL for internal jobs. */ - char *id; - - /** The type of this job. */ - const JobDriver *driver; - - /** Reference count of the block job */ - int refcnt; - - /** Current state; See @JobStatus for details. */ - JobStatus status; - - /** AioContext to run the job coroutine in */ - AioContext *aio_context; - - /** - * The coroutine that executes the job. If not NULL, it is reentered when - * busy is false and the job is cancelled. - */ - Coroutine *co; - - /** - * Timer that is used by @job_sleep_ns. Accessed under job_mutex (in - * job.c). - */ - QEMUTimer sleep_timer; - - /** - * Counter for pause request. If non-zero, the block job is either paused, - * or if busy == true will pause itself as soon as possible. - */ - int pause_count; - - /** - * Set to false by the job while the coroutine has yielded and may be - * re-entered by job_enter(). There may still be I/O or event loop activity - * pending. Accessed under block_job_mutex (in blockjob.c). - * - * When the job is deferred to the main loop, busy is true as long as the - * bottom half is still pending. - */ - bool busy; - - /** - * Set to true by the job while it is in a quiescent state, where - * no I/O or event loop activity is pending. - */ - bool paused; - - /** - * Set to true if the job is paused by user. Can be unpaused with the - * block-job-resume QMP command. - */ - bool user_paused; - - /** - * Set to true if the job should cancel itself. The flag must - * always be tested just before toggling the busy flag from false - * to true. After a job has been cancelled, it should only yield - * if #aio_poll will ("sooner or later") reenter the coroutine. - */ - bool cancelled; - - /** - * Set to true if the job should abort immediately without waiting - * for data to be in sync. - */ - bool force_cancel; - - /** Set to true when the job has deferred work to the main loop. */ - bool deferred_to_main_loop; - - /** True if this job should automatically finalize itself */ - bool auto_finalize; - - /** True if this job should automatically dismiss itself */ - bool auto_dismiss; - - ProgressMeter progress; - - /** - * Return code from @run and/or @prepare callback(s). - * Not final until the job has reached the CONCLUDED status. - * 0 on success, -errno on failure. - */ - int ret; - - /** - * Error object for a failed job. - * If job->ret is nonzero and an error object was not set, it will be set - * to strerror(-job->ret) during job_completed. - */ - Error *err; - - /** The completion function that will be called when the job completes. */ - BlockCompletionFunc *cb; - - /** The opaque value that is passed to the completion function. */ - void *opaque; - - /** Notifiers called when a cancelled job is finalised */ - NotifierList on_finalize_cancelled; - - /** Notifiers called when a successfully completed job is finalised */ - NotifierList on_finalize_completed; - - /** Notifiers called when the job transitions to PENDING */ - NotifierList on_pending; - - /** Notifiers called when the job transitions to READY */ - NotifierList on_ready; - - /** Notifiers called when the job coroutine yields or terminates */ - NotifierList on_idle; - - /** Element of the list of jobs */ - QLIST_ENTRY(Job) job_list; - - /** Transaction this job is part of */ - JobTxn *txn; - - /** Element of the list of jobs in a job transaction */ - QLIST_ENTRY(Job) txn_list; -} Job; - -/** - * Callbacks and other information about a Job driver. - */ -struct JobDriver { - - /* Fields initialized in struct definition and never changed. */ - - /** Derived Job struct size */ - size_t instance_size; - - /** Enum describing the operation */ - JobType job_type; - - /* - * Functions run without regard to the BQL and may run in any - * arbitrary thread. These functions do not need to be thread-safe - * because the caller ensures that are invoked from one thread at time. - */ - - /** - * Mandatory: Entrypoint for the Coroutine. - * - * This callback will be invoked when moving from CREATED to RUNNING. - * - * If this callback returns nonzero, the job transaction it is part of is - * aborted. If it returns zero, the job moves into the WAITING state. If it - * is the last job to complete in its transaction, all jobs in the - * transaction move from WAITING to PENDING. - */ - int coroutine_fn (*run)(Job *job, Error **errp); - - /** - * If the callback is not NULL, it will be invoked when the job transitions - * into the paused state. Paused jobs must not perform any asynchronous - * I/O or event loop activity. This callback is used to quiesce jobs. - */ - void coroutine_fn (*pause)(Job *job); - - /** - * If the callback is not NULL, it will be invoked when the job transitions - * out of the paused state. Any asynchronous I/O or event loop activity - * should be restarted from this callback. - */ - void coroutine_fn (*resume)(Job *job); - - /* - * Global state (GS) API. These functions run under the BQL lock. - * - * See include/block/block-global-state.h for more information about - * the GS API. - */ - - /** - * Called when the job is resumed by the user (i.e. user_paused becomes - * false). .user_resume is called before .resume. - */ - void (*user_resume)(Job *job); - - /** - * Optional callback for job types whose completion must be triggered - * manually. - */ - void (*complete)(Job *job, Error **errp); - - /** - * If the callback is not NULL, prepare will be invoked when all the jobs - * belonging to the same transaction complete; or upon this job's completion - * if it is not in a transaction. - * - * This callback will not be invoked if the job has already failed. - * If it fails, abort and then clean will be called. - */ - int (*prepare)(Job *job); - - /** - * If the callback is not NULL, it will be invoked when all the jobs - * belonging to the same transaction complete; or upon this job's - * completion if it is not in a transaction. Skipped if NULL. - * - * All jobs will complete with a call to either .commit() or .abort() but - * never both. - */ - void (*commit)(Job *job); - - /** - * If the callback is not NULL, it will be invoked when any job in the - * same transaction fails; or upon this job's failure (due to error or - * cancellation) if it is not in a transaction. Skipped if NULL. - * - * All jobs will complete with a call to either .commit() or .abort() but - * never both. - */ - void (*abort)(Job *job); - - /** - * If the callback is not NULL, it will be invoked after a call to either - * .commit() or .abort(). Regardless of which callback is invoked after - * completion, .clean() will always be called, even if the job does not - * belong to a transaction group. - */ - void (*clean)(Job *job); - - /** - * If the callback is not NULL, it will be invoked in job_cancel_async - * - * This function must return true if the job will be cancelled - * immediately without any further I/O (mandatory if @force is - * true), and false otherwise. This lets the generic job layer - * know whether a job has been truly (force-)cancelled, or whether - * it is just in a special completion mode (like mirror after - * READY). - * (If the callback is NULL, the job is assumed to terminate - * without I/O.) - */ - bool (*cancel)(Job *job, bool force); - - - /** Called when the job is freed */ - void (*free)(Job *job); -}; - -typedef enum JobCreateFlags { - /* Default behavior */ - JOB_DEFAULT = 0x00, - /* Job is not QMP-created and should not send QMP events */ - JOB_INTERNAL = 0x01, - /* Job requires manual finalize step */ - JOB_MANUAL_FINALIZE = 0x02, - /* Job requires manual dismiss step */ - JOB_MANUAL_DISMISS = 0x04, -} JobCreateFlags; +#include "job-common.h" /** * Allocate and return a new job transaction. Jobs can be added to the From patchwork Fri Oct 29 16:39:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548085 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YLTbrRw8; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpDL5zSMz9sX3 for ; Sat, 30 Oct 2021 03:44:18 +1100 (AEDT) Received: from localhost ([::1]:50172 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgUzI-0004Ds-7j for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:44:16 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48518) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUus-0006dm-Dd for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:44 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:48575) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUup-00034u-C4 for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525577; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ltWo+GB6rYOMSymVSNYznOCn6F2xid7QqDmS8wQDg34=; b=YLTbrRw8kFqFEckrMc6cp2wJUbzBWGhNCXt2GUyw5DqPpeTVeTNrNW6Ik5mHz9UbjLRWHc YFV5Dzj/AYbDAgQRQFACeu/SE+PY0YHXtRjGvXjgYpnh9PnwCzNbS20LgX4bOkv7NFCWNA MUKWKO1+60eK0L5x0YrNoaXirfWEPkA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-525-p5ANkuCJM8qcG7QB1M0t_g-1; Fri, 29 Oct 2021 12:39:34 -0400 X-MC-Unique: p5ANkuCJM8qcG7QB1M0t_g-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B52B4362F8; Fri, 29 Oct 2021 16:39:32 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id C3A5269119; Fri, 29 Oct 2021 16:39:31 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 02/15] job.c: make job_lock/unlock public Date: Fri, 29 Oct 2021 12:39:01 -0400 Message-Id: <20211029163914.4044794-3-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- include/qemu/job-common.h | 18 ++++++++++++++++++ job.c | 12 +++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/include/qemu/job-common.h b/include/qemu/job-common.h index c115028e33..dcc24fba48 100644 --- a/include/qemu/job-common.h +++ b/include/qemu/job-common.h @@ -297,4 +297,22 @@ typedef enum JobCreateFlags { JOB_MANUAL_DISMISS = 0x04, } JobCreateFlags; +/** + * job_lock: + * + * Take the mutex protecting the list of jobs and their status. + * Most functions called by the monitor need to call job_lock + * and job_unlock manually. On the other hand, function called + * by the block jobs themselves and by the block layer will take the + * lock for you. + */ +void job_lock(void); + +/** + * job_unlock: + * + * Release the mutex protecting the list of jobs and their status. + */ +void job_unlock(void); + #endif diff --git a/job.c b/job.c index 94b142684f..e003f136f0 100644 --- a/job.c +++ b/job.c @@ -32,6 +32,9 @@ #include "trace/trace-root.h" #include "qapi/qapi-events-job.h" +/* job_mutex protects the jobs list, but also makes the job API thread-safe. */ +static QemuMutex job_mutex; + static QLIST_HEAD(, Job) jobs = QLIST_HEAD_INITIALIZER(jobs); /* Job State Transition Table */ @@ -74,17 +77,12 @@ struct JobTxn { int refcnt; }; -/* Right now, this mutex is only needed to synchronize accesses to job->busy - * and job->sleep_timer, such as concurrent calls to job_do_yield and - * job_enter. */ -static QemuMutex job_mutex; - -static void job_lock(void) +void job_lock(void) { qemu_mutex_lock(&job_mutex); } -static void job_unlock(void) +void job_unlock(void) { qemu_mutex_unlock(&job_mutex); } From patchwork Fri Oct 29 16:39:02 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548086 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KOXuJqqf; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpDQ5cLNz9sX3 for ; Sat, 30 Oct 2021 03:44:22 +1100 (AEDT) Received: from localhost ([::1]:50142 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgUzM-0004Cn-Hb for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:44:20 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48534) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUut-0006dn-FZ for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:44 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:36931) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUur-0003HW-Or for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525581; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YT++q4Do5uMkpBMGxtNmwY/JFoVDm+ahkXWWN5J2KyM=; b=KOXuJqqfVOQBROWRblY/E6KnCBV45gXiRZ7+uMlCG7ppjxA7/hz3v5oWdHE6cwTA+wNlPO gb2Ca7AGpypvoRKH37tFOkHGf4pFbCxF64sAC62pP5wMRSNKI50JbfM00g2xb+hj3XqnXw g58xytuB5cSyomy3ap+zg7x0V/opeu4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-373-_PwnyEE4NIOJAZrbV1PDug-1; Fri, 29 Oct 2021 12:39:40 -0400 X-MC-Unique: _PwnyEE4NIOJAZrbV1PDug-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AAAC1362FB; Fri, 29 Oct 2021 16:39:38 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id CF2BC69117; Fri, 29 Oct 2021 16:39:32 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 03/15] job-common.h: categorize fields in struct Job Date: Fri, 29 Oct 2021 12:39:02 -0400 Message-Id: <20211029163914.4044794-4-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Categorize the fields in struct Job to understand which need to be protected by the job muutex and which not. Also move job_type() and job_type_str() there, as they are common helper functions. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job-common.h | 62 +++++++++++++++++++++++++-------------- include/qemu/job.h | 6 ---- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/include/qemu/job-common.h b/include/qemu/job-common.h index dcc24fba48..d2968a848e 100644 --- a/include/qemu/job-common.h +++ b/include/qemu/job-common.h @@ -40,27 +40,52 @@ typedef struct JobTxn JobTxn; * Long-running operation. */ typedef struct Job { + + /* Fields set at initialization (job_create), and never modified */ + /** The ID of the job. May be NULL for internal jobs. */ char *id; - /** The type of this job. */ + /** + * The type of this job. + * All callbacks are called with job_mutex *not* held. + */ const JobDriver *driver; - /** Reference count of the block job */ - int refcnt; - - /** Current state; See @JobStatus for details. */ - JobStatus status; - /** AioContext to run the job coroutine in */ AioContext *aio_context; /** * The coroutine that executes the job. If not NULL, it is reentered when * busy is false and the job is cancelled. + * Initialized in job_start() */ Coroutine *co; + /** True if this job should automatically finalize itself */ + bool auto_finalize; + + /** True if this job should automatically dismiss itself */ + bool auto_dismiss; + + /** The completion function that will be called when the job completes. */ + BlockCompletionFunc *cb; + + /** The opaque value that is passed to the completion function. */ + void *opaque; + + /* ProgressMeter API is thread-safe */ + ProgressMeter progress; + + + /** Protected by job_mutex */ + + /** Reference count of the block job */ + int refcnt; + + /** Current state; See @JobStatus for details. */ + JobStatus status; + /** * Timer that is used by @job_sleep_ns. Accessed under job_mutex (in * job.c). @@ -76,7 +101,7 @@ typedef struct Job { /** * Set to false by the job while the coroutine has yielded and may be * re-entered by job_enter(). There may still be I/O or event loop activity - * pending. Accessed under block_job_mutex (in blockjob.c). + * pending. Accessed under job_mutex. * * When the job is deferred to the main loop, busy is true as long as the * bottom half is still pending. @@ -112,14 +137,6 @@ typedef struct Job { /** Set to true when the job has deferred work to the main loop. */ bool deferred_to_main_loop; - /** True if this job should automatically finalize itself */ - bool auto_finalize; - - /** True if this job should automatically dismiss itself */ - bool auto_dismiss; - - ProgressMeter progress; - /** * Return code from @run and/or @prepare callback(s). * Not final until the job has reached the CONCLUDED status. @@ -134,12 +151,6 @@ typedef struct Job { */ Error *err; - /** The completion function that will be called when the job completes. */ - BlockCompletionFunc *cb; - - /** The opaque value that is passed to the completion function. */ - void *opaque; - /** Notifiers called when a cancelled job is finalised */ NotifierList on_finalize_cancelled; @@ -167,6 +178,7 @@ typedef struct Job { /** * Callbacks and other information about a Job driver. + * All callbacks are invoked with job_mutex *not* held. */ struct JobDriver { @@ -315,4 +327,10 @@ void job_lock(void); */ void job_unlock(void); +/** Returns the JobType of a given Job. */ +JobType job_type(const Job *job); + +/** Returns the enum string for the JobType of a given Job. */ +const char *job_type_str(const Job *job); + #endif diff --git a/include/qemu/job.h b/include/qemu/job.h index 3cfd79088c..e4e0c26672 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -174,12 +174,6 @@ void job_yield(Job *job); void coroutine_fn job_sleep_ns(Job *job, int64_t ns); -/** Returns the JobType of a given Job. */ -JobType job_type(const Job *job); - -/** Returns the enum string for the JobType of a given Job. */ -const char *job_type_str(const Job *job); - /** Returns true if the job should not be visible to the management layer. */ bool job_is_internal(Job *job); From patchwork Fri Oct 29 16:39:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548083 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=b6pxghgt; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4Hgp9J4Cv5z9sX3 for ; Sat, 30 Oct 2021 03:41:39 +1100 (AEDT) Received: from localhost ([::1]:42608 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgUwh-0007Oz-K0 for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:41:35 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48612) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv2-0006jE-OC for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:55 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:53101) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUuv-0003Lp-3h for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:50 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525584; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DY5ZD8ylORg/qMGHFiLOdK/62qb51+HdjO9Y0BsQ70s=; b=b6pxghgt3desm67h65t8PIihBCr5ba8tWGKXezdmzUgknVOAuC5mkmsDmjl6ldef7yHkva 9K/5cpIE1IfsQ7KdI7+suQgsBtG6+YLBC6pQ245lk0jpSt9wE+LizzcrvrdNIeXBYnhw1r vaJZKrW+9IrBm3r6PbGxWibkeOfLG2s= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-437-mWOfH6BRNy2sXwX3i5KEtw-1; Fri, 29 Oct 2021 12:39:41 -0400 X-MC-Unique: mWOfH6BRNy2sXwX3i5KEtw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B715E10A8E00; Fri, 29 Oct 2021 16:39:39 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id C4F1969214; Fri, 29 Oct 2021 16:39:38 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 04/15] jobs: add job-monitor.h Date: Fri, 29 Oct 2021 12:39:03 -0400 Message-Id: <20211029163914.4044794-5-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" job-monitor.h contains all functions of job.h that are used by the monitor and essentially all functions that do not define a JobDriver/Blockdriver. Right now just move the headers, proper categorization and API definition will come in the next commit. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job-monitor.h | 220 +++++++++++++++++++++++++++++++++++++ include/qemu/job.h | 180 +----------------------------- 2 files changed, 221 insertions(+), 179 deletions(-) create mode 100644 include/qemu/job-monitor.h diff --git a/include/qemu/job-monitor.h b/include/qemu/job-monitor.h new file mode 100644 index 0000000000..b5b41a7548 --- /dev/null +++ b/include/qemu/job-monitor.h @@ -0,0 +1,220 @@ +/* + * Declarations for background jobs + * + * Copyright (c) 2011 IBM Corp. + * Copyright (c) 2012, 2018 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef JOB_MONITOR_H +#define JOB_MONITOR_H + +#include "job-common.h" + +/** + * Allocate and return a new job transaction. Jobs can be added to the + * transaction using job_txn_add_job(). + * + * The transaction is automatically freed when the last job completes or is + * cancelled. + * + * All jobs in the transaction either complete successfully or fail/cancel as a + * group. Jobs wait for each other before completing. Cancelling one job + * cancels all jobs in the transaction. + */ +JobTxn *job_txn_new(void); + +/** + * Release a reference that was previously acquired with job_txn_add_job or + * job_txn_new. If it's the last reference to the object, it will be freed. + */ +void job_txn_unref(JobTxn *txn); + +/** + * @txn: The transaction (may be NULL) + * @job: Job to add to the transaction + * + * Add @job to the transaction. The @job must not already be in a transaction. + * The caller must call either job_txn_unref() or job_completed() to release + * the reference that is automatically grabbed here. + * + * If @txn is NULL, the function does nothing. + */ +void job_txn_add_job(JobTxn *txn, Job *job); + +/** + * Add a reference to Job refcnt, it will be decreased with job_unref, and then + * be freed if it comes to be the last reference. + */ +void job_ref(Job *job); + +/** + * Release a reference that was previously acquired with job_ref() or + * job_create(). If it's the last reference to the object, it will be freed. + */ +void job_unref(Job *job); + +/** + * Conditionally enter the job coroutine if the job is ready to run, not + * already busy and fn() returns true. fn() is called while under the job_lock + * critical section. + */ +void job_enter_cond(Job *job, bool(*fn)(Job *job)); + +/** + * Returns true if the job should not be visible to the management layer. + */ +bool job_is_internal(Job *job); + +/** + * Returns whether the job is in a completed state. + */ +bool job_is_completed(Job *job); + +/** + * Request @job to pause at the next pause point. Must be paired with + * job_resume(). If the job is supposed to be resumed by user action, call + * job_user_pause() instead. + */ +void job_pause(Job *job); + +/** + * Resumes a @job paused with job_pause. + */ +void job_resume(Job *job); + +/** + * Asynchronously pause the specified @job. + * Do not allow a resume until a matching call to job_user_resume. + */ +void job_user_pause(Job *job, Error **errp); + +/** + * Returns true if the job is user-paused. + */ +bool job_user_paused(Job *job); + +/** + * Resume the specified @job. + * Must be paired with a preceding job_user_pause. + */ +void job_user_resume(Job *job, Error **errp); + +/** + * Get the next element from the list of block jobs after @job, or the + * first one if @job is %NULL. + * + * Returns the requested job, or %NULL if there are no more jobs left. + */ +Job *job_next(Job *job); + +/** + * Get the job identified by @id (which must not be %NULL). + * + * Returns the requested job, or %NULL if it doesn't exist. + */ +Job *job_get(const char *id); + +/** + * Check whether the verb @verb can be applied to @job in its current state. + * Returns 0 if the verb can be applied; otherwise errp is set and -EPERM + * returned. + */ +int job_apply_verb(Job *job, JobVerb verb, Error **errp); + +/** + * Asynchronously complete the specified @job. + */ +void job_complete(Job *job, Error **errp); + +/** + * Asynchronously cancel the specified @job. If @force is true, the job should + * be cancelled immediately without waiting for a consistent state. + */ +void job_cancel(Job *job, bool force); + +/** + * Cancels the specified job like job_cancel(), but may refuse to do so if the + * operation isn't meaningful in the current state of the job. + */ +void job_user_cancel(Job *job, bool force, Error **errp); + +/** + * Synchronously cancel the @job. The completion callback is called + * before the function returns. If @force is false, the job may + * actually complete instead of canceling itself; the circumstances + * under which this happens depend on the kind of job that is active. + * + * Returns the return value from the job if the job actually completed + * during the call, or -ECANCELED if it was canceled. + * + * Callers must hold the AioContext lock of job->aio_context. + */ +int job_cancel_sync(Job *job, bool force); + +/** + * Synchronously force-cancels all jobs using job_cancel_sync(). + */ +void job_cancel_sync_all(void); + +/** + * @job: The job to be completed. + * @errp: Error object which may be set by job_complete(); this is not + * necessarily set on every error, the job return value has to be + * checked as well. + * + * Synchronously complete the job. The completion callback is called before the + * function returns, unless it is NULL (which is permissible when using this + * function). + * + * Returns the return value from the job. + * + * Callers must hold the AioContext lock of job->aio_context. + */ +int job_complete_sync(Job *job, Error **errp); + +/** + * For a @job that has finished its work and is pending awaiting explicit + * acknowledgement to commit its work, this will commit that work. + * + * FIXME: Make the below statement universally true: + * For jobs that support the manual workflow mode, all graph changes that occur + * as a result will occur after this command and before a successful reply. + */ +void job_finalize(Job *job, Error **errp); + +/** + * Remove the concluded @job from the query list and resets the passed pointer + * to %NULL. Returns an error if the job is not actually concluded. + */ +void job_dismiss(Job **job, Error **errp); + +/** + * Synchronously finishes the given @job. If @finish is given, it is called to + * trigger completion or cancellation of the job. + * + * Returns 0 if the job is successfully completed, -ECANCELED if the job was + * cancelled before completing, and -errno in other error cases. + * + * Callers must hold the AioContext lock of job->aio_context. + */ +int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp); + +#endif diff --git a/include/qemu/job.h b/include/qemu/job.h index e4e0c26672..8d189ed1c2 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -26,38 +26,7 @@ #ifndef JOB_H #define JOB_H -#include "job-common.h" - -/** - * Allocate and return a new job transaction. Jobs can be added to the - * transaction using job_txn_add_job(). - * - * The transaction is automatically freed when the last job completes or is - * cancelled. - * - * All jobs in the transaction either complete successfully or fail/cancel as a - * group. Jobs wait for each other before completing. Cancelling one job - * cancels all jobs in the transaction. - */ -JobTxn *job_txn_new(void); - -/** - * Release a reference that was previously acquired with job_txn_add_job or - * job_txn_new. If it's the last reference to the object, it will be freed. - */ -void job_txn_unref(JobTxn *txn); - -/** - * @txn: The transaction (may be NULL) - * @job: Job to add to the transaction - * - * Add @job to the transaction. The @job must not already be in a transaction. - * The caller must call either job_txn_unref() or job_completed() to release - * the reference that is automatically grabbed here. - * - * If @txn is NULL, the function does nothing. - */ -void job_txn_add_job(JobTxn *txn, Job *job); +#include "job-monitor.h" /** * Create a new long-running job and return it. @@ -75,18 +44,6 @@ void *job_create(const char *job_id, const JobDriver *driver, JobTxn *txn, AioContext *ctx, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp); -/** - * Add a reference to Job refcnt, it will be decreased with job_unref, and then - * be freed if it comes to be the last reference. - */ -void job_ref(Job *job); - -/** - * Release a reference that was previously acquired with job_ref() or - * job_create(). If it's the last reference to the object, it will be freed. - */ -void job_unref(Job *job); - /** * @job: The job that has made progress * @done: How much progress the job made since the last call @@ -126,13 +83,6 @@ void job_event_cancelled(Job *job); /** To be called when a successfully completed job is finalised. */ void job_event_completed(Job *job); -/** - * Conditionally enter the job coroutine if the job is ready to run, not - * already busy and fn() returns true. fn() is called while under the job_lock - * critical section. - */ -void job_enter_cond(Job *job, bool(*fn)(Job *job)); - /** * @job: A job that has not yet been started. * @@ -173,10 +123,6 @@ void job_yield(Job *job); */ void coroutine_fn job_sleep_ns(Job *job, int64_t ns); - -/** Returns true if the job should not be visible to the management layer. */ -bool job_is_internal(Job *job); - /** Returns whether the job is being cancelled. */ bool job_is_cancelled(Job *job); @@ -186,137 +132,13 @@ bool job_is_cancelled(Job *job); */ bool job_cancel_requested(Job *job); -/** Returns whether the job is in a completed state. */ -bool job_is_completed(Job *job); - /** Returns whether the job is ready to be completed. */ bool job_is_ready(Job *job); -/** - * Request @job to pause at the next pause point. Must be paired with - * job_resume(). If the job is supposed to be resumed by user action, call - * job_user_pause() instead. - */ -void job_pause(Job *job); - -/** Resumes a @job paused with job_pause. */ -void job_resume(Job *job); - -/** - * Asynchronously pause the specified @job. - * Do not allow a resume until a matching call to job_user_resume. - */ -void job_user_pause(Job *job, Error **errp); - -/** Returns true if the job is user-paused. */ -bool job_user_paused(Job *job); - -/** - * Resume the specified @job. - * Must be paired with a preceding job_user_pause. - */ -void job_user_resume(Job *job, Error **errp); - -/** - * Get the next element from the list of block jobs after @job, or the - * first one if @job is %NULL. - * - * Returns the requested job, or %NULL if there are no more jobs left. - */ -Job *job_next(Job *job); - -/** - * Get the job identified by @id (which must not be %NULL). - * - * Returns the requested job, or %NULL if it doesn't exist. - */ -Job *job_get(const char *id); - -/** - * Check whether the verb @verb can be applied to @job in its current state. - * Returns 0 if the verb can be applied; otherwise errp is set and -EPERM - * returned. - */ -int job_apply_verb(Job *job, JobVerb verb, Error **errp); - /** The @job could not be started, free it. */ void job_early_fail(Job *job); /** Moves the @job from RUNNING to READY */ void job_transition_to_ready(Job *job); -/** Asynchronously complete the specified @job. */ -void job_complete(Job *job, Error **errp); - -/** - * Asynchronously cancel the specified @job. If @force is true, the job should - * be cancelled immediately without waiting for a consistent state. - */ -void job_cancel(Job *job, bool force); - -/** - * Cancels the specified job like job_cancel(), but may refuse to do so if the - * operation isn't meaningful in the current state of the job. - */ -void job_user_cancel(Job *job, bool force, Error **errp); - -/** - * Synchronously cancel the @job. The completion callback is called - * before the function returns. If @force is false, the job may - * actually complete instead of canceling itself; the circumstances - * under which this happens depend on the kind of job that is active. - * - * Returns the return value from the job if the job actually completed - * during the call, or -ECANCELED if it was canceled. - * - * Callers must hold the AioContext lock of job->aio_context. - */ -int job_cancel_sync(Job *job, bool force); - -/** Synchronously force-cancels all jobs using job_cancel_sync(). */ -void job_cancel_sync_all(void); - -/** - * @job: The job to be completed. - * @errp: Error object which may be set by job_complete(); this is not - * necessarily set on every error, the job return value has to be - * checked as well. - * - * Synchronously complete the job. The completion callback is called before the - * function returns, unless it is NULL (which is permissible when using this - * function). - * - * Returns the return value from the job. - * - * Callers must hold the AioContext lock of job->aio_context. - */ -int job_complete_sync(Job *job, Error **errp); - -/** - * For a @job that has finished its work and is pending awaiting explicit - * acknowledgement to commit its work, this will commit that work. - * - * FIXME: Make the below statement universally true: - * For jobs that support the manual workflow mode, all graph changes that occur - * as a result will occur after this command and before a successful reply. - */ -void job_finalize(Job *job, Error **errp); - -/** - * Remove the concluded @job from the query list and resets the passed pointer - * to %NULL. Returns an error if the job is not actually concluded. - */ -void job_dismiss(Job **job, Error **errp); - -/** - * Synchronously finishes the given @job. If @finish is given, it is called to - * trigger completion or cancellation of the job. - * - * Returns 0 if the job is successfully completed, -ECANCELED if the job was - * cancelled before completing, and -errno in other error cases. - * - * Callers must hold the AioContext lock of job->aio_context. - */ -int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp); - #endif From patchwork Fri Oct 29 16:39:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548087 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=L/59R5nG; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpGC40cXz9sX3 for ; Sat, 30 Oct 2021 03:45:55 +1100 (AEDT) Received: from localhost ([::1]:52602 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV0q-0005s9-LA for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:45:52 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48660) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv5-0006jW-EB for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:57 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:37431) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUuv-0003R1-OK for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:54 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525585; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZfA5e0BfbKPP6LolE9DInCP0HpFqmPCWYjTyHw8QFok=; b=L/59R5nG7T2md9aUXRlnChytj+O5p2J+/qJFtHdeUQNDlG3lGxDLHFkuvjiwpbRjE2AEHX 6R8oDLXds0L/DMZ8vtef/KjtaLPYphZUao3Qm/hyL9MFA4Kl+jIYR3ggbPE0mGU067dW25 qu/ENVft0e9pXJit3sECfPUUCYWnz5Q= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-382-vcNBAMv9NZaxBCXIxUK10Q-1; Fri, 29 Oct 2021 12:39:41 -0400 X-MC-Unique: vcNBAMv9NZaxBCXIxUK10Q-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C22E7806689; Fri, 29 Oct 2021 16:39:40 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id D0D7869117; Fri, 29 Oct 2021 16:39:39 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 05/15] job-monitor.h: define the job monitor API Date: Fri, 29 Oct 2021 12:39:04 -0400 Message-Id: <20211029163914.4044794-6-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" These functions assume that the job lock is held by the caller, to avoid TOC/TOU conditions. Introduce also additional helpers that define _locked functions (useful when the job_mutex is globally applied). Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job-monitor.h | 61 ++++++++++++++++++++++++++++++++++++++ job.c | 22 ++++++++++++-- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/include/qemu/job-monitor.h b/include/qemu/job-monitor.h index b5b41a7548..d92bc4f39d 100644 --- a/include/qemu/job-monitor.h +++ b/include/qemu/job-monitor.h @@ -28,6 +28,21 @@ #include "job-common.h" +/* + * Job monitor API. + * + * These functions use are used by the QEMU monitor, for example + * to execute QMP commands. The monitor is aware of the job_mutex + * presence, so these functions assume it is held by the caller + * to protect job fields (see job-common.h). + * This prevents TOC/TOU bugs, allowing the caller to hold the + * lock between a check in the job state and the actual action. + * + * Therefore, each function in this API that needs protection + * must have the comment + * "Called between job_lock and job_unlock." + */ + /** * Allocate and return a new job transaction. Jobs can be added to the * transaction using job_txn_add_job(). @@ -56,18 +71,24 @@ void job_txn_unref(JobTxn *txn); * the reference that is automatically grabbed here. * * If @txn is NULL, the function does nothing. + * + * Called between job_lock and job_unlock. */ void job_txn_add_job(JobTxn *txn, Job *job); /** * Add a reference to Job refcnt, it will be decreased with job_unref, and then * be freed if it comes to be the last reference. + * + * Called between job_lock and job_unlock. */ void job_ref(Job *job); /** * Release a reference that was previously acquired with job_ref() or * job_create(). If it's the last reference to the object, it will be freed. + * + * Called between job_lock and job_unlock. */ void job_unref(Job *job); @@ -75,6 +96,8 @@ void job_unref(Job *job); * Conditionally enter the job coroutine if the job is ready to run, not * already busy and fn() returns true. fn() is called while under the job_lock * critical section. + * + * Called between job_lock and job_unlock, but it releases the lock temporarly. */ void job_enter_cond(Job *job, bool(*fn)(Job *job)); @@ -85,6 +108,7 @@ bool job_is_internal(Job *job); /** * Returns whether the job is in a completed state. + * Called between job_lock and job_unlock. */ bool job_is_completed(Job *job); @@ -92,28 +116,36 @@ bool job_is_completed(Job *job); * Request @job to pause at the next pause point. Must be paired with * job_resume(). If the job is supposed to be resumed by user action, call * job_user_pause() instead. + * + * Called between job_lock and job_unlock. */ void job_pause(Job *job); /** * Resumes a @job paused with job_pause. + * Called between job_lock and job_unlock. */ void job_resume(Job *job); /** * Asynchronously pause the specified @job. * Do not allow a resume until a matching call to job_user_resume. + * + * Called between job_lock and job_unlock. */ void job_user_pause(Job *job, Error **errp); /** * Returns true if the job is user-paused. + * Called between job_lock and job_unlock. */ bool job_user_paused(Job *job); /** * Resume the specified @job. * Must be paired with a preceding job_user_pause. + * + * Called between job_lock and job_unlock. */ void job_user_resume(Job *job, Error **errp); @@ -122,6 +154,8 @@ void job_user_resume(Job *job, Error **errp); * first one if @job is %NULL. * * Returns the requested job, or %NULL if there are no more jobs left. + * + * Called between job_lock and job_unlock. */ Job *job_next(Job *job); @@ -129,6 +163,8 @@ Job *job_next(Job *job); * Get the job identified by @id (which must not be %NULL). * * Returns the requested job, or %NULL if it doesn't exist. + * + * Called between job_lock and job_unlock. */ Job *job_get(const char *id); @@ -136,23 +172,30 @@ Job *job_get(const char *id); * Check whether the verb @verb can be applied to @job in its current state. * Returns 0 if the verb can be applied; otherwise errp is set and -EPERM * returned. + * + * Called between job_lock and job_unlock. */ int job_apply_verb(Job *job, JobVerb verb, Error **errp); /** * Asynchronously complete the specified @job. + * Called between job_lock and job_unlock, but it releases the lock temporarly. */ void job_complete(Job *job, Error **errp); /** * Asynchronously cancel the specified @job. If @force is true, the job should * be cancelled immediately without waiting for a consistent state. + * + * Called between job_lock and job_unlock. */ void job_cancel(Job *job, bool force); /** * Cancels the specified job like job_cancel(), but may refuse to do so if the * operation isn't meaningful in the current state of the job. + * + * Called between job_lock and job_unlock. */ void job_user_cancel(Job *job, bool force, Error **errp); @@ -171,6 +214,10 @@ int job_cancel_sync(Job *job, bool force); /** * Synchronously force-cancels all jobs using job_cancel_sync(). + * + * Called with job_lock *not* held, unlike most other APIs consumed + * by the monitor! This is primarly to avoid adding unnecessary lock-unlock + * patterns in the caller. */ void job_cancel_sync_all(void); @@ -187,6 +234,8 @@ void job_cancel_sync_all(void); * Returns the return value from the job. * * Callers must hold the AioContext lock of job->aio_context. + * + * Called between job_lock and job_unlock. */ int job_complete_sync(Job *job, Error **errp); @@ -197,12 +246,16 @@ int job_complete_sync(Job *job, Error **errp); * FIXME: Make the below statement universally true: * For jobs that support the manual workflow mode, all graph changes that occur * as a result will occur after this command and before a successful reply. + * + * Called between job_lock and job_unlock. */ void job_finalize(Job *job, Error **errp); /** * Remove the concluded @job from the query list and resets the passed pointer * to %NULL. Returns an error if the job is not actually concluded. + * + * Called between job_lock and job_unlock. */ void job_dismiss(Job **job, Error **errp); @@ -214,7 +267,15 @@ void job_dismiss(Job **job, Error **errp); * cancelled before completing, and -errno in other error cases. * * Callers must hold the AioContext lock of job->aio_context. + * + * Called between job_lock and job_unlock, but it releases the lock temporarly. */ int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp); +/** Same as job_is_ready(), but assumes job_lock is held. */ +bool job_is_ready_locked(Job *job); + +/** Same as job_early_fail(), but assumes job_lock is held. */ +void job_early_fail_locked(Job *job); + #endif diff --git a/job.c b/job.c index e003f136f0..b66d59b746 100644 --- a/job.c +++ b/job.c @@ -225,7 +225,8 @@ bool job_cancel_requested(Job *job) return job->cancelled; } -bool job_is_ready(Job *job) +/* Called with job_mutex held. */ +bool job_is_ready_locked(Job *job) { switch (job->status) { case JOB_STATUS_UNDEFINED: @@ -247,6 +248,16 @@ bool job_is_ready(Job *job) return false; } +/* Called with job_mutex lock *not* held */ +bool job_is_ready(Job *job) +{ + bool res; + job_lock(); + res = job_is_ready_locked(job); + job_unlock(); + return res; +} + bool job_is_completed(Job *job) { switch (job->status) { @@ -642,12 +653,19 @@ void job_dismiss(Job **jobptr, Error **errp) *jobptr = NULL; } -void job_early_fail(Job *job) +void job_early_fail_locked(Job *job) { assert(job->status == JOB_STATUS_CREATED); job_do_dismiss(job); } +void job_early_fail(Job *job) +{ + job_lock(); + job_early_fail_locked(job); + job_unlock(); +} + static void job_conclude(Job *job) { job_state_transition(job, JOB_STATUS_CONCLUDED); From patchwork Fri Oct 29 16:39:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548090 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Hx0V+Wm0; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpLT5gqCz9sX3 for ; Sat, 30 Oct 2021 03:49:37 +1100 (AEDT) Received: from localhost ([::1]:33150 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV4R-0003W4-2l for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:49:35 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48726) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvC-0006m6-7t for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:02 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:34706) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv0-0003WG-NM for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525586; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=SoLHWBqwFq+3bp5mpxge2JH30i6yXLxBS9GlgYhV+PA=; b=Hx0V+Wm0Bb9oomsbiJ3f0Rznf9034cguYpdT4LIjq2kjd6q6RIShsqnvwkMoMUJpu1puEh yVrXBvk/QRRYOC/vnHc/ZvGkkomQMXnbzwbpuuJYiHD6+75WcGZjD+F7AatOlCmrLTsTox PHnlRpF1zjFnG/1gFxQ+d6Z0Uztq2TY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-595-A4YPyzYCO0epW3KfJHN2Hg-1; Fri, 29 Oct 2021 12:39:43 -0400 X-MC-Unique: A4YPyzYCO0epW3KfJHN2Hg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CD17110A8E04; Fri, 29 Oct 2021 16:39:41 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id DBBBF69214; Fri, 29 Oct 2021 16:39:40 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 06/15] jobs: add job-driver.h Date: Fri, 29 Oct 2021 12:39:05 -0400 Message-Id: <20211029163914.4044794-7-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" job-driver.h contains all functions of job.h that are used by the drivers (JobDriver, BlockJobDriver). These functions are unaware of the job_mutex, so they all will take and release the lock internally. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job-driver.h | 152 +++++++++++++++++++++++++++++++++++++ include/qemu/job-monitor.h | 4 +- include/qemu/job.h | 114 +--------------------------- 3 files changed, 156 insertions(+), 114 deletions(-) create mode 100644 include/qemu/job-driver.h diff --git a/include/qemu/job-driver.h b/include/qemu/job-driver.h new file mode 100644 index 0000000000..1efd196da8 --- /dev/null +++ b/include/qemu/job-driver.h @@ -0,0 +1,152 @@ +/* + * Declarations for background jobs + * + * Copyright (c) 2011 IBM Corp. + * Copyright (c) 2012, 2018 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef JOB_DRIVER_H +#define JOB_DRIVER_H + +#include "job-common.h" + +/* + * Job driver API. + * + * These functions use are used by job drivers like mirror, + * stream, commit etc. The driver is not aware of the job_mutex + * presence, so these functions use it internally to protect + * job fields (see job-common.h). + * + * Therefore, each function in this API that requires protection + * must have the comment + * "Called with job_mutext *not* held" + * in job.c + */ + +/** + * Create a new long-running job and return it. + * + * @job_id: The id of the newly-created job, or %NULL for internal jobs + * @driver: The class object for the newly-created job. + * @txn: The transaction this job belongs to, if any. %NULL otherwise. + * @ctx: The AioContext to run the job coroutine in. + * @flags: Creation flags for the job. See @JobCreateFlags. + * @cb: Completion function for the job. + * @opaque: Opaque pointer value passed to @cb. + * @errp: Error object. + */ +void *job_create(const char *job_id, const JobDriver *driver, JobTxn *txn, + AioContext *ctx, int flags, BlockCompletionFunc *cb, + void *opaque, Error **errp); + +/** + * @job: The job that has made progress + * @done: How much progress the job made since the last call + * + * Updates the progress counter of the job. + */ +void job_progress_update(Job *job, uint64_t done); + +/** + * @job: The job whose expected progress end value is set + * @remaining: Missing progress (on top of the current progress counter value) + * until the new expected end value is reached + * + * Sets the expected end value of the progress counter of a job so that a + * completion percentage can be calculated when the progress is updated. + */ +void job_progress_set_remaining(Job *job, uint64_t remaining); + +/** + * @job: The job whose expected progress end value is updated + * @delta: Value which is to be added to the current expected end + * value + * + * Increases the expected end value of the progress counter of a job. + * This is useful for parenthesis operations: If a job has to + * conditionally perform a high-priority operation as part of its + * progress, it calls this function with the expected operation's + * length before, and job_progress_update() afterwards. + * (So the operation acts as a parenthesis in regards to the main job + * operation running in background.) + */ +void job_progress_increase_remaining(Job *job, uint64_t delta); + +/** + * @job: A job that has not yet been started. + * + * Begins execution of a job. + * Takes ownership of one reference to the job object. + */ +void job_start(Job *job); + +/** + * @job: The job to enter. + * + * Continue the specified job by entering the coroutine. + */ +void job_enter(Job *job); + +/** + * @job: The job that is ready to pause. + * + * Pause now if job_pause() has been called. Jobs that perform lots of I/O + * must call this between requests so that the job can be paused. + */ +void coroutine_fn job_pause_point(Job *job); + +/** + * @job: The job that calls the function. + * + * Yield the job coroutine. + */ +void job_yield(Job *job); + +/** + * @job: The job that calls the function. + * @ns: How many nanoseconds to stop for. + * + * Put the job to sleep (assuming that it wasn't canceled) for @ns + * %QEMU_CLOCK_REALTIME nanoseconds. Canceling the job will immediately + * interrupt the wait. + */ +void coroutine_fn job_sleep_ns(Job *job, int64_t ns); + +/** + * Returns whether the job is scheduled for cancellation (at an + * indefinite point). + */ +bool job_cancel_requested(Job *job); + +/** Returns whether the job is scheduled for cancellation. */ +bool job_is_cancelled(Job *job); + +/** Returns whether the job is ready to be completed. */ +bool job_is_ready(Job *job); + +/** The @job could not be started, free it. */ +void job_early_fail(Job *job); + +/** Moves the @job from RUNNING to READY */ +void job_transition_to_ready(Job *job); + +#endif /* JOB_DRIVER_H */ diff --git a/include/qemu/job-monitor.h b/include/qemu/job-monitor.h index d92bc4f39d..f473bd298f 100644 --- a/include/qemu/job-monitor.h +++ b/include/qemu/job-monitor.h @@ -272,10 +272,10 @@ void job_dismiss(Job **job, Error **errp); */ int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp); -/** Same as job_is_ready(), but assumes job_lock is held. */ +/** Same as job_is_ready() in job-driver.h, but assumes job_lock is held. */ bool job_is_ready_locked(Job *job); -/** Same as job_early_fail(), but assumes job_lock is held. */ +/** Same as job_early_fail() in job-driver.h, but assumes job_lock is held. */ void job_early_fail_locked(Job *job); #endif diff --git a/include/qemu/job.h b/include/qemu/job.h index 8d189ed1c2..4a0b01dd1d 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -27,118 +27,8 @@ #define JOB_H #include "job-monitor.h" +#include "job-driver.h" -/** - * Create a new long-running job and return it. - * - * @job_id: The id of the newly-created job, or %NULL for internal jobs - * @driver: The class object for the newly-created job. - * @txn: The transaction this job belongs to, if any. %NULL otherwise. - * @ctx: The AioContext to run the job coroutine in. - * @flags: Creation flags for the job. See @JobCreateFlags. - * @cb: Completion function for the job. - * @opaque: Opaque pointer value passed to @cb. - * @errp: Error object. - */ -void *job_create(const char *job_id, const JobDriver *driver, JobTxn *txn, - AioContext *ctx, int flags, BlockCompletionFunc *cb, - void *opaque, Error **errp); - -/** - * @job: The job that has made progress - * @done: How much progress the job made since the last call - * - * Updates the progress counter of the job. - */ -void job_progress_update(Job *job, uint64_t done); - -/** - * @job: The job whose expected progress end value is set - * @remaining: Missing progress (on top of the current progress counter value) - * until the new expected end value is reached - * - * Sets the expected end value of the progress counter of a job so that a - * completion percentage can be calculated when the progress is updated. - */ -void job_progress_set_remaining(Job *job, uint64_t remaining); - -/** - * @job: The job whose expected progress end value is updated - * @delta: Value which is to be added to the current expected end - * value - * - * Increases the expected end value of the progress counter of a job. - * This is useful for parenthesis operations: If a job has to - * conditionally perform a high-priority operation as part of its - * progress, it calls this function with the expected operation's - * length before, and job_progress_update() afterwards. - * (So the operation acts as a parenthesis in regards to the main job - * operation running in background.) - */ -void job_progress_increase_remaining(Job *job, uint64_t delta); - -/** To be called when a cancelled job is finalised. */ -void job_event_cancelled(Job *job); - -/** To be called when a successfully completed job is finalised. */ -void job_event_completed(Job *job); - -/** - * @job: A job that has not yet been started. - * - * Begins execution of a job. - * Takes ownership of one reference to the job object. - */ -void job_start(Job *job); - -/** - * @job: The job to enter. - * - * Continue the specified job by entering the coroutine. - */ -void job_enter(Job *job); - -/** - * @job: The job that is ready to pause. - * - * Pause now if job_pause() has been called. Jobs that perform lots of I/O - * must call this between requests so that the job can be paused. - */ -void coroutine_fn job_pause_point(Job *job); - -/** - * @job: The job that calls the function. - * - * Yield the job coroutine. - */ -void job_yield(Job *job); - -/** - * @job: The job that calls the function. - * @ns: How many nanoseconds to stop for. - * - * Put the job to sleep (assuming that it wasn't canceled) for @ns - * %QEMU_CLOCK_REALTIME nanoseconds. Canceling the job will immediately - * interrupt the wait. - */ -void coroutine_fn job_sleep_ns(Job *job, int64_t ns); - -/** Returns whether the job is being cancelled. */ -bool job_is_cancelled(Job *job); - -/** - * Returns whether the job is scheduled for cancellation (at an - * indefinite point). - */ -bool job_cancel_requested(Job *job); - -/** Returns whether the job is ready to be completed. */ -bool job_is_ready(Job *job); - -/** The @job could not be started, free it. */ -void job_early_fail(Job *job); - -/** Moves the @job from RUNNING to READY */ -void job_transition_to_ready(Job *job); +/* DO NOT ADD ANYTHING IN HERE. USE ONE OF THE HEADERS INCLUDED ABOVE */ #endif From patchwork Fri Oct 29 16:39:06 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548093 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=cvHeNTvU; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpPm4R3Xz9sX3 for ; Sat, 30 Oct 2021 03:52:28 +1100 (AEDT) Received: from localhost ([::1]:41336 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV7B-0000mv-Me for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:52:25 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48728) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvC-0006m7-Bl for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:02 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:40736) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv2-0003WU-Et for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:39:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525587; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ej4w4id2oIvNpWP8bETfoA2U8uTAohXxo7FV0unjmOE=; b=cvHeNTvUwKz9r+4WrkfjNcnNwA4McMykUT23j7Yj/Ig0d5ywe9dNypBFqfM3cf4wU9M32y 0U0O4NzOPQfEKNiC5caEIlfqAWCm4C38eNmpdE1so+tiM4bTQg0noryoqgPRappokd32Ah tBOTqMqYgkTSCwWb/yBlX7JfQoUR7B4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-27-JAwvG79UMCOE1M6WZZxBbw-1; Fri, 29 Oct 2021 12:39:44 -0400 X-MC-Unique: JAwvG79UMCOE1M6WZZxBbw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D711D362F9; Fri, 29 Oct 2021 16:39:42 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id E751569117; Fri, 29 Oct 2021 16:39:41 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 07/15] job-driver.h: add helper functions Date: Fri, 29 Oct 2021 12:39:06 -0400 Message-Id: <20211029163914.4044794-8-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" These functions will be useful when job_lock is globally applied, as they will allow drivers to access the job struct fields without worrying about the job lock. Now that we are done with the job API header split, update also the comments in blockjob.c (and move them in job.c). Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job-driver.h | 21 +++++++ blockjob.c | 20 ------- job.c | 116 +++++++++++++++++++++++++++++++++++++- 3 files changed, 135 insertions(+), 22 deletions(-) diff --git a/include/qemu/job-driver.h b/include/qemu/job-driver.h index 1efd196da8..19ae5ce8f0 100644 --- a/include/qemu/job-driver.h +++ b/include/qemu/job-driver.h @@ -149,4 +149,25 @@ void job_early_fail(Job *job); /** Moves the @job from RUNNING to READY */ void job_transition_to_ready(Job *job); +/** Enters the @job if it is not paused */ +void job_enter_not_paused(Job *job); + +/** returns @job->ret */ +bool job_has_failed(Job *job); + +/** Returns the @job->status */ +JobStatus job_get_status(Job *job); + +/** Returns the @job->pause_count */ +int job_get_pause_count(Job *job); + +/** Returns @job->paused */ +bool job_get_paused(Job *job); + +/** Returns @job->busy */ +bool job_get_busy(Job *job); + +/** Return true if @job not paused and not cancelled */ +bool job_not_paused_nor_cancelled(Job *job); + #endif /* JOB_DRIVER_H */ diff --git a/blockjob.c b/blockjob.c index 4982f6a2b5..53c1e9c406 100644 --- a/blockjob.c +++ b/blockjob.c @@ -36,21 +36,6 @@ #include "qemu/main-loop.h" #include "qemu/timer.h" -/* - * The block job API is composed of two categories of functions. - * - * The first includes functions used by the monitor. The monitor is - * peculiar in that it accesses the block job list with block_job_get, and - * therefore needs consistency across block_job_get and the actual operation - * (e.g. block_job_set_speed). The consistency is achieved with - * aio_context_acquire/release. These functions are declared in blockjob.h. - * - * The second includes functions used by the block job drivers and sometimes - * by the core block layer. These do not care about locking, because the - * whole coroutine runs under the AioContext lock, and are declared in - * blockjob_int.h. - */ - static bool is_block_job(Job *job) { return job_type(job) == JOB_TYPE_BACKUP || @@ -433,11 +418,6 @@ static void block_job_event_ready(Notifier *n, void *opaque) } -/* - * API for block job drivers and the block layer. These functions are - * declared in blockjob_int.h. - */ - void *block_job_create(const char *job_id, const BlockJobDriver *driver, JobTxn *txn, BlockDriverState *bs, uint64_t perm, uint64_t shared_perm, int64_t speed, int flags, diff --git a/job.c b/job.c index b66d59b746..db7ad79745 100644 --- a/job.c +++ b/job.c @@ -32,6 +32,23 @@ #include "trace/trace-root.h" #include "qapi/qapi-events-job.h" +/* + * The job API is composed of two categories of functions. + * + * The first includes functions used by the monitor. The monitor is + * peculiar in that it accesses the block job list with job_get, and + * therefore needs consistency across job_get and the actual operation + * (e.g. job_user_cancel). To achieve this consistency, the caller + * calls job_lock/job_unlock itself around the whole operation. + * These functions are declared in job-monitor.h. + * + * + * The second includes functions used by the block job drivers and sometimes + * by the core block layer. These delegate the locking to the callee instead, + * and are declared in job-driver.h. + */ + + /* job_mutex protects the jobs list, but also makes the job API thread-safe. */ static QemuMutex job_mutex; @@ -213,18 +230,94 @@ const char *job_type_str(const Job *job) return JobType_str(job_type(job)); } -bool job_is_cancelled(Job *job) +JobStatus job_get_status(Job *job) +{ + JobStatus status; + job_lock(); + status = job->status; + job_unlock(); + return status; +} + +int job_get_pause_count(Job *job) +{ + int ret; + job_lock(); + ret = job->pause_count; + job_unlock(); + return ret; +} + +bool job_get_paused(Job *job) +{ + bool ret; + job_lock(); + ret = job->paused; + job_unlock(); + return ret; +} + +bool job_get_busy(Job *job) +{ + bool ret; + job_lock(); + ret = job->busy; + job_unlock(); + return ret; +} + +bool job_has_failed(Job *job) +{ + bool ret; + job_lock(); + ret = job->ret < 0; + job_unlock(); + return ret; +} + +/* Called with job_mutex held. */ +static bool job_is_cancelled_locked(Job *job) { /* force_cancel may be true only if cancelled is true, too */ assert(job->cancelled || !job->force_cancel); return job->force_cancel; } -bool job_cancel_requested(Job *job) +/* Called with job_mutex *not* held. */ +bool job_is_cancelled(Job *job) +{ + bool ret; + job_lock(); + ret = job_is_cancelled_locked(job); + job_unlock(); + return ret; +} + +bool job_not_paused_nor_cancelled(Job *job) +{ + bool ret; + job_lock(); + ret = !job->paused && !job_is_cancelled_locked(job); + job_unlock(); + return ret; +} + +/* Called with job_mutex held. */ +static bool job_cancel_requested_locked(Job *job) { return job->cancelled; } +/* Called with job_mutex *not* held. */ +bool job_cancel_requested(Job *job) +{ + bool ret; + job_lock(); + ret = job_cancel_requested_locked(job); + job_unlock(); + return ret; +} + /* Called with job_mutex held. */ bool job_is_ready_locked(Job *job) { @@ -280,6 +373,16 @@ bool job_is_completed(Job *job) return false; } +/* Called with job_mutex lock *not* held */ +static bool job_is_completed_unlocked(Job *job) +{ + bool res; + job_lock(); + res = job_is_completed(job); + job_unlock(); + return res; +} + static bool job_started(Job *job) { return job->co; @@ -579,6 +682,15 @@ void job_pause(Job *job) } } +void job_enter_not_paused(Job *job) +{ + job_lock(); + if (!job->paused) { + job_enter_cond(job, NULL); + } + job_unlock(); +} + void job_resume(Job *job) { assert(job->pause_count > 0); From patchwork Fri Oct 29 16:39:07 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548094 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=dR0zADp0; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpPq5fPcz9sX3 for ; Sat, 30 Oct 2021 03:52:31 +1100 (AEDT) Received: from localhost ([::1]:41834 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV7F-00017t-HJ for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:52:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48882) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvL-00070P-Mj for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:21 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:45923) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvE-000483-92 for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525603; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=thCZk/geEE7YFogYN1D0mdVEF+tNHmEnlgVP2vDHd5I=; b=dR0zADp0DJhIlZP67iYsOmSOcP/fZVufzr8jpdrLvTJHnaZ8s+3GnuCTmVqXLAmWYKm/rK 41KfLvNQNOu8BLlk4Q81mye0DwQpN3SxYxCydeZMfYMixKiwp9J5pTLVmsqCRJZS2y6Rie 9P8C3JTZ3oKI6EfbEhg5n/mAg6ALZj4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-71-Kgze3KgGO7yLiKKkd7w3QA-1; Fri, 29 Oct 2021 12:39:45 -0400 X-MC-Unique: Kgze3KgGO7yLiKKkd7w3QA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E06DA1006AA3; Fri, 29 Oct 2021 16:39:43 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id F104569214; Fri, 29 Oct 2021 16:39:42 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 08/15] job.c: minor adjustments in preparation to job-driver Date: Fri, 29 Oct 2021 12:39:07 -0400 Message-Id: <20211029163914.4044794-9-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" job_event_* functions can be all static, as they are not used outside job.c Add also missing notifier initialization for the on_idle list in job_create(). Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/job.c b/job.c index db7ad79745..88d911f2d7 100644 --- a/job.c +++ b/job.c @@ -464,6 +464,7 @@ void *job_create(const char *job_id, const JobDriver *driver, JobTxn *txn, notifier_list_init(&job->on_finalize_completed); notifier_list_init(&job->on_pending); notifier_list_init(&job->on_ready); + notifier_list_init(&job->on_idle); job_state_transition(job, JOB_STATUS_CREATED); aio_timer_init(qemu_get_aio_context(), &job->sleep_timer, @@ -527,12 +528,20 @@ void job_progress_increase_remaining(Job *job, uint64_t delta) progress_increase_remaining(&job->progress, delta); } -void job_event_cancelled(Job *job) +/** + * To be called when a cancelled job is finalised. + * Called with job_mutex held. + */ +static void job_event_cancelled(Job *job) { notifier_list_notify(&job->on_finalize_cancelled, job); } -void job_event_completed(Job *job) +/** + * To be called when a successfully completed job is finalised. + * Called with job_mutex held. + */ +static void job_event_completed(Job *job) { notifier_list_notify(&job->on_finalize_completed, job); } From patchwork Fri Oct 29 16:39:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548089 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=GRaEJDfW; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpLQ422rz9sX3 for ; Sat, 30 Oct 2021 03:49:33 +1100 (AEDT) Received: from localhost ([::1]:32858 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV4M-0003Kf-A7 for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:49:30 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48730) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvC-0006mB-Ay for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:02 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:40857) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv2-0003Wh-Fg for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525589; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jmjvroSopI72pXZHN/uxPLNKVF3UScbVUP6pftVmTw8=; b=GRaEJDfWA6OHiJrvlfujzIacP20qp3gHJmC6YFgvqe8jc/JWwmP5kFT0pwetb5qpTmDqIF 8fzu9lzWo0wEs3feSQ96K7QWr7PsPoN2O7XxyynXn+k2+Hl8CwX+UZ2bTdEtQM7SOt/Q04 bcctbIE04n4Dpm/nzH4/sTwPnoxslBU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-86-D-NOd1MIP7OCLqPlDndCZg-1; Fri, 29 Oct 2021 12:39:46 -0400 X-MC-Unique: D-NOd1MIP7OCLqPlDndCZg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ED9B310A8E03; Fri, 29 Oct 2021 16:39:44 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 06A2A69117; Fri, 29 Oct 2021 16:39:43 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 09/15] job.c: move inner aiocontext lock in callbacks Date: Fri, 29 Oct 2021 12:39:08 -0400 Message-Id: <20211029163914.4044794-10-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/job.c b/job.c index 88d911f2d7..eb6d321960 100644 --- a/job.c +++ b/job.c @@ -153,7 +153,6 @@ static void job_txn_del_job(Job *job) static int job_txn_apply(Job *job, int fn(Job *)) { - AioContext *inner_ctx; Job *other_job, *next; JobTxn *txn = job->txn; int rc = 0; @@ -168,10 +167,7 @@ static int job_txn_apply(Job *job, int fn(Job *)) aio_context_release(job->aio_context); QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) { - inner_ctx = other_job->aio_context; - aio_context_acquire(inner_ctx); rc = fn(other_job); - aio_context_release(inner_ctx); if (rc) { break; } @@ -836,7 +832,10 @@ static void job_clean(Job *job) static int job_finalize_single(Job *job) { + AioContext *ctx = job->aio_context; + assert(job_is_completed(job)); + aio_context_acquire(ctx); /* Ensure abort is called for late-transactional failures */ job_update_rc(job); @@ -863,6 +862,7 @@ static int job_finalize_single(Job *job) job_txn_del_job(job); job_conclude(job); + aio_context_release(ctx); return 0; } @@ -968,11 +968,16 @@ static void job_completed_txn_abort(Job *job) static int job_prepare(Job *job) { + AioContext *ctx = job->aio_context; assert(qemu_in_main_thread()); + + aio_context_acquire(ctx); if (job->ret == 0 && job->driver->prepare) { job->ret = job->driver->prepare(job); job_update_rc(job); } + aio_context_release(ctx); + return job->ret; } From patchwork Fri Oct 29 16:39:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548088 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=imU9gTgo; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpGm4ghnz9sfG for ; Sat, 30 Oct 2021 03:46:24 +1100 (AEDT) Received: from localhost ([::1]:54242 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV1K-00072e-52 for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:46:22 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48758) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvE-0006sl-2n for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:04 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:58321) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv2-0003Wq-GH for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:03 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525590; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hx3r2KUL5QHtcRGIVt9AjkqkTKKz9o4kc1FoO1ja1LY=; b=imU9gTgoLBWLj/evx9cOrNopIvgma0qXZWwBYhnsuYUo9N0xyjHnvifnJeIj8Bgm2AqC74 XV9BSb+kWoLo7DCx1qdiDdxJF26LtXx3KVB3VA8MRqW0xvzEsBWp3F6GlxPhX2cwHZ4tkw xN9ElRXK0BtY0LKIJCTM3vsH6sUD30I= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-311-UJfaxHEFM8W2gtw3XWMAqw-1; Fri, 29 Oct 2021 12:39:47 -0400 X-MC-Unique: UJfaxHEFM8W2gtw3XWMAqw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 05053362FB; Fri, 29 Oct 2021 16:39:46 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 137FF69117; Fri, 29 Oct 2021 16:39:45 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 10/15] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED Date: Fri, 29 Oct 2021 12:39:09 -0400 Message-Id: <20211029163914.4044794-11-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 changed, 11 insertions(+), 4 deletions(-) diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h index b39eefb38d..ff27fe4eab 100644 --- a/include/block/aio-wait.h +++ b/include/block/aio-wait.h @@ -59,10 +59,11 @@ typedef struct { extern AioWait global_aio_wait; /** - * AIO_WAIT_WHILE: + * _AIO_WAIT_WHILE: * @ctx: the aio context, or NULL if multiple aio contexts (for which the * caller does not hold a lock) are involved in the polling condition. * @cond: wait while this conditional expression is true + * @unlock: whether to unlock and then lock again @ctx * * Wait while a condition is true. Use this to implement synchronous * operations that require event loop activity. @@ -75,7 +76,7 @@ extern AioWait global_aio_wait; * wait on conditions between two IOThreads since that could lead to deadlock, * go via the main loop instead. */ -#define AIO_WAIT_WHILE(ctx, cond) ({ \ +#define _AIO_WAIT_WHILE(ctx, cond, unlock) ({ \ bool waited_ = false; \ AioWait *wait_ = &global_aio_wait; \ AioContext *ctx_ = (ctx); \ @@ -90,11 +91,11 @@ extern AioWait global_aio_wait; assert(qemu_get_current_aio_context() == \ qemu_get_aio_context()); \ while ((cond)) { \ - if (ctx_) { \ + if (unlock && ctx_) { \ aio_context_release(ctx_); \ } \ aio_poll(qemu_get_aio_context(), true); \ - if (ctx_) { \ + if (unlock && ctx_) { \ aio_context_acquire(ctx_); \ } \ waited_ = true; \ @@ -103,6 +104,12 @@ extern AioWait global_aio_wait; qatomic_dec(&wait_->num_waiters); \ waited_; }) +#define AIO_WAIT_WHILE(ctx, cond) \ + _AIO_WAIT_WHILE(ctx, cond, true) + +#define AIO_WAIT_WHILE_UNLOCKED(ctx, cond) \ + _AIO_WAIT_WHILE(ctx, cond, false) + /** * aio_wait_kick: * Wake up the main thread if it is waiting on AIO_WAIT_WHILE(). During From patchwork Fri Oct 29 16:39:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548097 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=hLS8+1+T; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpTp0pgyz9sX3 for ; Sat, 30 Oct 2021 03:55:57 +1100 (AEDT) Received: from localhost ([::1]:50344 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgVAX-0006pi-Rp for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:55:53 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48934) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvU-00070w-89 for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:25 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:50948) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvQ-0004Dh-EO for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525615; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=p94Yp9hT0ZiVop2XyugpJj2jnVtBOkeJM12bjvwGUWE=; b=hLS8+1+TmcjdHYD+K0b0ytCNqi5hi4SGbGMxoem25BomWiIvG0lc2OTHMvTt6xXUhmkyre Fcrz4ZUT6RIJxrKX+VFpTvktmZXvQTtQ41rHmQB49nN2hoibZn2hY2JSjwESaaXM6pVWvK m3YEashWCHg1wgY7Ip40Hims+qq46iM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-564-UJNhWfrTOte2bzvJXMjJGg-1; Fri, 29 Oct 2021 12:39:48 -0400 X-MC-Unique: UJNhWfrTOte2bzvJXMjJGg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0FA0910A8E00; Fri, 29 Oct 2021 16:39:47 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1E99669117; Fri, 29 Oct 2021 16:39:46 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 11/15] jobs: remove aiocontext locks since the functions are under BQL Date: Fri, 29 Oct 2021 12:39:10 -0400 Message-Id: <20211029163914.4044794-12-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 acquire/release. We don't want this, as job_lock can only be *wrapped by* the aiocontext lock, and not vice-versa, to avoid deadlocks. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 4 ---- job-qmp.c | 4 ---- 2 files changed, 8 deletions(-) diff --git a/blockdev.c b/blockdev.c index d9bf842a81..67b55eec11 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3719,15 +3719,11 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp) for (job = block_job_next(NULL); job; job = block_job_next(job)) { BlockJobInfo *value; - AioContext *aio_context; if (block_job_is_internal(job)) { continue; } - aio_context = blk_get_aio_context(job->blk); - aio_context_acquire(aio_context); value = block_job_query(job, errp); - aio_context_release(aio_context); if (!value) { qapi_free_BlockJobInfoList(head); return NULL; diff --git a/job-qmp.c b/job-qmp.c index 829a28aa70..a6774aaaa5 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -173,15 +173,11 @@ JobInfoList *qmp_query_jobs(Error **errp) for (job = job_next(NULL); job; job = job_next(job)) { JobInfo *value; - AioContext *aio_context; if (job_is_internal(job)) { continue; } - aio_context = job->aio_context; - aio_context_acquire(aio_context); value = job_query_single(job, errp); - aio_context_release(aio_context); if (!value) { qapi_free_JobInfoList(head); return NULL; From patchwork Fri Oct 29 16:39:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548096 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YICYbAQj; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpR84MTHz9sX3 for ; Sat, 30 Oct 2021 03:53:40 +1100 (AEDT) Received: from localhost ([::1]:44326 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV8M-0002pR-Cq for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:53:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48862) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvJ-00070C-W2 for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:18 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:34495) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv3-0003ag-Ne for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525593; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zixjv+hu9e4uP07xIiTPbwJ18ceNzBIVInJ1uMUlIXI=; b=YICYbAQj7xJeVCSxRIN74tcwVhpzExtwjbIwS1jkGVb08/Ky64bkbXZ575IHg1BMja9WIO 4P1zW3eJrEWUTqk7CyWOjhHndFO9K3x80hIhw9IixmzDHOVtZopYKwYno3Sn5hANWP+WTn 42GnVCwXiZ5OoQIFRdY5brLKQe7UDS4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-503-0BxslCyMPoa15_D1paYA5A-1; Fri, 29 Oct 2021 12:39:49 -0400 X-MC-Unique: 0BxslCyMPoa15_D1paYA5A-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2A040806688; Fri, 29 Oct 2021 16:39:48 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 29BB169117; Fri, 29 Oct 2021 16:39:47 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 12/15] jobs: protect jobs with job_lock/unlock Date: Fri, 29 Oct 2021 12:39:11 -0400 Message-Id: <20211029163914.4044794-13-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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). At this point, we do not care if the job lock is inside or outside the aiocontext. The aiocontext is going away and it is useless to add additional temporary job_lock/unlock pairs just to keep this commit valid. For example: /* assumes job_lock *not* held */ static void job_exit(void *opaque) { job_lock(); job_ref(job); // assumes job_lock held aio_context_acquire(job->aio_context); // we do not want this! However, adding an additional unlock after job_ref() and lock under the aiocontext acquire seems unnecessary, as it will all go away in the next commits. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 6 ++ block/mirror.c | 8 +- block/replication.c | 4 + blockdev.c | 13 +++ blockjob.c | 42 +++++++++- job-qmp.c | 4 + job.c | 190 ++++++++++++++++++++++++++++++++++++-------- monitor/qmp-cmds.c | 2 + qemu-img.c | 8 +- 9 files changed, 232 insertions(+), 45 deletions(-) diff --git a/block.c b/block.c index da80e89ad4..a6dcd9eb36 100644 --- a/block.c +++ b/block.c @@ -4826,7 +4826,9 @@ static void bdrv_close(BlockDriverState *bs) void bdrv_close_all(void) { + job_lock(); assert(job_next(NULL) == NULL); + job_unlock(); assert(qemu_in_main_thread()); /* Drop references from requests still in flight, such as canceled block @@ -5965,6 +5967,8 @@ XDbgBlockGraph *bdrv_get_xdbg_block_graph(Error **errp) } } + job_lock(); + for (job = block_job_next(NULL); job; job = block_job_next(job)) { GSList *el; @@ -5975,6 +5979,8 @@ XDbgBlockGraph *bdrv_get_xdbg_block_graph(Error **errp) } } + job_unlock(); + QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) { xdbg_graph_add_node(gr, bs, X_DBG_BLOCK_GRAPH_NODE_TYPE_BLOCK_DRIVER, bs->node_name); diff --git a/block/mirror.c b/block/mirror.c index 00089e519b..5c4445a8c6 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -653,7 +653,7 @@ static int mirror_exit_common(Job *job) BlockDriverState *target_bs; BlockDriverState *mirror_top_bs; Error *local_err = NULL; - bool abort = job->ret < 0; + bool abort = job_has_failed(job); int ret = 0; if (s->prepared) { @@ -1161,9 +1161,7 @@ static void mirror_complete(Job *job, Error **errp) s->should_complete = true; /* If the job is paused, it will be re-entered when it is resumed */ - if (!job->paused) { - job_enter(job); - } + job_enter_not_paused(job); } static void coroutine_fn mirror_pause(Job *job) @@ -1182,7 +1180,7 @@ static bool mirror_drained_poll(BlockJob *job) * from one of our own drain sections, to avoid a deadlock waiting for * ourselves. */ - if (!s->common.job.paused && !job_is_cancelled(&job->job) && !s->in_drain) { + if (!job_not_paused_nor_cancelled(&s->common.job) && !s->in_drain) { return true; } diff --git a/block/replication.c b/block/replication.c index 55c8f894aa..0f487cc215 100644 --- a/block/replication.c +++ b/block/replication.c @@ -149,7 +149,9 @@ static void replication_close(BlockDriverState *bs) if (s->stage == BLOCK_REPLICATION_FAILOVER) { commit_job = &s->commit_job->job; assert(commit_job->aio_context == qemu_get_current_aio_context()); + job_lock(); job_cancel_sync(commit_job, false); + job_unlock(); } if (s->mode == REPLICATION_MODE_SECONDARY) { @@ -726,7 +728,9 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp) * disk, secondary disk in backup_job_completed(). */ if (s->backup_job) { + job_lock(); job_cancel_sync(&s->backup_job->job, true); + job_unlock(); } if (!failover) { diff --git a/blockdev.c b/blockdev.c index 67b55eec11..c5a835d9ed 100644 --- a/blockdev.c +++ b/blockdev.c @@ -150,6 +150,8 @@ void blockdev_mark_auto_del(BlockBackend *blk) return; } + job_lock(); + for (job = block_job_next(NULL); job; job = block_job_next(job)) { if (block_job_has_bdrv(job, blk_bs(blk))) { AioContext *aio_context = job->job.aio_context; @@ -161,6 +163,8 @@ void blockdev_mark_auto_del(BlockBackend *blk) } } + job_unlock(); + dinfo->auto_del = 1; } @@ -1844,7 +1848,9 @@ static void drive_backup_abort(BlkActionState *common) aio_context = bdrv_get_aio_context(state->bs); aio_context_acquire(aio_context); + job_lock(); job_cancel_sync(&state->job->job, true); + job_unlock(); aio_context_release(aio_context); } @@ -1945,7 +1951,9 @@ static void blockdev_backup_abort(BlkActionState *common) aio_context = bdrv_get_aio_context(state->bs); aio_context_acquire(aio_context); + job_lock(); job_cancel_sync(&state->job->job, true); + job_unlock(); aio_context_release(aio_context); } @@ -2394,7 +2402,9 @@ exit: if (!has_props) { qapi_free_TransactionProperties(props); } + job_lock(); job_txn_unref(block_job_txn); + job_unlock(); } BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node, @@ -3717,6 +3727,7 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp) BlockJobInfoList *head = NULL, **tail = &head; BlockJob *job; + job_lock(); for (job = block_job_next(NULL); job; job = block_job_next(job)) { BlockJobInfo *value; @@ -3726,10 +3737,12 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp) value = block_job_query(job, errp); if (!value) { qapi_free_BlockJobInfoList(head); + job_unlock(); return NULL; } QAPI_LIST_APPEND(tail, value); } + job_unlock(); return head; } diff --git a/blockjob.c b/blockjob.c index 53c1e9c406..426dcddcc1 100644 --- a/blockjob.c +++ b/blockjob.c @@ -88,19 +88,25 @@ static char *child_job_get_parent_desc(BdrvChild *c) static void child_job_drained_begin(BdrvChild *c) { BlockJob *job = c->opaque; + job_lock(); job_pause(&job->job); + job_unlock(); } static bool child_job_drained_poll(BdrvChild *c) { BlockJob *bjob = c->opaque; Job *job = &bjob->job; + bool inactive_incomplete; const BlockJobDriver *drv = block_job_driver(bjob); /* An inactive or completed job doesn't have any pending requests. Jobs * with !job->busy are either already paused or have a pause point after * being reentered, so no job driver code will run before they pause. */ - if (!job->busy || job_is_completed(job)) { + job_lock(); + inactive_incomplete = !job->busy || job_is_completed(job); + job_unlock(); + if (inactive_incomplete) { return false; } @@ -116,7 +122,9 @@ static bool child_job_drained_poll(BdrvChild *c) static void child_job_drained_end(BdrvChild *c, int *drained_end_counter) { BlockJob *job = c->opaque; + job_lock(); job_resume(&job->job); + job_unlock(); } static bool child_job_can_set_aio_ctx(BdrvChild *c, AioContext *ctx, @@ -236,9 +244,16 @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, return 0; } +/* Called with job_mutex lock held. */ static void block_job_on_idle(Notifier *n, void *opaque) { + /* + * we can't kick with job_mutex held, but we also want + * to protect the notifier list. + */ + job_unlock(); aio_wait_kick(); + job_lock(); } bool block_job_is_internal(BlockJob *job) @@ -257,6 +272,7 @@ static bool job_timer_pending(Job *job) return timer_pending(&job->sleep_timer); } +/* Called with job_mutex held. May temporarly release the lock. */ bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) { const BlockJobDriver *drv = block_job_driver(job); @@ -278,7 +294,9 @@ bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) job->speed = speed; if (drv->set_speed) { + job_unlock(); drv->set_speed(job, speed); + job_lock(); } if (speed && speed <= old_speed) { @@ -296,6 +314,7 @@ int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n) return ratelimit_calculate_delay(&job->limit, n); } +/* Called with job_mutex held */ BlockJobInfo *block_job_query(BlockJob *job, Error **errp) { BlockJobInfo *info; @@ -314,13 +333,13 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp) info = g_new0(BlockJobInfo, 1); info->type = g_strdup(job_type_str(&job->job)); info->device = g_strdup(job->job.id); - info->busy = qatomic_read(&job->job.busy); + info->busy = job->job.busy; info->paused = job->job.pause_count > 0; info->offset = progress_current; info->len = progress_total; info->speed = job->speed; info->io_status = job->iostatus; - info->ready = job_is_ready(&job->job), + info->ready = job_is_ready_locked(&job->job), info->status = job->job.status; info->auto_finalize = job->job.auto_finalize; info->auto_dismiss = job->job.auto_dismiss; @@ -341,6 +360,7 @@ static void block_job_iostatus_set_err(BlockJob *job, int error) } } +/* Called with job_mutex lock held. */ static void block_job_event_cancelled(Notifier *n, void *opaque) { BlockJob *job = opaque; @@ -360,6 +380,7 @@ static void block_job_event_cancelled(Notifier *n, void *opaque) job->speed); } +/* Called with job_mutex lock held. */ static void block_job_event_completed(Notifier *n, void *opaque) { BlockJob *job = opaque; @@ -386,6 +407,7 @@ static void block_job_event_completed(Notifier *n, void *opaque) msg); } +/* Called with job_mutex lock held. */ static void block_job_event_pending(Notifier *n, void *opaque) { BlockJob *job = opaque; @@ -398,6 +420,7 @@ static void block_job_event_pending(Notifier *n, void *opaque) job->job.id); } +/* Called with job_mutex lock held. */ static void block_job_event_ready(Notifier *n, void *opaque) { BlockJob *job = opaque; @@ -458,6 +481,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, job->ready_notifier.notify = block_job_event_ready; job->idle_notifier.notify = block_job_on_idle; + job_lock(); notifier_list_add(&job->job.on_finalize_cancelled, &job->finalize_cancelled_notifier); notifier_list_add(&job->job.on_finalize_completed, @@ -465,6 +489,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, notifier_list_add(&job->job.on_pending, &job->pending_notifier); notifier_list_add(&job->job.on_ready, &job->ready_notifier); notifier_list_add(&job->job.on_idle, &job->idle_notifier); + job_unlock(); error_setg(&job->blocker, "block device is in use by block job: %s", job_type_str(&job->job)); @@ -477,14 +502,19 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, blk_set_disable_request_queuing(blk, true); blk_set_allow_aio_context_change(blk, true); + job_lock(); if (!block_job_set_speed(job, speed, errp)) { - job_early_fail(&job->job); + job_early_fail_locked(&job->job); + job_unlock(); return NULL; } + job_unlock(); + return job; } +/* Called with job_mutex lock held. */ void block_job_iostatus_reset(BlockJob *job) { assert(qemu_in_main_thread()); @@ -499,7 +529,9 @@ void block_job_user_resume(Job *job) { assert(qemu_in_main_thread()); BlockJob *bjob = container_of(job, BlockJob, job); + job_lock(); block_job_iostatus_reset(bjob); + job_unlock(); } BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err, @@ -532,11 +564,13 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err, action); } if (action == BLOCK_ERROR_ACTION_STOP) { + job_lock(); if (!job->job.user_paused) { job_pause(&job->job); /* make the pause user visible, which will be resumed from QMP. */ job->job.user_paused = true; } + job_unlock(); block_job_iostatus_set_err(job, error); } return action; diff --git a/job-qmp.c b/job-qmp.c index a6774aaaa5..a355dc2954 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -171,6 +171,8 @@ JobInfoList *qmp_query_jobs(Error **errp) JobInfoList *head = NULL, **tail = &head; Job *job; + job_lock(); + for (job = job_next(NULL); job; job = job_next(job)) { JobInfo *value; @@ -180,10 +182,12 @@ JobInfoList *qmp_query_jobs(Error **errp) value = job_query_single(job, errp); if (!value) { qapi_free_JobInfoList(head); + job_unlock(); return NULL; } QAPI_LIST_APPEND(tail, value); } + job_unlock(); return head; } diff --git a/job.c b/job.c index eb6d321960..6b3e860fa7 100644 --- a/job.c +++ b/job.c @@ -52,6 +52,7 @@ /* job_mutex protects the jobs list, but also makes the job API thread-safe. */ static QemuMutex job_mutex; +/* Protected by job_mutex */ static QLIST_HEAD(, Job) jobs = QLIST_HEAD_INITIALIZER(jobs); /* Job State Transition Table */ @@ -117,6 +118,7 @@ JobTxn *job_txn_new(void) return txn; } +/* Called with job_mutex held. */ static void job_txn_ref(JobTxn *txn) { txn->refcnt++; @@ -142,6 +144,7 @@ void job_txn_add_job(JobTxn *txn, Job *job) job_txn_ref(txn); } +/* Called with job_mutex held. */ static void job_txn_del_job(Job *job) { if (job->txn) { @@ -151,6 +154,7 @@ static void job_txn_del_job(Job *job) } } +/* Called with job_mutex held. */ static int job_txn_apply(Job *job, int fn(Job *)) { Job *other_job, *next; @@ -187,6 +191,7 @@ bool job_is_internal(Job *job) return (job->id == NULL); } +/* Called with job_mutex held. */ static void job_state_transition(Job *job, JobStatus s1) { JobStatus s0 = job->status; @@ -384,6 +389,7 @@ static bool job_started(Job *job) return job->co; } +/* Called with job_mutex held. */ static bool job_should_pause(Job *job) { return job->pause_count > 0; @@ -410,6 +416,7 @@ Job *job_get(const char *id) return NULL; } +/* Called with job_mutex *not* held. */ static void job_sleep_timer_cb(void *opaque) { Job *job = opaque; @@ -417,28 +424,31 @@ static void job_sleep_timer_cb(void *opaque) job_enter(job); } +/* Called with job_mutex *not* held. */ void *job_create(const char *job_id, const JobDriver *driver, JobTxn *txn, AioContext *ctx, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp) { - Job *job; + Job *job = NULL; + + job_lock(); if (job_id) { if (flags & JOB_INTERNAL) { error_setg(errp, "Cannot specify job ID for internal job"); - return NULL; + goto out; } if (!id_wellformed(job_id)) { error_setg(errp, "Invalid job ID '%s'", job_id); - return NULL; + goto out; } if (job_get(job_id)) { error_setg(errp, "Job ID '%s' already in use", job_id); - return NULL; + goto out; } } else if (!(flags & JOB_INTERNAL)) { error_setg(errp, "An explicit job ID is required"); - return NULL; + goto out; } job = g_malloc0(driver->instance_size); @@ -479,6 +489,8 @@ void *job_create(const char *job_id, const JobDriver *driver, JobTxn *txn, job_txn_add_job(txn, job); } +out: + job_unlock(); return job; } @@ -497,7 +509,9 @@ void job_unref(Job *job) assert(!job->txn); if (job->driver->free) { + job_unlock(); job->driver->free(job); + job_lock(); } QLIST_REMOVE(job, job_list); @@ -509,16 +523,19 @@ void job_unref(Job *job) } } +/* Progress API is thread safe */ void job_progress_update(Job *job, uint64_t done) { progress_work_done(&job->progress, done); } +/* Progress API is thread safe */ void job_progress_set_remaining(Job *job, uint64_t remaining) { progress_set_remaining(&job->progress, remaining); } +/* Progress API is thread safe */ void job_progress_increase_remaining(Job *job, uint64_t delta) { progress_increase_remaining(&job->progress, delta); @@ -542,16 +559,19 @@ static void job_event_completed(Job *job) notifier_list_notify(&job->on_finalize_completed, job); } +/* Called with job_mutex held. */ static void job_event_pending(Job *job) { notifier_list_notify(&job->on_pending, job); } +/* Called with job_mutex held. */ static void job_event_ready(Job *job) { notifier_list_notify(&job->on_ready, job); } +/* Called with job_mutex held. */ static void job_event_idle(Job *job) { notifier_list_notify(&job->on_idle, job); @@ -566,14 +586,11 @@ void job_enter_cond(Job *job, bool(*fn)(Job *job)) return; } - job_lock(); if (job->busy) { - job_unlock(); return; } if (fn && !fn(job)) { - job_unlock(); return; } @@ -582,11 +599,15 @@ void job_enter_cond(Job *job, bool(*fn)(Job *job)) job->busy = true; job_unlock(); aio_co_enter(job->aio_context, job->co); + job_lock(); } +/* Called with job_mutex *not* held. */ void job_enter(Job *job) { + job_lock(); job_enter_cond(job, NULL); + job_unlock(); } /* Yield, and schedule a timer to reenter the coroutine after @ns nanoseconds. @@ -594,10 +615,12 @@ void job_enter(Job *job) * is allowed and cancels the timer. * * If @ns is (uint64_t) -1, no timer is scheduled and job_enter() must be - * called explicitly. */ + * called explicitly. + * + * Called with job_mutex held, but releases it temporarly. + */ static void coroutine_fn job_do_yield(Job *job, uint64_t ns) { - job_lock(); if (ns != -1) { timer_mod(&job->sleep_timer, ns); } @@ -605,27 +628,37 @@ static void coroutine_fn job_do_yield(Job *job, uint64_t ns) job_event_idle(job); job_unlock(); qemu_coroutine_yield(); + job_lock(); /* Set by job_enter_cond() before re-entering the coroutine. */ assert(job->busy); } +/* + * Called with job_mutex *not* held (we don't want the coroutine + * to yield with the lock held!). + */ void coroutine_fn job_pause_point(Job *job) { assert(job && job_started(job)); + job_lock(); if (!job_should_pause(job)) { + job_unlock(); return; } - if (job_is_cancelled(job)) { + if (job_is_cancelled_locked(job)) { + job_unlock(); return; } if (job->driver->pause) { + job_unlock(); job->driver->pause(job); + job_lock(); } - if (job_should_pause(job) && !job_is_cancelled(job)) { + if (job_should_pause(job) && !job_is_cancelled_locked(job)) { JobStatus status = job->status; job_state_transition(job, status == JOB_STATUS_READY ? JOB_STATUS_STANDBY @@ -635,45 +668,60 @@ void coroutine_fn job_pause_point(Job *job) job->paused = false; job_state_transition(job, status); } + job_unlock(); if (job->driver->resume) { job->driver->resume(job); } } +/* + * Called with job_mutex *not* held (we don't want the coroutine + * to yield with the lock held!). + */ void job_yield(Job *job) { + job_lock(); assert(job->busy); /* Check cancellation *before* setting busy = false, too! */ - if (job_is_cancelled(job)) { + if (job_is_cancelled_locked(job)) { + job_unlock(); return; } if (!job_should_pause(job)) { job_do_yield(job, -1); } + job_unlock(); job_pause_point(job); } +/* + * Called with job_mutex *not* held (we don't want the coroutine + * to yield with the lock held!). + */ void coroutine_fn job_sleep_ns(Job *job, int64_t ns) { + job_lock(); assert(job->busy); /* Check cancellation *before* setting busy = false, too! */ - if (job_is_cancelled(job)) { + if (job_is_cancelled_locked(job)) { + job_unlock(); return; } if (!job_should_pause(job)) { job_do_yield(job, qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + ns); } + job_unlock(); job_pause_point(job); } -/* Assumes the block_job_mutex is held */ +/* Assumes the job_mutex is held */ static bool job_timer_not_pending(Job *job) { return !timer_pending(&job->sleep_timer); @@ -683,7 +731,7 @@ void job_pause(Job *job) { job->pause_count++; if (!job->paused) { - job_enter(job); + job_enter_cond(job, NULL); } } @@ -738,12 +786,15 @@ void job_user_resume(Job *job, Error **errp) return; } if (job->driver->user_resume) { + job_unlock(); job->driver->user_resume(job); + job_lock(); } job->user_paused = false; job_resume(job); } +/* Called with job_mutex held. */ static void job_do_dismiss(Job *job) { assert(job); @@ -783,6 +834,7 @@ void job_early_fail(Job *job) job_unlock(); } +/* Called with job_mutex held. */ static void job_conclude(Job *job) { job_state_transition(job, JOB_STATUS_CONCLUDED); @@ -791,9 +843,10 @@ static void job_conclude(Job *job) } } +/* Called with job_mutex held. */ static void job_update_rc(Job *job) { - if (!job->ret && job_is_cancelled(job)) { + if (!job->ret && job_is_cancelled_locked(job)) { job->ret = -ECANCELED; } if (job->ret) { @@ -804,42 +857,54 @@ static void job_update_rc(Job *job) } } +/* Called with job_mutex held, but releases it temporarly */ static void job_commit(Job *job) { assert(!job->ret); assert(qemu_in_main_thread()); if (job->driver->commit) { + job_unlock(); job->driver->commit(job); + job_lock(); } } +/* Called with job_mutex held, but releases it temporarly */ static void job_abort(Job *job) { assert(job->ret); assert(qemu_in_main_thread()); if (job->driver->abort) { + job_unlock(); job->driver->abort(job); + job_lock(); } } +/* Called with job_mutex held, but releases it temporarly */ static void job_clean(Job *job) { assert(qemu_in_main_thread()); if (job->driver->clean) { + job_unlock(); job->driver->clean(job); + job_lock(); } } +/* Called with job_mutex held, but releases it temporarly. */ static int job_finalize_single(Job *job) { + int job_ret; AioContext *ctx = job->aio_context; assert(job_is_completed(job)); - aio_context_acquire(ctx); /* Ensure abort is called for late-transactional failures */ job_update_rc(job); + aio_context_acquire(ctx); + if (!job->ret) { job_commit(job); } else { @@ -847,13 +912,18 @@ static int job_finalize_single(Job *job) } job_clean(job); + aio_context_release(ctx); + if (job->cb) { - job->cb(job->opaque, job->ret); + job_ret = job->ret; + job_unlock(); + job->cb(job->opaque, job_ret); + job_lock(); } /* Emit events only if we actually started */ if (job_started(job)) { - if (job_is_cancelled(job)) { + if (job_is_cancelled_locked(job)) { job_event_cancelled(job); } else { job_event_completed(job); @@ -862,15 +932,18 @@ static int job_finalize_single(Job *job) job_txn_del_job(job); job_conclude(job); - aio_context_release(ctx); return 0; } +/* Called with job_mutex held, but releases it temporarly. */ static void job_cancel_async(Job *job, bool force) { assert(qemu_in_main_thread()); + if (job->driver->cancel) { + job_unlock(); force = job->driver->cancel(job, force); + job_lock(); } else { /* No .cancel() means the job will behave as if force-cancelled */ force = true; @@ -879,7 +952,9 @@ static void job_cancel_async(Job *job, bool force) if (job->user_paused) { /* Do not call job_enter here, the caller will handle it. */ if (job->driver->user_resume) { + job_unlock(); job->driver->user_resume(job); + job_lock(); } job->user_paused = false; assert(job->pause_count > 0); @@ -900,6 +975,7 @@ static void job_cancel_async(Job *job, bool force) } } +/* Called with job_mutex held. */ static void job_completed_txn_abort(Job *job) { AioContext *ctx; @@ -949,7 +1025,7 @@ static void job_completed_txn_abort(Job *job) ctx = other_job->aio_context; aio_context_acquire(ctx); if (!job_is_completed(other_job)) { - assert(job_cancel_requested(other_job)); + assert(job_cancel_requested_locked(other_job)); job_finish_sync(other_job, NULL, NULL); } job_finalize_single(other_job); @@ -966,26 +1042,33 @@ static void job_completed_txn_abort(Job *job) job_txn_unref(txn); } +/* Called with job_mutex held, but releases it temporarly. */ static int job_prepare(Job *job) { + int ret; AioContext *ctx = job->aio_context; assert(qemu_in_main_thread()); - aio_context_acquire(ctx); if (job->ret == 0 && job->driver->prepare) { - job->ret = job->driver->prepare(job); + job_unlock(); + aio_context_acquire(ctx); + ret = job->driver->prepare(job); + aio_context_release(ctx); + job_lock(); + job->ret = ret; job_update_rc(job); } - aio_context_release(ctx); return job->ret; } +/* Called with job_mutex held. */ static int job_needs_finalize(Job *job) { return !job->auto_finalize; } +/* Called with job_mutex held. */ static void job_do_finalize(Job *job) { int rc; @@ -1009,6 +1092,7 @@ void job_finalize(Job *job, Error **errp) job_do_finalize(job); } +/* Called with job_mutex held. */ static int job_transition_to_pending(Job *job) { job_state_transition(job, JOB_STATUS_PENDING); @@ -1018,12 +1102,16 @@ static int job_transition_to_pending(Job *job) return 0; } +/* Called with job_mutex *not* held. */ void job_transition_to_ready(Job *job) { + job_lock(); job_state_transition(job, JOB_STATUS_READY); job_event_ready(job); + job_unlock(); } +/* Called with job_mutex held. */ static void job_completed_txn_success(Job *job) { JobTxn *txn = job->txn; @@ -1050,6 +1138,7 @@ static void job_completed_txn_success(Job *job) } } +/* Called with job_mutex held. */ static void job_completed(Job *job) { assert(job && job->txn && !job_is_completed(job)); @@ -1063,12 +1152,16 @@ static void job_completed(Job *job) } } -/** Useful only as a type shim for aio_bh_schedule_oneshot. */ +/** + * Useful only as a type shim for aio_bh_schedule_oneshot. + * Called with job_mutex *not* held. + */ static void job_exit(void *opaque) { Job *job = (Job *)opaque; AioContext *ctx; + job_lock(); job_ref(job); aio_context_acquire(job->aio_context); @@ -1089,27 +1182,35 @@ static void job_exit(void *opaque) */ ctx = job->aio_context; job_unref(job); + job_unlock(); aio_context_release(ctx); } /** * All jobs must allow a pause point before entering their job proper. This * ensures that jobs can be paused prior to being started, then resumed later. + * + * Called with job_mutex *not* held. */ static void coroutine_fn job_co_entry(void *opaque) { Job *job = opaque; - + int ret; assert(job && job->driver && job->driver->run); job_pause_point(job); - job->ret = job->driver->run(job, &job->err); + ret = job->driver->run(job, &job->err); + job_lock(); + job->ret = ret; job->deferred_to_main_loop = true; job->busy = true; + job_unlock(); aio_bh_schedule_oneshot(qemu_get_aio_context(), job_exit, job); } +/* Called with job_mutex *not* held. */ void job_start(Job *job) { + job_lock(); assert(job && !job_started(job) && job->paused && job->driver && job->driver->run); job->co = qemu_coroutine_create(job_co_entry, job); @@ -1117,6 +1218,7 @@ void job_start(Job *job) job->busy = true; job->paused = false; job_state_transition(job, JOB_STATUS_RUNNING); + job_unlock(); aio_co_enter(job->aio_context, job->co); } @@ -1140,11 +1242,11 @@ void job_cancel(Job *job, bool force) * choose to call job_is_cancelled() to show that we invoke * job_completed_txn_abort() only for force-cancelled jobs.) */ - if (job_is_cancelled(job)) { + if (job_is_cancelled_locked(job)) { job_completed_txn_abort(job); } } else { - job_enter(job); + job_enter_cond(job, NULL); } } @@ -1156,9 +1258,13 @@ void job_user_cancel(Job *job, bool force, Error **errp) job_cancel(job, force); } -/* A wrapper around job_cancel() taking an Error ** parameter so it may be +/* + * A wrapper around job_cancel() taking an Error ** parameter so it may be * used with job_finish_sync() without the need for (rather nasty) function - * pointer casts there. */ + * pointer casts there. + * + * Called with job_mutex held. + */ static void job_cancel_err(Job *job, Error **errp) { job_cancel(job, false); @@ -1166,6 +1272,8 @@ static void job_cancel_err(Job *job, Error **errp) /** * Same as job_cancel_err(), but force-cancel. + * + * Called with job_mutex held. */ static void job_force_cancel_err(Job *job, Error **errp) { @@ -1181,17 +1289,24 @@ int job_cancel_sync(Job *job, bool force) } } +/* + * Called with job_lock *not* held, unlike most other APIs consumed + * by the monitor! This is primarly to avoid adding lock-unlock + * patterns in the caller. + */ void job_cancel_sync_all(void) { Job *job; AioContext *aio_context; + job_lock(); while ((job = job_next(NULL))) { aio_context = job->aio_context; aio_context_acquire(aio_context); job_cancel_sync(job, true); aio_context_release(aio_context); } + job_unlock(); } int job_complete_sync(Job *job, Error **errp) @@ -1207,13 +1322,15 @@ void job_complete(Job *job, Error **errp) if (job_apply_verb(job, JOB_VERB_COMPLETE, errp)) { return; } - if (job_cancel_requested(job) || !job->driver->complete) { + if (job_cancel_requested_locked(job) || !job->driver->complete) { error_setg(errp, "The active block job '%s' cannot be completed", job->id); return; } + job_unlock(); job->driver->complete(job, errp); + job_lock(); } int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp) @@ -1232,10 +1349,13 @@ int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp) return -EBUSY; } + job_unlock(); AIO_WAIT_WHILE(job->aio_context, - (job_enter(job), !job_is_completed(job))); + (job_enter(job), !job_is_completed_unlocked(job))); + job_lock(); - ret = (job_is_cancelled(job) && job->ret == 0) ? -ECANCELED : job->ret; + ret = (job_is_cancelled_locked(job) && job->ret == 0) ? + -ECANCELED : job->ret; job_unref(job); return ret; } diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 5c0d5e116b..a0b023cac1 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -129,9 +129,11 @@ void qmp_cont(Error **errp) blk_iostatus_reset(blk); } + job_lock(); for (job = block_job_next(NULL); job; job = block_job_next(job)) { block_job_iostatus_reset(job); } + job_unlock(); /* Continuing after completed migration. Images have been inactivated to * allow the destination to take control. Need to get control back now. diff --git a/qemu-img.c b/qemu-img.c index f036a1d428..170c65b1b7 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -906,9 +906,11 @@ static void run_block_job(BlockJob *job, Error **errp) int ret = 0; aio_context_acquire(aio_context); + job_lock(); job_ref(&job->job); do { float progress = 0.0f; + job_unlock(); aio_poll(aio_context, true); progress_get_snapshot(&job->job.progress, &progress_current, @@ -917,7 +919,8 @@ static void run_block_job(BlockJob *job, Error **errp) progress = (float)progress_current / progress_total * 100.f; } qemu_progress_print(progress, 0); - } while (!job_is_ready(&job->job) && !job_is_completed(&job->job)); + job_lock(); + } while (!job_is_ready_locked(&job->job) && !job_is_completed(&job->job)); if (!job_is_completed(&job->job)) { ret = job_complete_sync(&job->job, errp); @@ -925,6 +928,7 @@ static void run_block_job(BlockJob *job, Error **errp) ret = job->job.ret; } job_unref(&job->job); + job_unlock(); aio_context_release(aio_context); /* publish completion progress only when success */ @@ -1077,7 +1081,9 @@ static int img_commit(int argc, char **argv) bdrv_ref(bs); } + job_lock(); job = block_job_get("commit"); + job_unlock(); assert(job); run_block_job(job, &local_err); if (local_err) { From patchwork Fri Oct 29 16:39:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548098 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=hP61eub5; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpVY2zHPz9sX3 for ; Sat, 30 Oct 2021 03:56:37 +1100 (AEDT) Received: from localhost ([::1]:51608 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgVBD-0007eL-7n for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:56:35 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48816) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvF-0006y3-GZ for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:06 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:26626) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv4-0003g6-U7 for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525593; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZyfrhaY+I1k/eGtHRCcKtOeEiS7ECZVnlGV89SUIEYg=; b=hP61eub5Lf8GU7EBO87yfoQIyJ3hdc+/nGw8tMU7+9w11wQILvccPue2eoTe8DATDt4WVJ A9uKoavGtZEaDe1Cawmr6tn/pEJHggiJPNmUWFAs+T+9fWYxnXvedVmbhUUg2hO9a2MZX0 mcluW8MDO2/gYApaZkqQ5tDgG1Rd+Qk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-498-AKXjCbYYP4i6vD5o-0VqIg-1; Fri, 29 Oct 2021 12:39:50 -0400 X-MC-Unique: AKXjCbYYP4i6vD5o-0VqIg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3343F18D6A2F; Fri, 29 Oct 2021 16:39:49 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4320A69214; Fri, 29 Oct 2021 16:39:48 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 13/15] jobs: use job locks and helpers also in the unit tests Date: Fri, 29 Oct 2021 12:39:12 -0400 Message-Id: <20211029163914.4044794-14-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Add missing job synchronization in the unit tests, with both explicit locks and helpers. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 40 +++++++++++----------- tests/unit/test-block-iothread.c | 4 +++ tests/unit/test-blockjob-txn.c | 10 ++++++ tests/unit/test-blockjob.c | 57 +++++++++++++++++++++----------- 4 files changed, 72 insertions(+), 39 deletions(-) diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 2d3c17e566..535c39b5a8 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -941,61 +941,63 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, } } - g_assert_cmpint(job->job.pause_count, ==, 0); - g_assert_false(job->job.paused); + g_assert_cmpint(job_get_pause_count(&job->job), ==, 0); + g_assert_false(job_get_paused(&job->job)); g_assert_true(tjob->running); - g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */ + g_assert_true(job_get_busy(&job->job)); /* We're in qemu_co_sleep_ns() */ do_drain_begin_unlocked(drain_type, drain_bs); if (drain_type == BDRV_DRAIN_ALL) { /* bdrv_drain_all() drains both src and target */ - g_assert_cmpint(job->job.pause_count, ==, 2); + g_assert_cmpint(job_get_pause_count(&job->job), ==, 2); } else { - g_assert_cmpint(job->job.pause_count, ==, 1); + g_assert_cmpint(job_get_pause_count(&job->job), ==, 1); } - g_assert_true(job->job.paused); - g_assert_false(job->job.busy); /* The job is paused */ + g_assert_true(job_get_paused(&job->job)); + g_assert_false(job_get_busy(&job->job)); /* The job is paused */ do_drain_end_unlocked(drain_type, drain_bs); if (use_iothread) { /* paused is reset in the I/O thread, wait for it */ - while (job->job.paused) { + while (job_get_paused(&job->job)) { aio_poll(qemu_get_aio_context(), false); } } - g_assert_cmpint(job->job.pause_count, ==, 0); - g_assert_false(job->job.paused); - g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */ + g_assert_cmpint(job_get_pause_count(&job->job), ==, 0); + g_assert_false(job_get_paused(&job->job)); + g_assert_true(job_get_busy(&job->job)); /* We're in qemu_co_sleep_ns() */ do_drain_begin_unlocked(drain_type, target); if (drain_type == BDRV_DRAIN_ALL) { /* bdrv_drain_all() drains both src and target */ - g_assert_cmpint(job->job.pause_count, ==, 2); + g_assert_cmpint(job_get_pause_count(&job->job), ==, 2); } else { - g_assert_cmpint(job->job.pause_count, ==, 1); + g_assert_cmpint(job_get_pause_count(&job->job), ==, 1); } - g_assert_true(job->job.paused); - g_assert_false(job->job.busy); /* The job is paused */ + g_assert_true(job_get_paused(&job->job)); + g_assert_false(job_get_busy(&job->job)); /* The job is paused */ do_drain_end_unlocked(drain_type, target); if (use_iothread) { /* paused is reset in the I/O thread, wait for it */ - while (job->job.paused) { + while (job_get_paused(&job->job)) { aio_poll(qemu_get_aio_context(), false); } } - g_assert_cmpint(job->job.pause_count, ==, 0); - g_assert_false(job->job.paused); - g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */ + g_assert_cmpint(job_get_pause_count(&job->job), ==, 0); + g_assert_false(job_get_paused(&job->job)); + g_assert_true(job_get_busy(&job->job)); /* We're in qemu_co_sleep_ns() */ aio_context_acquire(ctx); + job_lock(); ret = job_complete_sync(&job->job, &error_abort); + job_unlock(); g_assert_cmpint(ret, ==, (result == TEST_JOB_SUCCESS ? 0 : -EIO)); if (use_iothread) { diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c index aea660aeed..f39cb8b7ef 100644 --- a/tests/unit/test-block-iothread.c +++ b/tests/unit/test-block-iothread.c @@ -456,7 +456,9 @@ static void test_attach_blockjob(void) } aio_context_acquire(ctx); + job_lock(); job_complete_sync(&tjob->common.job, &error_abort); + job_unlock(); blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort); aio_context_release(ctx); @@ -630,7 +632,9 @@ static void test_propagate_mirror(void) BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT, false, "filter_node", MIRROR_COPY_MODE_BACKGROUND, &error_abort); + job_lock(); job = job_get("job0"); + job_unlock(); filter = bdrv_find_node("filter_node"); /* Change the AioContext of src */ diff --git a/tests/unit/test-blockjob-txn.c b/tests/unit/test-blockjob-txn.c index 8bd13b9949..1ae3a9d443 100644 --- a/tests/unit/test-blockjob-txn.c +++ b/tests/unit/test-blockjob-txn.c @@ -124,16 +124,20 @@ static void test_single_job(int expected) job = test_block_job_start(1, true, expected, &result, txn); job_start(&job->job); + job_lock(); if (expected == -ECANCELED) { job_cancel(&job->job, false); } + job_unlock(); while (result == -EINPROGRESS) { aio_poll(qemu_get_aio_context(), true); } g_assert_cmpint(result, ==, expected); + job_lock(); job_txn_unref(txn); + job_unlock(); } static void test_single_job_success(void) @@ -168,6 +172,7 @@ 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_lock(); job_txn_unref(txn); if (expected1 == -ECANCELED) { @@ -176,6 +181,7 @@ static void test_pair_jobs(int expected1, int expected2) if (expected2 == -ECANCELED) { job_cancel(&job2->job, false); } + job_unlock(); while (result1 == -EINPROGRESS || result2 == -EINPROGRESS) { aio_poll(qemu_get_aio_context(), true); @@ -227,7 +233,9 @@ static void test_pair_jobs_fail_cancel_race(void) job_start(&job1->job); job_start(&job2->job); + job_lock(); job_cancel(&job1->job, false); + job_unlock(); /* Now make job2 finish before the main loop kicks jobs. This simulates * the race between a pending kick and another job completing. @@ -242,7 +250,9 @@ static void test_pair_jobs_fail_cancel_race(void) g_assert_cmpint(result1, ==, -ECANCELED); g_assert_cmpint(result2, ==, -ECANCELED); + job_lock(); job_txn_unref(txn); + job_unlock(); } int main(int argc, char **argv) diff --git a/tests/unit/test-blockjob.c b/tests/unit/test-blockjob.c index 4c9e1bf1e5..b94e1510c9 100644 --- a/tests/unit/test-blockjob.c +++ b/tests/unit/test-blockjob.c @@ -211,8 +211,11 @@ static CancelJob *create_common(Job **pjob) bjob = mk_job(blk, "Steve", &test_cancel_driver, true, JOB_MANUAL_FINALIZE | JOB_MANUAL_DISMISS); job = &bjob->job; + job_lock(); job_ref(job); assert(job->status == JOB_STATUS_CREATED); + job_unlock(); + s = container_of(bjob, CancelJob, common); s->blk = blk; @@ -230,6 +233,7 @@ static void cancel_common(CancelJob *s) ctx = job->job.aio_context; aio_context_acquire(ctx); + job_lock(); job_cancel_sync(&job->job, true); if (sts != JOB_STATUS_CREATED && sts != JOB_STATUS_CONCLUDED) { Job *dummy = &job->job; @@ -237,6 +241,7 @@ static void cancel_common(CancelJob *s) } assert(job->job.status == JOB_STATUS_NULL); job_unref(&job->job); + job_unlock(); destroy_blk(blk); aio_context_release(ctx); @@ -259,7 +264,7 @@ static void test_cancel_running(void) s = create_common(&job); job_start(job); - assert(job->status == JOB_STATUS_RUNNING); + assert(job_get_status(job) == JOB_STATUS_RUNNING); cancel_common(s); } @@ -272,11 +277,13 @@ static void test_cancel_paused(void) s = create_common(&job); job_start(job); - assert(job->status == JOB_STATUS_RUNNING); + assert(job_get_status(job) == JOB_STATUS_RUNNING); + job_lock(); job_user_pause(job, &error_abort); + job_unlock(); job_enter(job); - assert(job->status == JOB_STATUS_PAUSED); + assert(job_get_status(job) == JOB_STATUS_PAUSED); cancel_common(s); } @@ -289,11 +296,11 @@ static void test_cancel_ready(void) s = create_common(&job); job_start(job); - assert(job->status == JOB_STATUS_RUNNING); + assert(job_get_status(job) == JOB_STATUS_RUNNING); s->should_converge = true; job_enter(job); - assert(job->status == JOB_STATUS_READY); + assert(job_get_status(job) == JOB_STATUS_READY); cancel_common(s); } @@ -306,15 +313,17 @@ static void test_cancel_standby(void) s = create_common(&job); job_start(job); - assert(job->status == JOB_STATUS_RUNNING); + assert(job_get_status(job) == JOB_STATUS_RUNNING); s->should_converge = true; job_enter(job); - assert(job->status == JOB_STATUS_READY); + assert(job_get_status(job) == JOB_STATUS_READY); + job_lock(); job_user_pause(job, &error_abort); + job_unlock(); job_enter(job); - assert(job->status == JOB_STATUS_STANDBY); + assert(job_get_status(job) == JOB_STATUS_STANDBY); cancel_common(s); } @@ -327,20 +336,22 @@ static void test_cancel_pending(void) s = create_common(&job); job_start(job); - assert(job->status == JOB_STATUS_RUNNING); + assert(job_get_status(job) == JOB_STATUS_RUNNING); s->should_converge = true; job_enter(job); - assert(job->status == JOB_STATUS_READY); + assert(job_get_status(job) == JOB_STATUS_READY); + job_lock(); job_complete(job, &error_abort); + job_unlock(); job_enter(job); while (!job->deferred_to_main_loop) { aio_poll(qemu_get_aio_context(), true); } - assert(job->status == JOB_STATUS_READY); + assert(job_get_status(job) == JOB_STATUS_READY); aio_poll(qemu_get_aio_context(), true); - assert(job->status == JOB_STATUS_PENDING); + assert(job_get_status(job) == JOB_STATUS_PENDING); cancel_common(s); } @@ -353,25 +364,29 @@ static void test_cancel_concluded(void) s = create_common(&job); job_start(job); - assert(job->status == JOB_STATUS_RUNNING); + assert(job_get_status(job) == JOB_STATUS_RUNNING); s->should_converge = true; job_enter(job); - assert(job->status == JOB_STATUS_READY); + assert(job_get_status(job) == JOB_STATUS_READY); + job_lock(); job_complete(job, &error_abort); + job_unlock(); job_enter(job); while (!job->deferred_to_main_loop) { aio_poll(qemu_get_aio_context(), true); } - assert(job->status == JOB_STATUS_READY); + assert(job_get_status(job) == JOB_STATUS_READY); aio_poll(qemu_get_aio_context(), true); - assert(job->status == JOB_STATUS_PENDING); + assert(job_get_status(job) == JOB_STATUS_PENDING); aio_context_acquire(job->aio_context); + job_lock(); job_finalize(job, &error_abort); + job_unlock(); aio_context_release(job->aio_context); - assert(job->status == JOB_STATUS_CONCLUDED); + assert(job_get_status(job) == JOB_STATUS_CONCLUDED); cancel_common(s); } @@ -459,22 +474,23 @@ static void test_complete_in_standby(void) bjob = mk_job(blk, "job", &test_yielding_driver, true, JOB_MANUAL_FINALIZE | JOB_MANUAL_DISMISS); job = &bjob->job; - assert(job->status == JOB_STATUS_CREATED); + assert(job_get_status(job) == JOB_STATUS_CREATED); /* Wait for the job to become READY */ job_start(job); aio_context_acquire(ctx); - AIO_WAIT_WHILE(ctx, job->status != JOB_STATUS_READY); + AIO_WAIT_WHILE(ctx, job_get_status(job) != JOB_STATUS_READY); aio_context_release(ctx); /* Begin the drained section, pausing the job */ bdrv_drain_all_begin(); - assert(job->status == JOB_STATUS_STANDBY); + assert(job_get_status(job) == JOB_STATUS_STANDBY); /* Lock the IO thread to prevent the job from being run */ aio_context_acquire(ctx); /* This will schedule the job to resume it */ bdrv_drain_all_end(); + job_lock(); /* But the job cannot run, so it will remain on standby */ assert(job->status == JOB_STATUS_STANDBY); @@ -489,6 +505,7 @@ static void test_complete_in_standby(void) assert(job->status == JOB_STATUS_CONCLUDED); job_dismiss(&job, &error_abort); + job_unlock(); destroy_blk(blk); aio_context_release(ctx); From patchwork Fri Oct 29 16:39:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548091 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KkVbBRIJ; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpLW6WlCz9sX3 for ; Sat, 30 Oct 2021 03:49:39 +1100 (AEDT) Received: from localhost ([::1]:33380 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV4T-0003fL-LJ for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:49:37 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48864) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvJ-00070D-0f for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:17 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:34504) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUv8-0003hN-UM for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525594; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EDAhwS7nAT0w+wGzXAq361uetIozqftT+K8z8x0H0M8=; b=KkVbBRIJv/5R9MglY+AEuqoUj6LkEed/DHisyxRU9NW2bYIisguJH23zxpVfGo0gEZOsh5 0nhDxC1ht4I9mTDMbgWJm1Jtrw0tOQ8obRlZzu5OZ781/rDEfL/7zgoeWCVt/xmJ541pGA tf/A+u9QPqhZv7bWdzx+X8AsXZb8Ql8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-523-otdr25PBPMGgk1qJSBXJ1g-1; Fri, 29 Oct 2021 12:39:51 -0400 X-MC-Unique: otdr25PBPMGgk1qJSBXJ1g-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3E50C362F8; Fri, 29 Oct 2021 16:39:50 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4CAD469117; Fri, 29 Oct 2021 16:39:49 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 14/15] jobs: add missing job locks to replace aiocontext lock Date: Fri, 29 Oct 2021 12:39:13 -0400 Message-Id: <20211029163914.4044794-15-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" find_block_job() and find_job() cannot be handled like all other functions in the previous commit: in order to avoid unneccessary job lock/unlock, replace the aiocontext lock with the job_lock. However, once we start dropping these aiocontex locks we also break the assumptions of the callees in the job API, many of which assume the aiocontext lock is held. To keep this commit simple, handle the rest of the aiocontext removal in the next commit. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 52 ++++++++++++++++++---------------------------------- job-qmp.c | 46 ++++++++++++++++------------------------------ 2 files changed, 34 insertions(+), 64 deletions(-) diff --git a/blockdev.c b/blockdev.c index c5a835d9ed..592ed4a0fc 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3318,48 +3318,43 @@ out: aio_context_release(aio_context); } -/* Get a block job using its ID and acquire its AioContext */ -static BlockJob *find_block_job(const char *id, AioContext **aio_context, - Error **errp) +/* Get a block job using its ID. Returns with job_lock held on success */ +static BlockJob *find_block_job(const char *id, Error **errp) { BlockJob *job; assert(id != NULL); - *aio_context = NULL; + job_lock(); job = block_job_get(id); if (!job) { error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE, "Block job '%s' not found", id); + job_unlock(); return NULL; } - *aio_context = blk_get_aio_context(job->blk); - aio_context_acquire(*aio_context); - return job; } void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp) { - AioContext *aio_context; - BlockJob *job = find_block_job(device, &aio_context, errp); + BlockJob *job = find_block_job(device, errp); if (!job) { return; } block_job_set_speed(job, speed, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_cancel(const char *device, bool has_force, bool force, Error **errp) { - AioContext *aio_context; - BlockJob *job = find_block_job(device, &aio_context, errp); + BlockJob *job = find_block_job(device, errp); if (!job) { return; @@ -3378,13 +3373,12 @@ void qmp_block_job_cancel(const char *device, trace_qmp_block_job_cancel(job); job_user_cancel(&job->job, force, errp); out: - aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_pause(const char *device, Error **errp) { - AioContext *aio_context; - BlockJob *job = find_block_job(device, &aio_context, errp); + BlockJob *job = find_block_job(device, errp); if (!job) { return; @@ -3392,13 +3386,12 @@ void qmp_block_job_pause(const char *device, Error **errp) trace_qmp_block_job_pause(job); job_user_pause(&job->job, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_resume(const char *device, Error **errp) { - AioContext *aio_context; - BlockJob *job = find_block_job(device, &aio_context, errp); + BlockJob *job = find_block_job(device, errp); if (!job) { return; @@ -3406,13 +3399,12 @@ void qmp_block_job_resume(const char *device, Error **errp) trace_qmp_block_job_resume(job); job_user_resume(&job->job, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_complete(const char *device, Error **errp) { - AioContext *aio_context; - BlockJob *job = find_block_job(device, &aio_context, errp); + BlockJob *job = find_block_job(device, errp); if (!job) { return; @@ -3420,13 +3412,12 @@ void qmp_block_job_complete(const char *device, Error **errp) trace_qmp_block_job_complete(job); job_complete(&job->job, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_finalize(const char *id, Error **errp) { - AioContext *aio_context; - BlockJob *job = find_block_job(id, &aio_context, errp); + BlockJob *job = find_block_job(id, errp); if (!job) { return; @@ -3436,20 +3427,13 @@ void qmp_block_job_finalize(const char *id, Error **errp) job_ref(&job->job); job_finalize(&job->job, errp); - /* - * Job's context might have changed via job_finalize (and job_txn_apply - * automatically acquires the new one), so make sure we release the correct - * one. - */ - aio_context = blk_get_aio_context(job->blk); job_unref(&job->job); - aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_dismiss(const char *id, Error **errp) { - AioContext *aio_context; - BlockJob *bjob = find_block_job(id, &aio_context, errp); + BlockJob *bjob = find_block_job(id, errp); Job *job; if (!bjob) { @@ -3459,7 +3443,7 @@ void qmp_block_job_dismiss(const char *id, Error **errp) trace_qmp_block_job_dismiss(bjob); job = &bjob->job; job_dismiss(&job, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_change_backing_file(const char *device, diff --git a/job-qmp.c b/job-qmp.c index a355dc2954..d592780953 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -29,29 +29,26 @@ #include "qapi/error.h" #include "trace/trace-root.h" -/* Get a job using its ID and acquire its AioContext */ -static Job *find_job(const char *id, AioContext **aio_context, Error **errp) +/* Get a job using its ID. Returns with job_lock held on success. */ +static Job *find_job(const char *id, Error **errp) { Job *job; - *aio_context = NULL; + job_lock(); job = job_get(id); if (!job) { error_setg(errp, "Job not found"); + job_unlock(); return NULL; } - *aio_context = job->aio_context; - aio_context_acquire(*aio_context); - return job; } void qmp_job_cancel(const char *id, Error **errp) { - AioContext *aio_context; - Job *job = find_job(id, &aio_context, errp); + Job *job = find_job(id, errp); if (!job) { return; @@ -59,13 +56,12 @@ void qmp_job_cancel(const char *id, Error **errp) trace_qmp_job_cancel(job); job_user_cancel(job, true, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_job_pause(const char *id, Error **errp) { - AioContext *aio_context; - Job *job = find_job(id, &aio_context, errp); + Job *job = find_job(id, errp); if (!job) { return; @@ -73,13 +69,12 @@ void qmp_job_pause(const char *id, Error **errp) trace_qmp_job_pause(job); job_user_pause(job, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_job_resume(const char *id, Error **errp) { - AioContext *aio_context; - Job *job = find_job(id, &aio_context, errp); + Job *job = find_job(id, errp); if (!job) { return; @@ -87,13 +82,12 @@ void qmp_job_resume(const char *id, Error **errp) trace_qmp_job_resume(job); job_user_resume(job, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_job_complete(const char *id, Error **errp) { - AioContext *aio_context; - Job *job = find_job(id, &aio_context, errp); + Job *job = find_job(id, errp); if (!job) { return; @@ -101,13 +95,12 @@ void qmp_job_complete(const char *id, Error **errp) trace_qmp_job_complete(job); job_complete(job, errp); - aio_context_release(aio_context); + job_unlock(); } void qmp_job_finalize(const char *id, Error **errp) { - AioContext *aio_context; - Job *job = find_job(id, &aio_context, errp); + Job *job = find_job(id, errp); if (!job) { return; @@ -117,20 +110,13 @@ void qmp_job_finalize(const char *id, Error **errp) job_ref(job); job_finalize(job, errp); - /* - * Job's context might have changed via job_finalize (and job_txn_apply - * automatically acquires the new one), so make sure we release the correct - * one. - */ - aio_context = job->aio_context; job_unref(job); - aio_context_release(aio_context); + job_unlock(); } void qmp_job_dismiss(const char *id, Error **errp) { - AioContext *aio_context; - Job *job = find_job(id, &aio_context, errp); + Job *job = find_job(id, errp); if (!job) { return; @@ -138,7 +124,7 @@ void qmp_job_dismiss(const char *id, Error **errp) trace_qmp_job_dismiss(job); job_dismiss(&job, errp); - aio_context_release(aio_context); + job_unlock(); } static JobInfo *job_query_single(Job *job, Error **errp) From patchwork Fri Oct 29 16:39:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1548092 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=jUG5uIfj; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HgpMS58h1z9sX3 for ; Sat, 30 Oct 2021 03:50:28 +1100 (AEDT) Received: from localhost ([::1]:35386 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mgV5E-00054f-Ix for incoming@patchwork.ozlabs.org; Fri, 29 Oct 2021 12:50:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48878) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvL-00070O-Mf for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:21 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:39604) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mgUvB-0003iS-Uk for qemu-devel@nongnu.org; Fri, 29 Oct 2021 12:40:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635525596; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5x1ge5Bl3AXmiSxRACGPuPQrdAgUEDdBvy7BWEZ0Enc=; b=jUG5uIfjc0McfSlvhHFfDEVphUH2S1Y56US8DWnsVH2oUcYe8HYtI/1IiK8LGw9TyuBbUn gxXVpDolL92HCNcWR6P0bbPNmFmDEq34TnhELlIwBdNmyNKOL8r5mvd9GrGp9Qd5H1Jll7 UAY+jL9we7TBPfZ6xSbiVG7HMOZC+fM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-312-_JGQNN8RMECBg0fqA1hO2A-1; Fri, 29 Oct 2021 12:39:52 -0400 X-MC-Unique: _JGQNN8RMECBg0fqA1hO2A-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 47BFC806688; Fri, 29 Oct 2021 16:39:51 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5758169117; Fri, 29 Oct 2021 16:39:50 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [RFC PATCH 15/15] jobs: remove all unnecessary AioContext locks Date: Fri, 29 Oct 2021 12:39:14 -0400 Message-Id: <20211029163914.4044794-16-eesposit@redhat.com> In-Reply-To: <20211029163914.4044794-1-eesposit@redhat.com> References: <20211029163914.4044794-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Emanuele Giuseppe Esposito , Markus Armbruster , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Now that we removed the aiocontext in find_* functions, we need to remove it also in all other functions of the job API that assumed the lock was held. Therefore: - when possible, remove completely the aiocontext lock/unlock pair - if it is used by some other funcitons too, just leave the job API outside the aiocontext lock by adding unlock/lock pairs. There is only one JobDriver callback, ->free() that assumes that the aiocontext lock is held (because it calls bdrv_unref), so for now keep that under aiocontext lock. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job-monitor.h | 6 ---- block/replication.c | 2 ++ blockdev.c | 19 ----------- job.c | 57 ++++---------------------------- tests/unit/test-bdrv-drain.c | 4 +-- tests/unit/test-block-iothread.c | 2 +- tests/unit/test-blockjob.c | 13 ++------ 7 files changed, 14 insertions(+), 89 deletions(-) diff --git a/include/qemu/job-monitor.h b/include/qemu/job-monitor.h index f473bd298f..4ba7e503d1 100644 --- a/include/qemu/job-monitor.h +++ b/include/qemu/job-monitor.h @@ -207,8 +207,6 @@ void job_user_cancel(Job *job, bool force, Error **errp); * * Returns the return value from the job if the job actually completed * during the call, or -ECANCELED if it was canceled. - * - * Callers must hold the AioContext lock of job->aio_context. */ int job_cancel_sync(Job *job, bool force); @@ -233,8 +231,6 @@ void job_cancel_sync_all(void); * * Returns the return value from the job. * - * Callers must hold the AioContext lock of job->aio_context. - * * Called between job_lock and job_unlock. */ int job_complete_sync(Job *job, Error **errp); @@ -266,8 +262,6 @@ void job_dismiss(Job **job, Error **errp); * Returns 0 if the job is successfully completed, -ECANCELED if the job was * cancelled before completing, and -errno in other error cases. * - * Callers must hold the AioContext lock of job->aio_context. - * * Called between job_lock and job_unlock, but it releases the lock temporarly. */ int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp); diff --git a/block/replication.c b/block/replication.c index 0f487cc215..6a60c1af1a 100644 --- a/block/replication.c +++ b/block/replication.c @@ -728,9 +728,11 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp) * disk, secondary disk in backup_job_completed(). */ if (s->backup_job) { + aio_context_release(aio_context); job_lock(); job_cancel_sync(&s->backup_job->job, true); job_unlock(); + aio_context_acquire(aio_context); } if (!failover) { diff --git a/blockdev.c b/blockdev.c index 592ed4a0fc..dfc73ef1bf 100644 --- a/blockdev.c +++ b/blockdev.c @@ -154,12 +154,7 @@ void blockdev_mark_auto_del(BlockBackend *blk) for (job = block_job_next(NULL); job; job = block_job_next(job)) { if (block_job_has_bdrv(job, blk_bs(blk))) { - AioContext *aio_context = job->job.aio_context; - aio_context_acquire(aio_context); - job_cancel(&job->job, false); - - aio_context_release(aio_context); } } @@ -1843,16 +1838,9 @@ static void drive_backup_abort(BlkActionState *common) DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); if (state->job) { - AioContext *aio_context; - - aio_context = bdrv_get_aio_context(state->bs); - aio_context_acquire(aio_context); - job_lock(); job_cancel_sync(&state->job->job, true); job_unlock(); - - aio_context_release(aio_context); } } @@ -1946,16 +1934,9 @@ static void blockdev_backup_abort(BlkActionState *common) BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common); if (state->job) { - AioContext *aio_context; - - aio_context = bdrv_get_aio_context(state->bs); - aio_context_acquire(aio_context); - job_lock(); job_cancel_sync(&state->job->job, true); job_unlock(); - - aio_context_release(aio_context); } } diff --git a/job.c b/job.c index 6b3e860fa7..124b05f769 100644 --- a/job.c +++ b/job.c @@ -168,7 +168,6 @@ static int job_txn_apply(Job *job, int fn(Job *)) * break AIO_WAIT_WHILE from within fn. */ job_ref(job); - aio_context_release(job->aio_context); QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) { rc = fn(other_job); @@ -177,11 +176,6 @@ static int job_txn_apply(Job *job, int fn(Job *)) } } - /* - * Note that job->aio_context might have been changed by calling fn, so we - * can't use a local variable to cache it. - */ - aio_context_acquire(job->aio_context); job_unref(job); return rc; } @@ -510,7 +504,10 @@ void job_unref(Job *job) if (job->driver->free) { job_unlock(); + /* FIXME: aiocontext lock is required because cb calls blk_unref */ + aio_context_acquire(job->aio_context); job->driver->free(job); + aio_context_release(job->aio_context); job_lock(); } @@ -978,7 +975,6 @@ static void job_cancel_async(Job *job, bool force) /* Called with job_mutex held. */ static void job_completed_txn_abort(Job *job) { - AioContext *ctx; JobTxn *txn = job->txn; Job *other_job; @@ -991,54 +987,28 @@ static void job_completed_txn_abort(Job *job) txn->aborting = true; job_txn_ref(txn); - /* - * We can only hold the single job's AioContext lock while calling - * job_finalize_single() because the finalization callbacks can involve - * calls of AIO_WAIT_WHILE(), which could deadlock otherwise. - * Note that the job's AioContext may change when it is finalized. - */ - job_ref(job); - aio_context_release(job->aio_context); - /* Other jobs are effectively cancelled by us, set the status for * them; this job, however, may or may not be cancelled, depending * on the caller, so leave it. */ QLIST_FOREACH(other_job, &txn->jobs, txn_list) { if (other_job != job) { - ctx = other_job->aio_context; - aio_context_acquire(ctx); /* * This is a transaction: If one job failed, no result will matter. * Therefore, pass force=true to terminate all other jobs as quickly * as possible. */ job_cancel_async(other_job, true); - aio_context_release(ctx); } } while (!QLIST_EMPTY(&txn->jobs)) { other_job = QLIST_FIRST(&txn->jobs); - /* - * The job's AioContext may change, so store it in @ctx so we - * release the same context that we have acquired before. - */ - ctx = other_job->aio_context; - aio_context_acquire(ctx); if (!job_is_completed(other_job)) { assert(job_cancel_requested_locked(other_job)); job_finish_sync(other_job, NULL, NULL); } job_finalize_single(other_job); - aio_context_release(ctx); } - /* - * Use job_ref()/job_unref() so we can read the AioContext here - * even if the job went away during job_finalize_single(). - */ - aio_context_acquire(job->aio_context); - job_unref(job); - job_txn_unref(txn); } @@ -1159,11 +1129,8 @@ static void job_completed(Job *job) static void job_exit(void *opaque) { Job *job = (Job *)opaque; - AioContext *ctx; job_lock(); - job_ref(job); - aio_context_acquire(job->aio_context); /* This is a lie, we're not quiescent, but still doing the completion * callbacks. However, completion callbacks tend to involve operations that @@ -1174,16 +1141,7 @@ static void job_exit(void *opaque) job_completed(job); - /* - * Note that calling job_completed can move the job to a different - * aio_context, so we cannot cache from above. job_txn_apply takes care of - * acquiring the new lock, and we ref/unref to avoid job_completed freeing - * the job underneath us. - */ - ctx = job->aio_context; - job_unref(job); job_unlock(); - aio_context_release(ctx); } /** @@ -1297,14 +1255,10 @@ int job_cancel_sync(Job *job, bool force) void job_cancel_sync_all(void) { Job *job; - AioContext *aio_context; job_lock(); while ((job = job_next(NULL))) { - aio_context = job->aio_context; - aio_context_acquire(aio_context); job_cancel_sync(job, true); - aio_context_release(aio_context); } job_unlock(); } @@ -1350,8 +1304,9 @@ int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp) } job_unlock(); - AIO_WAIT_WHILE(job->aio_context, - (job_enter(job), !job_is_completed_unlocked(job))); + AIO_WAIT_WHILE_UNLOCKED(job->aio_context, + (job_enter(job), + !job_is_completed_unlocked(job))); job_lock(); ret = (job_is_cancelled_locked(job) && job->ret == 0) ? diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 535c39b5a8..83485a33aa 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -928,9 +928,9 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, tjob->prepare_ret = -EIO; break; } + aio_context_release(ctx); job_start(&job->job); - aio_context_release(ctx); if (use_iothread) { /* job_co_entry() is run in the I/O thread, wait for the actual job @@ -994,12 +994,12 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, g_assert_false(job_get_paused(&job->job)); g_assert_true(job_get_busy(&job->job)); /* We're in qemu_co_sleep_ns() */ - aio_context_acquire(ctx); job_lock(); ret = job_complete_sync(&job->job, &error_abort); job_unlock(); g_assert_cmpint(ret, ==, (result == TEST_JOB_SUCCESS ? 0 : -EIO)); + aio_context_acquire(ctx); if (use_iothread) { blk_set_aio_context(blk_src, qemu_get_aio_context(), &error_abort); assert(blk_get_aio_context(blk_target) == qemu_get_aio_context()); diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c index f39cb8b7ef..cf197347b7 100644 --- a/tests/unit/test-block-iothread.c +++ b/tests/unit/test-block-iothread.c @@ -455,10 +455,10 @@ static void test_attach_blockjob(void) aio_poll(qemu_get_aio_context(), false); } - aio_context_acquire(ctx); job_lock(); job_complete_sync(&tjob->common.job, &error_abort); job_unlock(); + aio_context_acquire(ctx); blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort); aio_context_release(ctx); diff --git a/tests/unit/test-blockjob.c b/tests/unit/test-blockjob.c index b94e1510c9..11cff70a6b 100644 --- a/tests/unit/test-blockjob.c +++ b/tests/unit/test-blockjob.c @@ -228,10 +228,6 @@ static void cancel_common(CancelJob *s) BlockJob *job = &s->common; BlockBackend *blk = s->blk; JobStatus sts = job->job.status; - AioContext *ctx; - - ctx = job->job.aio_context; - aio_context_acquire(ctx); job_lock(); job_cancel_sync(&job->job, true); @@ -244,7 +240,6 @@ static void cancel_common(CancelJob *s) job_unlock(); destroy_blk(blk); - aio_context_release(ctx); } static void test_cancel_created(void) @@ -381,11 +376,9 @@ static void test_cancel_concluded(void) aio_poll(qemu_get_aio_context(), true); assert(job_get_status(job) == JOB_STATUS_PENDING); - aio_context_acquire(job->aio_context); job_lock(); job_finalize(job, &error_abort); job_unlock(); - aio_context_release(job->aio_context); assert(job_get_status(job) == JOB_STATUS_CONCLUDED); cancel_common(s); @@ -478,9 +471,7 @@ static void test_complete_in_standby(void) /* Wait for the job to become READY */ job_start(job); - aio_context_acquire(ctx); - AIO_WAIT_WHILE(ctx, job_get_status(job) != JOB_STATUS_READY); - aio_context_release(ctx); + AIO_WAIT_WHILE_UNLOCKED(ctx, job_get_status(job) != JOB_STATUS_READY); /* Begin the drained section, pausing the job */ bdrv_drain_all_begin(); @@ -498,6 +489,7 @@ static void test_complete_in_standby(void) job_complete(job, &error_abort); /* The test is done now, clean up. */ + aio_context_release(ctx); job_finish_sync(job, NULL, &error_abort); assert(job->status == JOB_STATUS_PENDING); @@ -507,6 +499,7 @@ static void test_complete_in_standby(void) job_dismiss(&job, &error_abort); job_unlock(); + aio_context_acquire(ctx); destroy_blk(blk); aio_context_release(ctx); iothread_join(iothread);