Skip to content

Commit

Permalink
sfc: avoid hang from nested use of the filter_sem
Browse files Browse the repository at this point in the history
In some situations we may end up calling down_read while already
holding the semaphore for write, thus hanging. This has been seen
when setting the MAC address for the interface. The hung task log
in this situation includes this stack:
  down_read
  efx_ef10_filter_insert
  efx_ef10_filter_insert_addr_list
  efx_ef10_filter_vlan_sync_rx_mode
  efx_ef10_filter_add_vlan
  efx_ef10_filter_table_probe
  efx_ef10_set_mac_address
  efx_set_mac_address
  dev_set_mac_address

In addition, lockdep rightly points out that nested calling of
down_read is incorrect.

Fixes: c2bebe3 ("sfc: give ef10 its own rwsem in the filter table instead of filter_lock")
Tested-by: Jarod Wilson <jarod@redhat.com>
Signed-off-by: Bert Kenward <bkenward@solarflare.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
sf-hgk authored and davem330 committed Jul 12, 2018
1 parent 9e3bff9 commit 1c56c09
Showing 1 changed file with 21 additions and 9 deletions.
30 changes: 21 additions & 9 deletions drivers/net/ethernet/sfc/ef10.c
Original file line number Diff line number Diff line change
Expand Up @@ -4288,9 +4288,9 @@ static int efx_ef10_filter_pri(struct efx_ef10_filter_table *table,
return -EPROTONOSUPPORT;
}

static s32 efx_ef10_filter_insert(struct efx_nic *efx,
struct efx_filter_spec *spec,
bool replace_equal)
static s32 efx_ef10_filter_insert_locked(struct efx_nic *efx,
struct efx_filter_spec *spec,
bool replace_equal)
{
DECLARE_BITMAP(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT);
struct efx_ef10_nic_data *nic_data = efx->nic_data;
Expand All @@ -4307,7 +4307,7 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
bool is_mc_recip;
s32 rc;

down_read(&efx->filter_sem);
WARN_ON(!rwsem_is_locked(&efx->filter_sem));
table = efx->filter_state;
down_write(&table->lock);

Expand Down Expand Up @@ -4498,10 +4498,22 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
if (rss_locked)
mutex_unlock(&efx->rss_lock);
up_write(&table->lock);
up_read(&efx->filter_sem);
return rc;
}

static s32 efx_ef10_filter_insert(struct efx_nic *efx,
struct efx_filter_spec *spec,
bool replace_equal)
{
s32 ret;

down_read(&efx->filter_sem);
ret = efx_ef10_filter_insert_locked(efx, spec, replace_equal);
up_read(&efx->filter_sem);

return ret;
}

static void efx_ef10_filter_update_rx_scatter(struct efx_nic *efx)
{
/* no need to do anything here on EF10 */
Expand Down Expand Up @@ -5285,7 +5297,7 @@ static int efx_ef10_filter_insert_addr_list(struct efx_nic *efx,
EFX_WARN_ON_PARANOID(ids[i] != EFX_EF10_FILTER_ID_INVALID);
efx_filter_init_rx(&spec, EFX_FILTER_PRI_AUTO, filter_flags, 0);
efx_filter_set_eth_local(&spec, vlan->vid, addr_list[i].addr);
rc = efx_ef10_filter_insert(efx, &spec, true);
rc = efx_ef10_filter_insert_locked(efx, &spec, true);
if (rc < 0) {
if (rollback) {
netif_info(efx, drv, efx->net_dev,
Expand Down Expand Up @@ -5314,7 +5326,7 @@ static int efx_ef10_filter_insert_addr_list(struct efx_nic *efx,
efx_filter_init_rx(&spec, EFX_FILTER_PRI_AUTO, filter_flags, 0);
eth_broadcast_addr(baddr);
efx_filter_set_eth_local(&spec, vlan->vid, baddr);
rc = efx_ef10_filter_insert(efx, &spec, true);
rc = efx_ef10_filter_insert_locked(efx, &spec, true);
if (rc < 0) {
netif_warn(efx, drv, efx->net_dev,
"Broadcast filter insert failed rc=%d\n", rc);
Expand Down Expand Up @@ -5370,7 +5382,7 @@ static int efx_ef10_filter_insert_def(struct efx_nic *efx,
if (vlan->vid != EFX_FILTER_VID_UNSPEC)
efx_filter_set_eth_local(&spec, vlan->vid, NULL);

rc = efx_ef10_filter_insert(efx, &spec, true);
rc = efx_ef10_filter_insert_locked(efx, &spec, true);
if (rc < 0) {
const char *um = multicast ? "Multicast" : "Unicast";
const char *encap_name = "";
Expand Down Expand Up @@ -5430,7 +5442,7 @@ static int efx_ef10_filter_insert_def(struct efx_nic *efx,
filter_flags, 0);
eth_broadcast_addr(baddr);
efx_filter_set_eth_local(&spec, vlan->vid, baddr);
rc = efx_ef10_filter_insert(efx, &spec, true);
rc = efx_ef10_filter_insert_locked(efx, &spec, true);
if (rc < 0) {
netif_warn(efx, drv, efx->net_dev,
"Broadcast filter insert failed rc=%d\n",
Expand Down

0 comments on commit 1c56c09

Please sign in to comment.