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 X-Spam-Level: X-Spam-Status: No, score=-0.6 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, TVD_PH_BODY_ACCOUNTS_PRE autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BAEF4C11D0E for ; Thu, 20 Feb 2020 15:45:39 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 66DEF207FD for ; Thu, 20 Feb 2020 15:45:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="PJH5JuNS" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 66DEF207FD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=citrix.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id E719B6B0006; Thu, 20 Feb 2020 10:45:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E48CF6B0007; Thu, 20 Feb 2020 10:45:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D38D46B0008; Thu, 20 Feb 2020 10:45:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0111.hostedemail.com [216.40.44.111]) by kanga.kvack.org (Postfix) with ESMTP id 7A2ED6B0006 for ; Thu, 20 Feb 2020 10:45:38 -0500 (EST) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 9A2048248047 for ; Thu, 20 Feb 2020 15:45:37 +0000 (UTC) X-FDA: 76510930314.07.patch53_4e10ea758213b X-HE-Tag: patch53_4e10ea758213b X-Filterd-Recvd-Size: 10757 Received: from esa3.hc3370-68.iphmx.com (esa3.hc3370-68.iphmx.com [216.71.145.155]) by imf47.hostedemail.com (Postfix) with ESMTP for ; Thu, 20 Feb 2020 15:45:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1582213537; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=Hf2jfb9piSiHju6Ntp+FvddJlG4b1AFVZOCjx1jkZss=; b=PJH5JuNS2kQLBgCHBSIecSOHE09zj4pexuykGsQ8rjT+tOBGv0Bqq/W1 rtWSpXM51/fHxC+vvU8FKSsA9WvxoAhTnsOtQN3cADYeMixiZJURCGJ5f Dfw2cw1BFaJMnY1/+deJJEoFGGMKVHOZssGL1Btq990FziDciUZhruurv A=; Authentication-Results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=roger.pau@citrix.com; spf=Pass smtp.mailfrom=roger.pau@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa3.hc3370-68.iphmx.com: no sender authenticity information available from domain of roger.pau@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa3.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="roger.pau@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa3.hc3370-68.iphmx.com: domain of roger.pau@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa3.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="roger.pau@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa3.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa3.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: ZHDdixRenlbATRuHBknsZwWZz7xs0v0NuFZmuADao7Vcj1Y3cpXV28Hpg3tWGHyzoPqbmMUbpn 3RODTM7jZRSgjsyQkTF3P48v+0IReUEWZ/qXIH4g3s1pEH2JtscssVMtFiFSbpoQgfNeWqCMkb 0q7nIJ3+Peu47qKTW6NylrFNYzp5kHsYcD7w38iuUDqhFLh6hPmlWZvuKSAUdEpuUKProKqZLy qQu+6xMWY0Wg+XDKCXpD16MdjYaSofKLq9wdY7VLTpLCCcAeTgGxcwP8wx9THTPwEkScIK6usq D9s= X-SBRS: 2.7 X-MesageID: 12745476 X-Ironport-Server: esa3.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.70,464,1574139600"; d="scan'208";a="12745476" Date: Thu, 20 Feb 2020 16:45:07 +0100 From: Roger Pau =?utf-8?B?TW9ubsOp?= To: "Durrant, Paul" CC: "Agarwal, Anchal" , "Valentin, Eduardo" , "len.brown@intel.com" , "peterz@infradead.org" , "benh@kernel.crashing.org" , "x86@kernel.org" , "linux-mm@kvack.org" , "pavel@ucw.cz" , "hpa@zytor.com" , "tglx@linutronix.de" , "sstabellini@kernel.org" , "fllinden@amaozn.com" , "Kamata, Munehisa" , "mingo@redhat.com" , "xen-devel@lists.xenproject.org" , "Singh, Balbir" , "axboe@kernel.dk" , "konrad.wilk@oracle.com" , "bp@alien8.de" , "boris.ostrovsky@oracle.com" , "jgross@suse.com" , "netdev@vger.kernel.org" , "linux-pm@vger.kernel.org" , "rjw@rjwysocki.net" , "linux-kernel@vger.kernel.org" , "vkuznets@redhat.com" , "davem@davemloft.net" , "Woodhouse, David" Subject: Re: [Xen-devel] [RFC PATCH v3 06/12] xen-blkfront: add callbacks for PM suspend and hibernation Message-ID: <20200220154507.GO4679@Air-de-Roger> References: <890c404c585d7790514527f0c021056a7be6e748.1581721799.git.anchalag@amazon.com> <20200217100509.GE4679@Air-de-Roger> <20200217230553.GA8100@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> <20200218091611.GN4679@Air-de-Roger> <20200219180424.GA17584@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> <20200220083904.GI4679@Air-de-Roger> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: AMSPEX02CAS01.citrite.net (10.69.22.112) To AMSPEX02CL01.citrite.net (10.69.22.125) Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Feb 20, 2020 at 08:54:36AM +0000, Durrant, Paul wrote: > > -----Original Message----- > > From: Xen-devel On Behalf Of > > Roger Pau Monn=C3=A9 > > Sent: 20 February 2020 08:39 > > To: Agarwal, Anchal > > Cc: Valentin, Eduardo ; len.brown@intel.com; > > peterz@infradead.org; benh@kernel.crashing.org; x86@kernel.org; linux= - > > mm@kvack.org; pavel@ucw.cz; hpa@zytor.com; tglx@linutronix.de; > > sstabellini@kernel.org; fllinden@amaozn.com; Kamata, Munehisa > > ; mingo@redhat.com; xen-devel@lists.xenproject.or= g; > > Singh, Balbir ; axboe@kernel.dk; > > konrad.wilk@oracle.com; bp@alien8.de; boris.ostrovsky@oracle.com; > > jgross@suse.com; netdev@vger.kernel.org; linux-pm@vger.kernel.org; > > rjw@rjwysocki.net; linux-kernel@vger.kernel.org; vkuznets@redhat.com; > > davem@davemloft.net; Woodhouse, David > > Subject: Re: [Xen-devel] [RFC PATCH v3 06/12] xen-blkfront: add callb= acks > > for PM suspend and hibernation > >=20 > > Thanks for this work, please see below. > >=20 > > On Wed, Feb 19, 2020 at 06:04:24PM +0000, Anchal Agarwal wrote: > > > On Tue, Feb 18, 2020 at 10:16:11AM +0100, Roger Pau Monn=C3=A9 wrot= e: > > > > On Mon, Feb 17, 2020 at 11:05:53PM +0000, Anchal Agarwal wrote: > > > > > On Mon, Feb 17, 2020 at 11:05:09AM +0100, Roger Pau Monn=C3=A9 = wrote: > > > > > > On Fri, Feb 14, 2020 at 11:25:34PM +0000, Anchal Agarwal wrot= e: > > > > > Quiescing the queue seemed a better option here as we want to m= ake > > sure ongoing > > > > > requests dispatches are totally drained. > > > > > I should accept that some of these notion is borrowed from how = nvme > > freeze/unfreeze > > > > > is done although its not apple to apple comparison. > > > > > > > > That's fine, but I would still like to requests that you use the = same > > > > logic (as much as possible) for both the Xen and the PM initiated > > > > suspension. > > > > > > > > So you either apply this freeze/unfreeze to the Xen suspension (a= nd > > > > drop the re-issuing of requests on resume) or adapt the same appr= oach > > > > as the Xen initiated suspension. Keeping two completely different > > > > approaches to suspension / resume on blkfront is not suitable lon= g > > > > term. > > > > > > > I agree with you on overhaul of xen suspend/resume wrt blkfront is = a > > good > > > idea however, IMO that is a work for future and this patch series s= hould > > > not be blocked for it. What do you think? > >=20 > > It's not so much that I think an overhaul of suspend/resume in > > blkfront is needed, it's just that I don't want to have two completel= y > > different suspend/resume paths inside blkfront. > >=20 > > So from my PoV I think the right solution is to either use the same > > code (as much as possible) as it's currently used by Xen initiated > > suspend/resume, or to also switch Xen initiated suspension to use the > > newly introduced code. > >=20 > > Having two different approaches to suspend/resume in the same driver > > is a recipe for disaster IMO: it adds complexity by forcing developer= s > > to take into account two different suspend/resume approaches when > > there's no need for it. >=20 > I disagree. S3 or S4 suspend/resume (or perhaps we should call them pow= er state transitions to avoid confusion) are quite different from Xen sus= pend/resume. > Power state transitions ought to be, and indeed are, visible to the sof= tware running inside the guest. Applications, as well as drivers, can rec= eive notification and take whatever action they deem appropriate. > Xen suspend/resume OTOH is used when a guest is migrated and the code s= hould go to all lengths possible to make any software running inside the = guest (other than Xen specific enlightened code, such as PV drivers) comp= letely unaware that anything has actually happened. So from what you say above PM state transitions are notified to all drivers, and Xen suspend/resume is only notified to PV drivers, and here we are speaking about blkfront which is a PV driver, and should get notified in both cases. So I'm unsure why the same (or at least very similar) approach can't be used in both cases. The suspend/resume approach proposed by this patch is completely different than the one used by a xenbus initiated suspend/resume, and I don't see a technical reason that warrants this difference. I'm not saying that the approach used here is wrong, it's just that I don't see the point in having two different ways to do suspend/resume in the same driver, unless there's a technical reason for it, which I don't think has been provided. I would be fine with switching xenbus initiated suspend/resume to also use the approach proposed here: freeze the queues and drain the shared rings before suspending. > So, whilst it may be possible to use common routines to, for example, r= e-establish PV frontend/backend communication, PV frontend code should be= acutely aware of the circumstances they are operating in. I can cite exa= mple code in the Windows PV driver, which have supported guest S3/S4 powe= r state transitions since day 1. Hm, please bear with me, as I'm not sure I fully understand. Why isn't the current suspend/resume logic suitable for PM transitions? As said above, I'm happy to switch xenbus initiated suspend/resume to use the logic in this patch, but unless there's a technical reason for it I don't see why blkfront should have two completely different approaches to suspend/resume depending on whether it's a PM or a xenbus state change. Thanks, Roger.