Re: [PATCH v2 15/28] thunderbolt: Deactivate all paths before restarting them

From: Mika Westerberg
Date: Mon Feb 11 2019 - 03:52:10 EST


On Mon, Feb 11, 2019 at 07:35:55AM +0100, Lukas Wunner wrote:
> On Wed, Feb 06, 2019 at 04:17:25PM +0300, Mika Westerberg wrote:
> > We can't be sure the paths are actually properly deactivated when a
> > tunnel is restarted after resume.
>
> Why can't we be sure? Please provide proper reasoning.

IIRC the reason was that when you suspend, then reconfigure parts of the
topology and resume, establishing the tunnel again went wrong. I'll
update the changelog with a better reasoning.

> > So instead of marking all paths as
> > inactive we go ahead and deactivate them explicitly.
>
> This seems like a bad idea if the root partition is on a Thunderbolt-
> attached drive, the system is waking from hibernate and the EFI NHI
> driver has already established a tunnel to that drive. It would seem
> more appropriate to discover tunnels already existing on resume from
> system sleep and then attempt to establish any others that might be
> missing.

That's what we do in the patch following, no? We discover the EFI
created paths and use that information to re-establish tunnels upon S3
resume and also when they are torn down.

> > @@ -183,8 +183,15 @@ int tb_tunnel_restart(struct tb_tunnel *tunnel)
> >
> > tb_tunnel_info(tunnel, "activating\n");
> >
> > + /* Make sure all paths are properly disabled before enable them again */
>
> This isn't proper English, s/enable/enabling/.

Thanks, I'll fix it up.