Re: [PATCH 2/2] net: thunderbolt: Fix lockdep issue during tunnel tear down

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Tue, Apr 28, 2026 at 06:12:26PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
>     net: thunderbolt: Fix lockdep issue during tunnel tear down
> 
> This commit attempts to fix a circular locking dependency warning in
> tbnet_tear_down() by dropping the connection lock earlier and returning if the
> connection isn't fully established.
> 
> > diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> > index 631af237998d..d8fcf18fc55c 100644
> > --- a/drivers/net/thunderbolt/main.c
> > +++ b/drivers/net/thunderbolt/main.c
> [ ... ]
> > @@ -372,48 +372,47 @@ static void tbnet_free_buffers(struct tbnet_ring *ring)
> >  
> >  static void tbnet_tear_down(struct tbnet *net, bool send_logout)
> >  {
> > +	int ret, retries = TBNET_LOGOUT_RETRIES;
> > +
> >  	netif_carrier_off(net->dev);
> >  	netif_stop_queue(net->dev);
> >  
> >  	stop_login(net);
> >  
> >  	mutex_lock(&net->connection_lock);
> > +	if (!net->login_sent || !net->login_received) {
> > +		mutex_unlock(&net->connection_lock);
> > +		return;
> > +	}
> 
> If the connection is in a partially established state (e.g. login_sent is
> true, but login_received is false), this early return skips clearing the
> connection states.
> 
> Could this stall the connection if a new TBIP_LOGIN arrives later?
> tbnet_handle_packet() would see net->login_sent as true and might fail to
> queue the login_work to reply.

I don't think that matters because tbnet_tear_down() is actually tearing
down the connection so after this we do not expect the connection to be
established until start_login() is called again (this happens when the
interface is brought up again or on resume).

> > +	net->login_sent = false;
> > +	net->login_received = false;
> > +	mutex_unlock(&net->connection_lock);
> 
> By releasing net->connection_lock here, the rest of the teardown process runs
> without mutual exclusion. Since the network interface is still running, is it
> possible for tbnet_handle_packet() to receive a new TBIP_LOGIN request right
> after this lock is dropped?
> 
> If tbnet_handle_packet() acquires the lock, updates net->remote_transmit_path
> and brings up a new connection concurrently:
> 
> CPU1
> tbnet_tear_down()
>    mutex_unlock(&net->connection_lock);
> 
> CPU2
> tbnet_handle_packet()
>    mutex_lock(&net->connection_lock);
>    net->remote_transmit_path = pkg->transmit_path;
>    mutex_unlock(&net->connection_lock);
> 
> CPU1
>    tb_xdomain_release_in_hopid(net->xd, net->remote_transmit_path);
> 
> Would CPU1 then erroneously release the newly established connection's hopid
> and free active buffers?

Oh, this one definitely can happen.

> >  
> > -	if (net->login_sent && net->login_received) {
> > -		int ret, retries = TBNET_LOGOUT_RETRIES;
> [ ... ]
> > -		tb_xdomain_release_in_hopid(net->xd, net->remote_transmit_path);
> > -		net->remote_transmit_path = 0;
> > -	}
> > +	tb_xdomain_release_in_hopid(net->xd, net->remote_transmit_path);
> > +	net->remote_transmit_path = 0;
> >  
> > -	net->login_retries = 0;
> 
> With the unconditional clearing of net->login_retries removed, does the
> login_retries counter keep accumulating if tbnet_tear_down() returns early
> above?
> 
> If it exceeds TBNET_LOGIN_RETRIES, could it permanently prevent new
> connections until the module is reloaded?

Indeed, that's an accidental removal.

Okay let me try to figure out better fix for this lockdep issue.