Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net/sched: cls_u32: replace int refcounts with proper refcounts

Proper refcounts will always warn splat when something goes wrong,
be it underflow, saturation or object resurrection. As these are always
a source of bugs, use it in cls_u32 as a safeguard to prevent/catch issues.
Another benefit is that the refcount API self documents the code, making
clear when transitions to dead are expected.

For such an update we had to make minor adaptations on u32 to fit the refcount
API. First we set explicitly to '1' when objects are created, then the
objects are alive until a 1 -> 0 happens, which is then released appropriately.

The above made clear some redundant operations in the u32 code
around the root_ht handling that were removed. The root_ht is created
with a refcnt set to 1. Then when it's associated with tcf_proto it increments the refcnt to 2.
Throughout the entire code the root_ht is an exceptional case and can never be referenced,
therefore the refcnt never incremented/decremented.
Its lifetime is always bound to tcf_proto, meaning if you delete tcf_proto
the root_ht is deleted as well. The code made up for the fact that root_ht refcnt is 2 and did
a double decrement to free it, which is not a fit for the refcount API.

Even though refcount_t is implemented using atomics, we should observe
a negligible control plane impact.

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Link: https://lore.kernel.org/r/20231114141856.974326-2-pctammela@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Pedro Tammela and committed by
Jakub Kicinski
6b78debe 289354f2

+18 -18
+18 -18
net/sched/cls_u32.c
··· 71 71 struct tc_u_hnode __rcu *next; 72 72 u32 handle; 73 73 u32 prio; 74 - int refcnt; 74 + refcount_t refcnt; 75 75 unsigned int divisor; 76 76 struct idr handle_idr; 77 77 bool is_root; ··· 86 86 struct tc_u_common { 87 87 struct tc_u_hnode __rcu *hlist; 88 88 void *ptr; 89 - int refcnt; 89 + refcount_t refcnt; 90 90 struct idr handle_idr; 91 91 struct hlist_node hnode; 92 92 long knodes; ··· 359 359 if (root_ht == NULL) 360 360 return -ENOBUFS; 361 361 362 - root_ht->refcnt++; 362 + refcount_set(&root_ht->refcnt, 1); 363 363 root_ht->handle = tp_c ? gen_new_htid(tp_c, root_ht) : 0x80000000; 364 364 root_ht->prio = tp->prio; 365 365 root_ht->is_root = true; ··· 371 371 kfree(root_ht); 372 372 return -ENOBUFS; 373 373 } 374 + refcount_set(&tp_c->refcnt, 1); 374 375 tp_c->ptr = key; 375 376 INIT_HLIST_NODE(&tp_c->hnode); 376 377 idr_init(&tp_c->handle_idr); 377 378 378 379 hlist_add_head(&tp_c->hnode, tc_u_hash(key)); 380 + } else { 381 + refcount_inc(&tp_c->refcnt); 379 382 } 380 383 381 - tp_c->refcnt++; 382 384 RCU_INIT_POINTER(root_ht->next, tp_c->hlist); 383 385 rcu_assign_pointer(tp_c->hlist, root_ht); 384 386 385 - root_ht->refcnt++; 387 + /* root_ht must be destroyed when tcf_proto is destroyed */ 386 388 rcu_assign_pointer(tp->root, root_ht); 387 389 tp->data = tp_c; 388 390 return 0; ··· 395 393 struct tc_u_hnode *ht = rtnl_dereference(n->ht_down); 396 394 397 395 tcf_exts_destroy(&n->exts); 398 - if (ht && --ht->refcnt == 0) 396 + if (ht && refcount_dec_and_test(&ht->refcnt)) 399 397 kfree(ht); 400 398 kfree(n); 401 399 } ··· 603 601 struct tc_u_hnode __rcu **hn; 604 602 struct tc_u_hnode *phn; 605 603 606 - WARN_ON(--ht->refcnt); 607 - 608 604 u32_clear_hnode(tp, ht, extack); 609 605 610 606 hn = &tp_c->hlist; ··· 630 630 631 631 WARN_ON(root_ht == NULL); 632 632 633 - if (root_ht && --root_ht->refcnt == 1) 633 + if (root_ht && refcount_dec_and_test(&root_ht->refcnt)) 634 634 u32_destroy_hnode(tp, root_ht, extack); 635 635 636 - if (--tp_c->refcnt == 0) { 636 + if (refcount_dec_and_test(&tp_c->refcnt)) { 637 637 struct tc_u_hnode *ht; 638 638 639 639 hlist_del(&tp_c->hnode); ··· 645 645 /* u32_destroy_key() will later free ht for us, if it's 646 646 * still referenced by some knode 647 647 */ 648 - if (--ht->refcnt == 0) 648 + if (refcount_dec_and_test(&ht->refcnt)) 649 649 kfree_rcu(ht, rcu); 650 650 } 651 651 ··· 674 674 return -EINVAL; 675 675 } 676 676 677 - if (ht->refcnt == 1) { 677 + if (refcount_dec_if_one(&ht->refcnt)) { 678 678 u32_destroy_hnode(tp, ht, extack); 679 679 } else { 680 680 NL_SET_ERR_MSG_MOD(extack, "Can not delete in-use filter"); ··· 682 682 } 683 683 684 684 out: 685 - *last = tp_c->refcnt == 1 && tp_c->knodes == 0; 685 + *last = refcount_read(&tp_c->refcnt) == 1 && tp_c->knodes == 0; 686 686 return ret; 687 687 } 688 688 ··· 766 766 NL_SET_ERR_MSG_MOD(extack, "Not linking to root node"); 767 767 return -EINVAL; 768 768 } 769 - ht_down->refcnt++; 769 + refcount_inc(&ht_down->refcnt); 770 770 } 771 771 772 772 ht_old = rtnl_dereference(n->ht_down); 773 773 rcu_assign_pointer(n->ht_down, ht_down); 774 774 775 775 if (ht_old) 776 - ht_old->refcnt--; 776 + refcount_dec(&ht_old->refcnt); 777 777 } 778 778 779 779 if (ifindex >= 0) ··· 852 852 853 853 /* bump reference count as long as we hold pointer to structure */ 854 854 if (ht) 855 - ht->refcnt++; 855 + refcount_inc(&ht->refcnt); 856 856 857 857 return new; 858 858 } ··· 932 932 933 933 ht_old = rtnl_dereference(n->ht_down); 934 934 if (ht_old) 935 - ht_old->refcnt++; 935 + refcount_inc(&ht_old->refcnt); 936 936 } 937 937 __u32_destroy_key(new); 938 938 return err; ··· 980 980 return err; 981 981 } 982 982 } 983 - ht->refcnt = 1; 983 + refcount_set(&ht->refcnt, 1); 984 984 ht->divisor = divisor; 985 985 ht->handle = handle; 986 986 ht->prio = tp->prio;