From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 71B41C43334 for ; Fri, 3 Jun 2022 16:43:07 +0000 (UTC) Received: from localhost ([::1]:33748 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nxAOA-0002YO-Ev for qemu-devel@archiver.kernel.org; Fri, 03 Jun 2022 12:43:06 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53738) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nxAMC-0000Om-Pc for qemu-devel@nongnu.org; Fri, 03 Jun 2022 12:41:05 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:51619) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nxAM8-0000lq-Kn for qemu-devel@nongnu.org; Fri, 03 Jun 2022 12:41:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1654274459; 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: in-reply-to:in-reply-to:references:references; bh=0dnJm8l9nQahvZ66klwWDDcByX9qZYoZkIzEd+scaXM=; b=hAXpGkxxPfAi0S5x6UABfniZ0/T8Apg0GONDs5TaMXOkm1lp2PTwerr3L8MXsOTLU+4HOQ qxcbYvztxiEcyAvzedywZ1ZAEAz4BxW36XWT94eezD8Ki/RrcBJQVEdyd1iyGrM0RNUEaH zjOMi6Bb1We/izRS0nBjEcKo36mHRHk= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-586-x1GRpdfMMWeo2b2zp2XPqw-1; Fri, 03 Jun 2022 12:40:56 -0400 X-MC-Unique: x1GRpdfMMWeo2b2zp2XPqw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1BC4C101A54E; Fri, 3 Jun 2022 16:40:56 +0000 (UTC) Received: from redhat.com (unknown [10.39.194.36]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 10C9640CF8E7; Fri, 3 Jun 2022 16:40:53 +0000 (UTC) Date: Fri, 3 Jun 2022 18:40:52 +0200 From: Kevin Wolf To: Emanuele Giuseppe Esposito Cc: qemu-block@nongnu.org, Hanna Reitz , Paolo Bonzini , John Snow , Vladimir Sementsov-Ogievskiy , Wen Congyang , Xie Changlong , Markus Armbruster , Stefan Hajnoczi , Fam Zheng , qemu-devel@nongnu.org Subject: Re: [PATCH v6 06/18] jobs: protect jobs with job_lock/unlock Message-ID: References: <20220314133707.2206082-1-eesposit@redhat.com> <20220314133707.2206082-7-eesposit@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220314133707.2206082-7-eesposit@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.1 Received-SPF: pass client-ip=170.10.133.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben: > Introduce the job locking mechanism through the whole job API, > following the comments in job.h and requirements of job-monitor > (like the functions in job-qmp.c, assume lock is held) and > job-driver (like in mirror.c and all other JobDriver, lock is not held). > > Use the _locked helpers introduced before to differentiate > between functions called with and without job_mutex. > This only applies to function that are called under both > cases, all the others will be renamed later. > > job_{lock/unlock} is independent from real_job_{lock/unlock}. > > Note: at this stage, job_{lock/unlock} and job lock guard macros > are *nop*. > > Signed-off-by: Emanuele Giuseppe Esposito > --- > block.c | 18 ++++--- > block/replication.c | 8 ++- > blockdev.c | 17 ++++-- > blockjob.c | 56 +++++++++++++------- > job-qmp.c | 2 + > job.c | 125 +++++++++++++++++++++++++++++++------------- > monitor/qmp-cmds.c | 6 ++- > qemu-img.c | 41 +++++++++------ > 8 files changed, 187 insertions(+), 86 deletions(-) > > diff --git a/block.c b/block.c > index 718e4cae8b..5dc46fde11 100644 > --- a/block.c > +++ b/block.c > @@ -4978,7 +4978,9 @@ static void bdrv_close(BlockDriverState *bs) > > void bdrv_close_all(void) > { > - assert(job_next(NULL) == NULL); > + WITH_JOB_LOCK_GUARD() { > + assert(job_next(NULL) == NULL); > + } > GLOBAL_STATE_CODE(); This series seems really hard to review patch by patch, in this case because I would have to know whether you intended job_next() to be called with the lock held or not. Nothing in job.h indicates either way at this point in the series. Patch 11 answers this by actually renaming it job_next_locked(), but always having to refer to the final state after the whole series is applied is really not how things should work. We're splitting the work into individual patches so that the state after each single patch makes sense on its own. Otherwise the whole series could as well be a single patch. :-( So I'd argue that patch 11 should probably come before this one. Anyway, I guess I'll try to make my way to the end of the series quickly and then somehow try to verify whatever the state is then. Kevin