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=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 5CC46C433E0 for ; Mon, 22 Jun 2020 08:39:00 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 18DE4206C3 for ; Mon, 22 Jun 2020 08:39:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 18DE4206C3 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 B3B016B0002; Mon, 22 Jun 2020 04:38:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B1B0D6B0005; Mon, 22 Jun 2020 04:38:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A28F06B0006; Mon, 22 Jun 2020 04:38:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0192.hostedemail.com [216.40.44.192]) by kanga.kvack.org (Postfix) with ESMTP id 8AEF96B0002 for ; Mon, 22 Jun 2020 04:38:59 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 36B008248047 for ; Mon, 22 Jun 2020 08:38:59 +0000 (UTC) X-FDA: 76956197598.09.crime96_1001c2026e31 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin09.hostedemail.com (Postfix) with ESMTP id 0E860180AD807 for ; Mon, 22 Jun 2020 08:38:59 +0000 (UTC) X-HE-Tag: crime96_1001c2026e31 X-Filterd-Recvd-Size: 8533 Received: from esa6.hc3370-68.iphmx.com (esa6.hc3370-68.iphmx.com [216.71.155.175]) by imf16.hostedemail.com (Postfix) with ESMTP for ; Mon, 22 Jun 2020 08:38:58 +0000 (UTC) Authentication-Results: esa6.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: 4TpWVnbtiqhHqTm8UcF3OpAb2ZHi7hzxMchVDlx84oPPC2OsOAjK2SGuA51o1ODupl2ywABbKs sTGTZwEKCHOWRk3j67f09FvjcIBqouRt4xz0ohd5suQxkDHg1yk+MHmJxOxAluIGqpnqFIF2OH 04YAJ30lc/a5kSPPrGakF2TBnKOjBMH1fUOJ6TzqIMxiVO3Q14HqKHl9cdBd1jOqInig3Z6IN1 SRgzwpiMw315v8aBPVr76xaemjJo64jKEZKyF3QQUDhxS0lx2+OvdOzT0m389dE4n5hBxoFbH/ 0mM= X-SBRS: 2.7 X-MesageID: 20942116 X-Ironport-Server: esa6.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.75,266,1589256000"; d="scan'208";a="20942116" Date: Mon, 22 Jun 2020 10:38:46 +0200 From: Roger Pau =?utf-8?B?TW9ubsOp?= To: Anchal Agarwal CC: Boris Ostrovsky , "tglx@linutronix.de" , "mingo@redhat.com" , "bp@alien8.de" , "hpa@zytor.com" , "x86@kernel.org" , "jgross@suse.com" , "linux-pm@vger.kernel.org" , "linux-mm@kvack.org" , "Kamata, Munehisa" , "sstabellini@kernel.org" , "konrad.wilk@oracle.com" , "axboe@kernel.dk" , "davem@davemloft.net" , "rjw@rjwysocki.net" , "len.brown@intel.com" , "pavel@ucw.cz" , "peterz@infradead.org" , "Valentin, Eduardo" , "Singh, Balbir" , "xen-devel@lists.xenproject.org" , "vkuznets@redhat.com" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Woodhouse, David" , "benh@kernel.crashing.org" Subject: Re: [PATCH 06/12] xen-blkfront: add callbacks for PM suspend and hibernation] Message-ID: <20200622083846.GF735@Air-de-Roger> References: <7FD7505E-79AA-43F6-8D5F-7A2567F333AB@amazon.com> <20200604070548.GH1195@Air-de-Roger> <20200616214925.GA21684@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> <20200617083528.GW735@Air-de-Roger> <20200619234312.GA24846@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline In-Reply-To: <20200619234312.GA24846@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> X-ClientProxiedBy: AMSPEX02CAS01.citrite.net (10.69.22.112) To AMSPEX02CL02.citrite.net (10.69.22.126) X-Rspamd-Queue-Id: 0E860180AD807 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam04 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 Fri, Jun 19, 2020 at 11:43:12PM +0000, Anchal Agarwal wrote: > On Wed, Jun 17, 2020 at 10:35:28AM +0200, Roger Pau Monn=C3=A9 wrote: > > CAUTION: This email originated from outside of the organization. Do n= ot click links or open attachments unless you can confirm the sender and = know the content is safe. > >=20 > >=20 > >=20 > > On Tue, Jun 16, 2020 at 09:49:25PM +0000, Anchal Agarwal wrote: > > > On Thu, Jun 04, 2020 at 09:05:48AM +0200, Roger Pau Monn=C3=A9 wrot= e: > > > > CAUTION: This email originated from outside of the organization. = Do not click links or open attachments unless you can confirm the sender = and know the content is safe. > > > > On Wed, Jun 03, 2020 at 11:33:52PM +0000, Agarwal, Anchal wrote: > > > > > CAUTION: This email originated from outside of the organizatio= n. Do not click links or open attachments unless you can confirm the send= er and know the content is safe. > > > > > > + xenbus_dev_error(dev, err, "Freezing timed = out;" > > > > > > + "the device may become inc= onsistent state"); > > > > > > > > > > Leaving the device in this state is quite bad, as it's in a= closed > > > > > state and with the queues frozen. You should make an attemp= t to > > > > > restore things to a working state. > > > > > > > > > > You mean if backend closed after timeout? Is there a way to kno= w that? I understand it's not good to > > > > > leave it in this state however, I am still trying to find if th= ere is a good way to know if backend is still connected after timeout. > > > > > Hence the message " the device may become inconsistent state". = I didn't see a timeout not even once on my end so that's why > > > > > I may be looking for an alternate perspective here. may be need= to thaw everything back intentionally is one thing I could think of. > > > > > > > > You can manually force this state, and then check that it will be= have > > > > correctly. I would expect that on a failure to disconnect from th= e > > > > backend you should switch the frontend to the 'Init' state in ord= er to > > > > try to reconnect to the backend when possible. > > > > > > > From what I understand forcing manually is, failing the freeze with= out > > > disconnect and try to revive the connection by unfreezing the > > > queues->reconnecting to backend [which never got diconnected]. May = be even > > > tearing down things manually because I am not sure what state will = frontend > > > see if backend fails to to disconnect at any point in time. I assum= ed connected. > > > Then again if its "CONNECTED" I may not need to tear down everythin= g and start > > > from Initialising state because that may not work. > > > > > > So I am not so sure about backend's state so much, lets say if xen= _blkif_disconnect fail, > > > I don't see it getting handled in the backend then what will be bac= kend's state? > > > Will it still switch xenbus state to 'Closed'? If not what will fro= ntend see, > > > if it tries to read backend's state through xenbus_read_driver_stat= e ? > > > > > > So the flow be like: > > > Front end marks XenbusStateClosing > > > Backend marks its state as XenbusStateClosing > > > Frontend marks XenbusStateClosed > > > Backend disconnects calls xen_blkif_disconnect > > > Backend fails to disconnect, the above function returns EBUS= Y > > > What will be state of backend here? > >=20 > > Backend should stay in state 'Closing' then, until it can finish > > tearing down. > >=20 > It disconnects the ring after switching to connected state too.=20 > > > Frontend did not tear down the rings if backend does not swi= tches the > > > state to 'Closed' in case of failure. > > > > > > If backend stays in CONNECTED state, then even if we mark it Initia= lised in frontend, backend > >=20 > > Backend will stay in state 'Closing' I think. > >=20 > > > won't be calling connect(). {From reading code in frontend_changed} > > > IMU, Initialising will fail since backend dev->state !=3D XenbusSta= teClosed plus > > > we did not tear down anything so calling talk_to_blkback may not be= needed > > > > > > Does that sound correct? > >=20 > > I think switching to the initial state in order to try to attempt a > > reconnection would be our best bet here. > > > It does not seems to work correctly, I get hung tasks all over and all = the > requests to filesystem gets stuck. Backend does shows the state as conn= ected > after xenbus_dev_suspend fails but I think there may be something missi= ng. > I don't seem to get IO interrupts thereafter i.e hitting the function b= lkif_interrupts. > I think just marking it initialised may not be the only thing. > Here is a short description of what I am trying to do: > So, on timeout: > Switch XenBusState to "Initialized" > unquiesce/unfreeze the queues and return > mark info->connected =3D BLKIF_STATE_CONNECTED If xenbus state is Initialized isn't it wrong to set info->connected =3D=3D CONNECTED? You should tear down all the internal state (like a proper close)? > return EBUSY >=20 > I even allowed blkfront_connect to switch state to "CONNECTED" rather m= e doing > it explicitly as mentioned above without re-allocating/re-registering t= he device > just to make sure bklfront_info object has all the right values. > Do you see anythign missing here? I'm afraid you will have to do a little bit of debugging here to figure out what's going on. You can add printk's to several places to see which path is taken, and why blkfront ends in such state. Thanks, Roger.