Skip to content

Commit

Permalink
Merge branch 'mlxsw-avoid-non-tracker-helpers-when-holding-and-puttin…
Browse files Browse the repository at this point in the history
…g-netdevices'

Petr Machata says:

====================
mlxsw: Avoid non-tracker helpers when holding and putting netdevices

Using the tracking helpers, netdev_hold() and netdev_put(), makes it easier
to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER
is enabled. For example, the following traceback shows the callpath to the
point of an outstanding hold that was never put:

    unregister_netdevice: waiting for swp3 to become free. Usage count = 6
    ref_tracker: eth%d@ffff888123c9a580 has 1/5 users at
	mlxsw_sp_switchdev_event+0x6bd/0xcc0 [mlxsw_spectrum]
	notifier_call_chain+0xbf/0x3b0
	atomic_notifier_call_chain+0x78/0x200
	br_switchdev_fdb_notify+0x25f/0x2c0 [bridge]
	fdb_notify+0x16a/0x1a0 [bridge]
	[...]

In this patchset, get rid of all non-ref-tracking helpers in mlxsw.

- Patch #1 drops two functions that are not used anymore, but contain
  dev_hold() / dev_put() calls.

- Patch #2 avoids taking a reference in one function which is called
  under RTNL.

- The remaining patches convert individual hold/put sites one by one
  from trackerless to tracker-enabled.

Suggested-by: Paolo Abeni <[email protected]>
Link: https://lore.kernel.org/netdev/[email protected]/
====================

Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jakub Kicinski <[email protected]>
  • Loading branch information
kuba-moo committed Jul 28, 2023
2 parents 5027d54 + cb21162 commit 97d0dca
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 36 deletions.
17 changes: 0 additions & 17 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum.c
Original file line number Diff line number Diff line change
Expand Up @@ -4112,23 +4112,6 @@ struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find_rcu(struct net_device *dev)
return (struct mlxsw_sp_port *)priv.data;
}

struct mlxsw_sp_port *mlxsw_sp_port_lower_dev_hold(struct net_device *dev)
{
struct mlxsw_sp_port *mlxsw_sp_port;

rcu_read_lock();
mlxsw_sp_port = mlxsw_sp_port_dev_lower_find_rcu(dev);
if (mlxsw_sp_port)
dev_hold(mlxsw_sp_port->dev);
rcu_read_unlock();
return mlxsw_sp_port;
}

void mlxsw_sp_port_dev_put(struct mlxsw_sp_port *mlxsw_sp_port)
{
dev_put(mlxsw_sp_port->dev);
}

int mlxsw_sp_parsing_depth_inc(struct mlxsw_sp *mlxsw_sp)
{
char mprs_pl[MLXSW_REG_MPRS_LEN];
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum.h
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,6 @@ int mlxsw_sp_txhdr_ptp_data_construct(struct mlxsw_core *mlxsw_core,
bool mlxsw_sp_port_dev_check(const struct net_device *dev);
struct mlxsw_sp *mlxsw_sp_lower_get(struct net_device *dev);
struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find(struct net_device *dev);
struct mlxsw_sp_port *mlxsw_sp_port_lower_dev_hold(struct net_device *dev);
void mlxsw_sp_port_dev_put(struct mlxsw_sp_port *mlxsw_sp_port);
struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find_rcu(struct net_device *dev);
int mlxsw_sp_parsing_depth_inc(struct mlxsw_sp *mlxsw_sp);
void mlxsw_sp_parsing_depth_dec(struct mlxsw_sp *mlxsw_sp);
Expand Down
7 changes: 4 additions & 3 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,9 @@ void mlxsw_sp_nve_fid_disable(struct mlxsw_sp *mlxsw_sp,
int nve_ifindex;
__be32 vni;

/* Necessary for __dev_get_by_index() below. */
ASSERT_RTNL();

mlxsw_sp_nve_flood_ip_flush(mlxsw_sp, fid);
mlxsw_sp_nve_fdb_flush_by_fid(mlxsw_sp, fid_index);
mlxsw_sp_nve_ipv6_addr_flush_by_fid(mlxsw_sp, fid_index);
Expand All @@ -997,15 +1000,13 @@ void mlxsw_sp_nve_fid_disable(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_fid_vni(fid, &vni)))
goto out;

nve_dev = dev_get_by_index(mlxsw_sp_net(mlxsw_sp), nve_ifindex);
nve_dev = __dev_get_by_index(mlxsw_sp_net(mlxsw_sp), nve_ifindex);
if (!nve_dev)
goto out;

mlxsw_sp_nve_fdb_clear_offload(mlxsw_sp, fid, nve_dev, vni);
mlxsw_sp_fid_fdb_clear_offload(fid, nve_dev);

dev_put(nve_dev);

out:
mlxsw_sp_fid_vni_clear(fid);
mlxsw_sp_nve_tunnel_fini(mlxsw_sp);
Expand Down
25 changes: 15 additions & 10 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ static const struct rhashtable_params mlxsw_sp_crif_ht_params = {

struct mlxsw_sp_rif {
struct mlxsw_sp_crif *crif; /* NULL for underlay RIF */
netdevice_tracker dev_tracker;
struct list_head neigh_list;
struct mlxsw_sp_fid *fid;
unsigned char addr[ETH_ALEN];
Expand Down Expand Up @@ -7547,6 +7548,7 @@ struct mlxsw_sp_fib6_event_work {

struct mlxsw_sp_fib_event_work {
struct work_struct work;
netdevice_tracker dev_tracker;
union {
struct mlxsw_sp_fib6_event_work fib6_work;
struct fib_entry_notifier_info fen_info;
Expand Down Expand Up @@ -7720,12 +7722,12 @@ static void mlxsw_sp_router_fibmr_event_work(struct work_struct *work)
&fib_work->ven_info);
if (err)
dev_warn(mlxsw_sp->bus_info->dev, "MR VIF add failed.\n");
dev_put(fib_work->ven_info.dev);
netdev_put(fib_work->ven_info.dev, &fib_work->dev_tracker);
break;
case FIB_EVENT_VIF_DEL:
mlxsw_sp_router_fibmr_vif_del(mlxsw_sp,
&fib_work->ven_info);
dev_put(fib_work->ven_info.dev);
netdev_put(fib_work->ven_info.dev, &fib_work->dev_tracker);
break;
}
mutex_unlock(&mlxsw_sp->router->lock);
Expand Down Expand Up @@ -7796,7 +7798,8 @@ mlxsw_sp_router_fibmr_event(struct mlxsw_sp_fib_event_work *fib_work,
case FIB_EVENT_VIF_ADD:
case FIB_EVENT_VIF_DEL:
memcpy(&fib_work->ven_info, info, sizeof(fib_work->ven_info));
dev_hold(fib_work->ven_info.dev);
netdev_hold(fib_work->ven_info.dev, &fib_work->dev_tracker,
GFP_ATOMIC);
break;
}
}
Expand Down Expand Up @@ -8306,6 +8309,7 @@ mlxsw_sp_router_port_l3_stats_report_delta(struct mlxsw_sp_rif *rif,
struct mlxsw_sp_router_hwstats_notify_work {
struct work_struct work;
struct net_device *dev;
netdevice_tracker dev_tracker;
};

static void mlxsw_sp_router_hwstats_notify_work(struct work_struct *work)
Expand All @@ -8317,7 +8321,7 @@ static void mlxsw_sp_router_hwstats_notify_work(struct work_struct *work)
rtnl_lock();
rtnl_offload_xstats_notify(hws_work->dev);
rtnl_unlock();
dev_put(hws_work->dev);
netdev_put(hws_work->dev, &hws_work->dev_tracker);
kfree(hws_work);
}

Expand All @@ -8337,7 +8341,7 @@ mlxsw_sp_router_hwstats_notify_schedule(struct net_device *dev)
return;

INIT_WORK(&hws_work->work, mlxsw_sp_router_hwstats_notify_work);
dev_hold(dev);
netdev_hold(dev, &hws_work->dev_tracker, GFP_KERNEL);
hws_work->dev = dev;
mlxsw_core_schedule_work(&hws_work->work);
}
Expand Down Expand Up @@ -8409,7 +8413,7 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
err = -ENOMEM;
goto err_rif_alloc;
}
dev_hold(params->dev);
netdev_hold(params->dev, &rif->dev_tracker, GFP_KERNEL);
mlxsw_sp->router->rifs[rif_index] = rif;
rif->mlxsw_sp = mlxsw_sp;
rif->ops = ops;
Expand Down Expand Up @@ -8466,7 +8470,7 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_fid_put(fid);
err_fid_get:
mlxsw_sp->router->rifs[rif_index] = NULL;
dev_put(params->dev);
netdev_put(params->dev, &rif->dev_tracker);
mlxsw_sp_rif_free(rif);
err_rif_alloc:
err_crif_lookup:
Expand Down Expand Up @@ -8508,7 +8512,7 @@ static void mlxsw_sp_rif_destroy(struct mlxsw_sp_rif *rif)
/* Loopback RIFs are not associated with a FID. */
mlxsw_sp_fid_put(fid);
mlxsw_sp->router->rifs[rif->rif_index] = NULL;
dev_put(dev);
netdev_put(dev, &rif->dev_tracker);
mlxsw_sp_rif_free(rif);
mlxsw_sp_rif_index_free(mlxsw_sp, rif_index, rif_entries);
vr->rif_count--;
Expand Down Expand Up @@ -9329,6 +9333,7 @@ struct mlxsw_sp_inet6addr_event_work {
struct work_struct work;
struct mlxsw_sp *mlxsw_sp;
struct net_device *dev;
netdevice_tracker dev_tracker;
unsigned long event;
};

Expand All @@ -9352,7 +9357,7 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
out:
mutex_unlock(&mlxsw_sp->router->lock);
rtnl_unlock();
dev_put(dev);
netdev_put(dev, &inet6addr_work->dev_tracker);
kfree(inet6addr_work);
}

Expand All @@ -9378,7 +9383,7 @@ static int mlxsw_sp_inet6addr_event(struct notifier_block *nb,
inet6addr_work->mlxsw_sp = router->mlxsw_sp;
inet6addr_work->dev = dev;
inet6addr_work->event = event;
dev_hold(dev);
netdev_hold(dev, &inet6addr_work->dev_tracker, GFP_ATOMIC);
mlxsw_core_schedule_work(&inet6addr_work->work);

return NOTIFY_DONE;
Expand Down
9 changes: 5 additions & 4 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -3380,6 +3380,7 @@ static void mlxsw_sp_fdb_notify_work(struct work_struct *work)

struct mlxsw_sp_switchdev_event_work {
struct work_struct work;
netdevice_tracker dev_tracker;
union {
struct switchdev_notifier_fdb_info fdb_info;
struct switchdev_notifier_vxlan_fdb_info vxlan_fdb_info;
Expand Down Expand Up @@ -3536,8 +3537,8 @@ static void mlxsw_sp_switchdev_bridge_fdb_event_work(struct work_struct *work)
out:
rtnl_unlock();
kfree(switchdev_work->fdb_info.addr);
netdev_put(dev, &switchdev_work->dev_tracker);
kfree(switchdev_work);
dev_put(dev);
}

static void
Expand Down Expand Up @@ -3692,8 +3693,8 @@ static void mlxsw_sp_switchdev_vxlan_fdb_event_work(struct work_struct *work)

out:
rtnl_unlock();
netdev_put(dev, &switchdev_work->dev_tracker);
kfree(switchdev_work);
dev_put(dev);
}

static int
Expand Down Expand Up @@ -3793,7 +3794,7 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
* upper device containig mlxsw_sp_port or just a
* mlxsw_sp_port
*/
dev_hold(dev);
netdev_hold(dev, &switchdev_work->dev_tracker, GFP_ATOMIC);
break;
case SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE:
case SWITCHDEV_VXLAN_FDB_DEL_TO_DEVICE:
Expand All @@ -3803,7 +3804,7 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
info);
if (err)
goto err_vxlan_work_prepare;
dev_hold(dev);
netdev_hold(dev, &switchdev_work->dev_tracker, GFP_ATOMIC);
break;
default:
kfree(switchdev_work);
Expand Down

0 comments on commit 97d0dca

Please sign in to comment.