Re: [Xen-devel] [PATCH] xen/netfront: Remove unneeded .resume callback

[Julien Grall]

Hi,

On 3/14/19 3:40 PM, Boris Ostrovsky wrote:
> On 3/14/19 11:10 AM, Oleksandr Andrushchenko wrote:
> > On 3/14/19 5:02 PM, Boris Ostrovsky wrote:
> > > On 3/14/19 10:52 AM, Oleksandr Andrushchenko wrote:
> > > > On 3/14/19 4:47 PM, Boris Ostrovsky wrote:
> > > > > On 3/14/19 9:17 AM, Oleksandr Andrushchenko wrote:
> > > > > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> > > > > >
> > > > > > Currently on driver resume we remove all the network queues and
> > > > > > destroy shared Tx/Rx rings leaving the driver in its current state
> > > > > > and never signaling the backend of this frontend's state change.
> > > > > > This leads to the number of consequences:
> > > > > > - when frontend withdraws granted references to the rings etc. it
> > > > > > cannot
> > > > > >  ÂÂÂ be cleanly done as the backend still holds those (it was not
> > > > > > told to
> > > > > >  ÂÂÂ free the resources)
> > > > > > - it is not possible to resume driver operation as all the
> > > > > > communication
> > > > > >  ÂÂÂ means with the backned were destroyed by the frontend, thus
> > > > > >  ÂÂÂ making the frontend appear to the guest OS as functional, but
> > > > > >  ÂÂÂ not really.
> > > > > What do you mean? Are you saying that after resume you lose
> > > > > connectivity?
> > > > Exactly, if you take a look at the .resume callback as it is now
> > > > what it does it destroys the rings etc. and never notifies the backend
> > > > of that, e.g. it stays in, say, connected state with communication
> > > > channels destroyed. It never goes into any other Xen bus state, so
> > > > there is
> > > > no way its state machine can help recovering.
> > >
> > > My tree is about a month old so perhaps there is some sort of regression
> > > but this certainly works for me. After resume netfront gets
> > > XenbusStateInitWait from backend which causes xennet_connect().
> > Ah, the difference can be of the way we get the guest enter
> > the suspend state. I am making my guest to suspend with:
> > echo mem > /sys/power/state
> > And then I use an interrupt to the guest (this is a test code)
> > to wake it up.
> > Could you please share your exact use-case when the guest enters suspend
> > and what you do to resume it?
>
>
> xl save / xl restore
>
> > I can see no way backend may want enter XenbusStateInitWait in my
> > use-case
> > as it simply doesn't know we want him to.
>
>
> Yours looks like ACPI path, I don't know how well it was tested TBH.

I remember a series from amazon [1] that plays around suspend and 
hibernation. The patch [2] leads me to think that guest triggered 
suspend/resume does not work properly. It looks like the series has 
never been fully reviewed. Not sure why...

Anyway, from my understanding this series may solve Oleksandr issue. 
However, this would only address the common code side. AFAIK Oleksandr 
is targeting Arm platform. If so, I think this would require more work 
than this series. Arm code still miss few bits properly suspend/resume 
arch specific code (see [2]).

I have a branch on my git to track the series. However, they never have 
been resent after Ian Campbell left Citrix. I would be happy to review 
them if someone wants to pick them up and repost them.

Cheers,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2018-06/msg00823.html

[2] 
http://xenbits.xen.org/gitweb/?p=people/julieng/linux-arm.git;a=shortlog;h=refs/heads/xen-migration/v2

> -boris
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

--
Julien Grall