From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.3 (2019-12-06) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-1.5 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.3 Received: by atuin.qyliss.net (Postfix, from userid 496) id 24D8B9B6D; Tue, 16 Jun 2020 09:39:40 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 6C7459B62; Tue, 16 Jun 2020 09:39:34 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 496) id 1B25D9B3D; Tue, 16 Jun 2020 09:39:32 +0000 (UTC) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by atuin.qyliss.net (Postfix) with ESMTPS id C361C9B9C for ; Tue, 16 Jun 2020 09:39:29 +0000 (UTC) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 0EB3E5C00E8; Tue, 16 Jun 2020 05:39:29 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Tue, 16 Jun 2020 05:39:29 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h= from:to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; s=fm3; bh=/Pamo5aYUyRC4xS0zk5cwMunvl Ke5Y8+HgC1If6ptqo=; b=R8ZxpxyZnUDuWS655GSU1/ws4zCc89cZzNNyFTTMQs U2554POnfoqFujHiQuRPA3mW30ZqfiUGdI4RshZtAXoRIrKq/T6CNtFQ2zIF8KXg Pcz714XwpJraZz0anOx9bbObexysQcFw0nM5GuuQDS8FonrDFaqHm3SishgDM9pG zS7OG+U0ET85Yc59neST/Q2uI6yEyhyxtjozRwKvuzPWAw2PgDg6M6IQXN9rqY5s nuVhV42CQuE3odW1rCQc6DgiUJuWM69pBPuHPvqUREhs2Vq/xouEZoCCdxPM4uFJ gfPuANxLgH9Iirc4IUMp4SIKgOcmylJ9IHN5OJUF1BQQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=/Pamo5 aYUyRC4xS0zk5cwMunvlKe5Y8+HgC1If6ptqo=; b=CA1YCjglij4RjWEtlnBOZI 8WqlYdH40FJOKtoTMQu4WIZfmncERmJUF+kU0Z14WmH0/RGqNOM4O25Sby7APlFT smT2Lgt71F70rFF3QS8jPOgKNVL8lxTf75gU2c+F8C0c5BsgQyBJWiGxMLdFkila tFSoFt2cBt7ATuALCcQMG5er2GMFix29U4xHBd657iZtWdSvLR57IfRViTuLhNxr qO+DIzPYiR2gH/szkip66NqL4OukrU06I5zpkOYjfB9z+yB9rsfomUeHf4crYegS JDKXoW6hBOK7JfFIgL/rc07H8VU3gHIL99tMwHvGt4U1OCJQ/oPuyuQ7BOnI/eqg == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrudejtddgvdduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufgjfhffkfggtgesghdtreertddttdenucfhrhhomheptehlhihsshgr ucftohhsshcuoehhihesrghlhihsshgrrdhisheqnecuggftrfgrthhtvghrnhepudffie euledtheevheefieetveevfffhvddutefhiedvgfehiedtfeefleevgfdunecuffhomhgr ihhnpehsphgvtghtrhhumhdqohhsrdhorhhgnecukfhppeegiedrkedtrdduvdekrdduhe dvnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhephhhi segrlhihshhsrgdrihhs X-ME-Proxy: Received: from x220.qyliss.net (p2e508098.dip0.t-ipconnect.de [46.80.128.152]) by mail.messagingengine.com (Postfix) with ESMTPA id 4B7093060FE7; Tue, 16 Jun 2020 05:39:28 -0400 (EDT) Received: by x220.qyliss.net (Postfix, from userid 1000) id 0E66310E5; Tue, 16 Jun 2020 09:39:27 +0000 (UTC) From: Alyssa Ross To: Cole Helbling Subject: Re: [PATCH crosvm 2/2] crosvm: fix deadlock on early VmRequest In-Reply-To: References: Date: Tue, 16 Jun 2020 09:39:25 +0000 Message-ID: <877dw7qrhu.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Message-ID-Hash: FRL7V3ICESDLWKBAQN7N67R3ZILGLQHJ X-Message-ID-Hash: FRL7V3ICESDLWKBAQN7N67R3ZILGLQHJ X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header CC: devel@spectrum-os.org X-Mailman-Version: 3.3.1 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable >> @@ -1667,6 +1668,45 @@ fn file_to_i64>(path: P) -> >> io::Result { >> .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "empty file")) >> } >>=20=20 >> +/// Returns a boolean indicating whether the VM should be exited. >> +fn do_vm_request( >> + request: VmRequest, >> + device_socket: Option<&UnixSeqpacket>, >> + control_socket: &VmControlResponseSocket, >> + run_mode_arc: &Arc, >> + vcpu_handles: &mut Vec>, >> + io_bus: &mut Bus, >> +) -> MsgResult { >> + let mut run_mode_opt =3D None; >> + let response =3D request.execute(&mut run_mode_opt, device_socket); >> + control_socket.send(&response)?; >> + if let Some(run_mode) =3D run_mode_opt { >> + info!("control socket changed run mode to {}", run_mode); >> + match run_mode { >> + VmRunMode::Exiting =3D> Ok(true), >> + VmRunMode::Running =3D> { >> + if let VmRunMode::Suspending =3D *run_mode_arc.mtx.lock() { >> + io_bus.notify_resume(); >> + } >> + run_mode_arc.set_and_notify(VmRunMode::Running); >> + for handle in vcpu_handles { >> + let _ =3D handle.kill(SIGRTMIN() + 0); > > I know this is essentially just moved (and probably isn't even something > you wrote), but do you know why this is `+ 0`? Does this somehow coerce > to the desired type or something? Maybe I'm overlooking something > obvious here. I have no idea! Had a look through the history but couldn't see anything obvious. I remvoed a random + 0 I found, and nothing looked like it went wrong. As an aside, it looks like your MUA has stripped repeated whitespace in the quote above. Makes the code rather difficult to read. :P You might want to look into that. > Either way, the above is just a nit. Good work on tracking this > particular issue down! > > Reviewed-by: Cole Helbling Thanks for the review! Again, much appreciated even though I pushed already, and I'll try to add it retrospectively as a git note. It may interest you to know that this patch actually had a bug in it, which I didn't notice either until I managed to crash crosvm because of it. Here's the fix: https://spectrum-os.org/git/crosvm/commit/?id=3Dca5bdd2ac3e473e9b082c44c287= 0f446b96323a2 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEH9wgcxqlHM/ARR3h+dvtSFmyccAFAl7ok00ACgkQ+dvtSFmy ccC4uw//V0Uts/4wGxLNU9hq0wGSyvhHm5RFB/vfqHivpQinorzu2mPI8ArFzQJC X3xBX05kGG2scFR6cjnb5N641sOe5t7OzdTyVQ+0mKsSzzXeAYY6oTIfe241AgOj 5CeXiiD+eaoFb31SPEv8xSmHG4nweimyBKzm7U5zfVQ258Zszlf8162HLizu8QUX G1DShhHGWObXPx9CAHFAymEDQ5gVO2atxu04LucvkFF82bvA0Qzw0Db4mhvrkT8+ qPkm0xB6bBNLV+t0Zipad1TaNy26AcwsfDEar03Vh8Bk8R5L1BKYUq5EiJ4ieaEc L0pk//xp2IVGe6m89Psa+g5JC9xuNQJ6L82j5FiIBsBQ9xhmPFl8mCeedmH/n5Bv OZTtfHblhM0VIcTCBm2R1A9QQftUnKEJQ3GgdgW53+DzVcik5m+T3xSNT2qp4VWd n9RN7KeG5jbtN4akHBU3DGNC3ykm1uuxgEGJTf/nd3oB8At21WED4Csu7k2MO3ye ElibJFvpEdUKrn1MKq8Ke14Qjfge9yZeVBhx87AkJQrCWFDU7fixB2VCDZr1juu7 zUup0Dx7tJp0t/wCk3FQDWPKSm73hICG27jRnltmaCvBk2Qd3nrwswvO1gwru/vE 4kuB22sMJWnpnMdTJW11eE4B+ZPinHvdzTm1rD892+ftd0lzqlU= =EPV4 -----END PGP SIGNATURE----- --=-=-=--