WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: [PATCH] pvops: Resume devices when suspend is failed

To: rshriram@xxxxxxxxx
Subject: Re: [Xen-devel] Re: [PATCH] pvops: Resume devices when suspend is failed
From: Frank Pan <frankpzh@xxxxxxxxx>
Date: Thu, 10 Mar 2011 18:42:58 +0800
Cc: Jeremy Fitzhardinge <Jeremy.Fitzhardinge@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Ian Campbell <Ian.Campbell@xxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Thu, 10 Mar 2011 02:44:02 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=n0x2KAryOJ57u4cg4s1kOIEbOJVQC4dv5kDHvzahAl8=; b=nXa+oS1utLy7rbg33cHeknh2p3xBfA/r/KwJamaiZNWXhZREie2X5qOV4AM48VglpW 42I7jKC1JpOIObI8d8rsarM0phH0tvE/U54Ncu88AKxcc1oImNtB/Av69dpJeEa42qMg vS+kL6bT+oD0T/P3oF14fVZdE/Ng/oKmwDwxI=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=NdYj/AlZopVOGFjCrE7+QZdS/7bYX4QEniAzgnRfZlbUP+Gw5nBVvO2p7HgDWES/hs MElVgqfUlXSD46AFTcV8Dq8p58X6CdWjDhjGwiSfJHcHLNqVu3nZ0bEtMn4K51VRDlac +RdyF6qLgjUOL8GCUvlg4nQCOJLFaU8tJuUZg=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <AANLkTinCU4w-Ec8teQii8de93Yb6jW4S09cRa3wPnX4U@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <AANLkTinXnNK+75Nye713q5jzuXAd3+2iwvZSJBgfiiFi@xxxxxxxxxxxxxx> <alpine.DEB.2.00.1103071718550.2968@kaball-desktop> <1299580348.17339.452.camel@xxxxxxxxxxxxxxxxxxxxxx> <AANLkTinCU4w-Ec8teQii8de93Yb6jW4S09cRa3wPnX4U@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Yes, that's what I doubt.

I write dpm_resume_end() inside the if clause just because this reason.

What if we call clock_was_set() before dpm_resume_end() ?
Is there any device need timer intr during resume?

IOW:

  if (dpm_suspend(..))
    goto out_thaw;
  xs_suspend()
  if (dpm_suspend_noirq())
    goto out_resume;
  xen_suspend()
  dpm_resume_noirq()

out_resume:
  xen_arch_resume()
  xs_resume()
  clock_was_set()

out_thaw:
  dpm_resume_end()
  return

On Wed, Mar 9, 2011 at 5:09 AM, Shriram Rajagopalan <rshriram@xxxxxxxxx> wrote:
> On Tue, Mar 8, 2011 at 2:32 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> wrote:
>>
>> On Mon, 2011-03-07 at 17:22 +0000, Stefano Stabellini wrote:
>> > On Fri, 4 Mar 2011, Frank Pan wrote:
>> > > Recent pvops kernel does not call dpm_resume_end when
>> > > dpm_suspend_start is failed. This makes some device remain suspended
>> > > after the unsuccessful call of do_suspend from xenbus.
>> > >
>> > > In my test, a PV-on-HVM guest printed the following message after
>> > > received a suspend request through xenbus, and then stucked due to
>> > > disk access.
>> > >
>> > > [41577.764748] sd 0:0:0:0: [sda] Stopping disk
>> > > [41577.765273] PM: Device input2 failed to suspend: error -22
>> > > [41577.765275] xen suspend: dpm_suspend_start -22
>> > >
>> > > The following patch fixes this by calling dpm_suspend_start after the
>> > > failure of dpm_resume_end.
>>
>> > Thanks for spotting this issue and for the patch!
>> > However I think it would be better to move out_thaw before
>> > dpm_resume_end instead.
>>
> FYI, the code flow after Frank's patch
>  if (dpm_suspend(..)) {
>   dpm_resume_end()
>   goto out_thaw;
> }
> xs_suspend()
> if (dpm_suspend_noirq())
>   goto out_resume;
> xen_suspend()
> dpm_resume_noirq()
>
> out_resume:
> xen_arch_resume()
> xs_resume()
> dpm_resume_end()
> clock_was_set()
>
> out_thaw:
>       return
>
> I am not sure if the clock_was_set() call is harmless (retriggering the
> timers),
> even if no dpm_suspend_noirq() calls were issued.
>>
>> ACK.
>>
>> This will likely interact with Shriram's recent changes to the PMSG_*
>> types used by Xen save/restore. Ideally this patch would be against
>> those.
>>
>> Also the choice of PMSG_* passed to dpm_resume_end will likely depend on
>> whether dpm_suspend_start succeeded or failed. Based on looking at
>> hibernate.c I think, after Shriram's patches, it needs to be
>> PMSG_RECOVER in the error case, PMSG_THAW in the checkpoint case and
>> PMSG_RESTORE in the normal resume case but you should check the
>> descriptions in pm.h to be sure.
>>
>> Ian.
>>
>> > Would you be OK to do that and send a patch?
>> >
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@xxxxxxxxxxxxxxxxxxx
>> > http://lists.xensource.com/xen-devel
>>
>>
>
>



-- 
潘震皓, Frank Pan

Computer Science and Technology
Tsinghua University

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>