Skip to content

Commit ad9421e

Browse files
roidayanSaeed Mahameed
authored andcommitted
net/mlx5: Fix possible deadlock from lockdep when adding fte to fg
This is a false positive report due to incorrect nested lock annotations as we lock multiple fgs with the same subclass. Instead of locking all fgs only lock the one being used as was done before. Fixes: bd71b08 ("net/mlx5: Support multiple updates of steering rules in parallel") Signed-off-by: Roi Dayan <[email protected]> Signed-off-by: Saeed Mahameed <[email protected]>
1 parent fc43382 commit ad9421e

File tree

1 file changed

+37
-37
lines changed
  • drivers/net/ethernet/mellanox/mlx5/core

1 file changed

+37
-37
lines changed

drivers/net/ethernet/mellanox/mlx5/core/fs_core.c

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,6 +1578,33 @@ static u64 matched_fgs_get_version(struct list_head *match_head)
15781578
return version;
15791579
}
15801580

1581+
static struct fs_fte *
1582+
lookup_fte_locked(struct mlx5_flow_group *g,
1583+
u32 *match_value,
1584+
bool take_write)
1585+
{
1586+
struct fs_fte *fte_tmp;
1587+
1588+
if (take_write)
1589+
nested_down_write_ref_node(&g->node, FS_LOCK_PARENT);
1590+
else
1591+
nested_down_read_ref_node(&g->node, FS_LOCK_PARENT);
1592+
fte_tmp = rhashtable_lookup_fast(&g->ftes_hash, match_value,
1593+
rhash_fte);
1594+
if (!fte_tmp || !tree_get_node(&fte_tmp->node)) {
1595+
fte_tmp = NULL;
1596+
goto out;
1597+
}
1598+
1599+
nested_down_write_ref_node(&fte_tmp->node, FS_LOCK_CHILD);
1600+
out:
1601+
if (take_write)
1602+
up_write_ref_node(&g->node);
1603+
else
1604+
up_read_ref_node(&g->node);
1605+
return fte_tmp;
1606+
}
1607+
15811608
static struct mlx5_flow_handle *
15821609
try_add_to_existing_fg(struct mlx5_flow_table *ft,
15831610
struct list_head *match_head,
@@ -1600,31 +1627,16 @@ try_add_to_existing_fg(struct mlx5_flow_table *ft,
16001627
if (IS_ERR(fte))
16011628
return ERR_PTR(-ENOMEM);
16021629

1603-
list_for_each_entry(iter, match_head, list) {
1604-
nested_down_read_ref_node(&iter->g->node, FS_LOCK_PARENT);
1605-
}
1606-
16071630
search_again_locked:
16081631
version = matched_fgs_get_version(match_head);
16091632
/* Try to find a fg that already contains a matching fte */
16101633
list_for_each_entry(iter, match_head, list) {
16111634
struct fs_fte *fte_tmp;
16121635

16131636
g = iter->g;
1614-
fte_tmp = rhashtable_lookup_fast(&g->ftes_hash, spec->match_value,
1615-
rhash_fte);
1616-
if (!fte_tmp || !tree_get_node(&fte_tmp->node))
1637+
fte_tmp = lookup_fte_locked(g, spec->match_value, take_write);
1638+
if (!fte_tmp)
16171639
continue;
1618-
1619-
nested_down_write_ref_node(&fte_tmp->node, FS_LOCK_CHILD);
1620-
if (!take_write) {
1621-
list_for_each_entry(iter, match_head, list)
1622-
up_read_ref_node(&iter->g->node);
1623-
} else {
1624-
list_for_each_entry(iter, match_head, list)
1625-
up_write_ref_node(&iter->g->node);
1626-
}
1627-
16281640
rule = add_rule_fg(g, spec->match_value,
16291641
flow_act, dest, dest_num, fte_tmp);
16301642
up_write_ref_node(&fte_tmp->node);
@@ -1633,19 +1645,6 @@ try_add_to_existing_fg(struct mlx5_flow_table *ft,
16331645
return rule;
16341646
}
16351647

1636-
/* No group with matching fte found. Try to add a new fte to any
1637-
* matching fg.
1638-
*/
1639-
1640-
if (!take_write) {
1641-
list_for_each_entry(iter, match_head, list)
1642-
up_read_ref_node(&iter->g->node);
1643-
list_for_each_entry(iter, match_head, list)
1644-
nested_down_write_ref_node(&iter->g->node,
1645-
FS_LOCK_PARENT);
1646-
take_write = true;
1647-
}
1648-
16491648
/* Check the ft version, for case that new flow group
16501649
* was added while the fgs weren't locked
16511650
*/
@@ -1657,27 +1656,30 @@ try_add_to_existing_fg(struct mlx5_flow_table *ft,
16571656
/* Check the fgs version, for case the new FTE with the
16581657
* same values was added while the fgs weren't locked
16591658
*/
1660-
if (version != matched_fgs_get_version(match_head))
1659+
if (version != matched_fgs_get_version(match_head)) {
1660+
take_write = true;
16611661
goto search_again_locked;
1662+
}
16621663

16631664
list_for_each_entry(iter, match_head, list) {
16641665
g = iter->g;
16651666

16661667
if (!g->node.active)
16671668
continue;
1669+
1670+
nested_down_write_ref_node(&g->node, FS_LOCK_PARENT);
1671+
16681672
err = insert_fte(g, fte);
16691673
if (err) {
1674+
up_write_ref_node(&g->node);
16701675
if (err == -ENOSPC)
16711676
continue;
1672-
list_for_each_entry(iter, match_head, list)
1673-
up_write_ref_node(&iter->g->node);
16741677
kmem_cache_free(steering->ftes_cache, fte);
16751678
return ERR_PTR(err);
16761679
}
16771680

16781681
nested_down_write_ref_node(&fte->node, FS_LOCK_CHILD);
1679-
list_for_each_entry(iter, match_head, list)
1680-
up_write_ref_node(&iter->g->node);
1682+
up_write_ref_node(&g->node);
16811683
rule = add_rule_fg(g, spec->match_value,
16821684
flow_act, dest, dest_num, fte);
16831685
up_write_ref_node(&fte->node);
@@ -1686,8 +1688,6 @@ try_add_to_existing_fg(struct mlx5_flow_table *ft,
16861688
}
16871689
rule = ERR_PTR(-ENOENT);
16881690
out:
1689-
list_for_each_entry(iter, match_head, list)
1690-
up_write_ref_node(&iter->g->node);
16911691
kmem_cache_free(steering->ftes_cache, fte);
16921692
return rule;
16931693
}

0 commit comments

Comments
 (0)