diff mbox

Soft lockup in tc_classify

Message ID 94c8dece-42b3-5e60-d73e-daabc4d4edc5@mellanox.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Shahar Klein Dec. 21, 2016, 11:25 a.m. UTC 
On 12/21/2016 9:03 AM, Cong Wang wrote:
> On Tue, Dec 20, 2016 at 10:44 PM, Shahar Klein <shahark@mellanox.com> wrote:
>>
>> Tried it with same results
>
> This piece is pretty interesting:
>
> [  408.554689] DEBUGG:SK thread-2853[cpu-1] setting tp_created to 1
> tp=ffff94b5b02805a0 back=ffff94b9ea932060
> [  408.574258] DEBUGG:SK thread-2853[cpu-1] add/change filter by:
> fl_get [cls_flower] tp=ffff94b5b02805a0 tp->next=ffff94b9ea932060
> [  408.587849] DEBUGG:SK destroy ffff94b5b0280780 tcf_destroy:1905
> [  408.595862] DEBUGG:SK thread-2845[cpu-1] add/change filter by:
> fl_get [cls_flower] tp=ffff94b5b02805a0 tp->next=ffff94b5b02805a0
>
> Looks like you added a debug printk inside tcf_destroy() too,
> which seems racy with filter creation, it should not happen since
> in both cases we take RTNL lock.
>
> Don't know if changing all RCU_INIT_POINTER in that file to
> rcu_assign_pointer could help anything or not. Mind to try?
>

Tried it with same results

>
> Thanks for debugging!
>
diff mbox

Patch

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 3fbba79..b8a66d8 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -305,7 +305,7 @@  static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 			kfree(tp);
 			goto errout;
 		}
-
+		printk(KERN_ERR "DEBUGG:SK thread-%d[cpu-%d] setting tp_created to 1 tp=%p back=%p\n", current->pid, current->on_cpu, tp, rtnl_dereference(*back));
 		tp_created = 1;
 
 	} else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind))
@@ -317,11 +317,13 @@  static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 		if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) {
 			struct tcf_proto *next = rtnl_dereference(tp->next);
 
-			RCU_INIT_POINTER(*back, next);
+			printk(KERN_ERR "DEBUGG:SK delete filter by: %pf\n", tp->ops->get);
+
+			rcu_assign_pointer(*back, next);
 
 			tfilter_notify(net, skb, n, tp, fh,
 				       RTM_DELTFILTER, false);
-			tcf_destroy(tp, true);
+			tcf_destroy(tp);
 			err = 0;
 			goto errout;
 		}
@@ -331,25 +333,30 @@  static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 		    !(n->nlmsg_flags & NLM_F_CREATE))
 			goto errout;
 	} else {
+		bool last;
+
 		switch (n->nlmsg_type) {
 		case RTM_NEWTFILTER:
 			err = -EEXIST;
 			if (n->nlmsg_flags & NLM_F_EXCL) {
 				if (tp_created)
-					tcf_destroy(tp, true);
+					tcf_destroy(tp);
 				goto errout;
 			}
 			break;
 		case RTM_DELTFILTER:
-			err = tp->ops->delete(tp, fh);
+			printk(KERN_ERR "DEBUGG:SK %s:%d\n", __func__, __LINE__);
+			err = tp->ops->delete(tp, fh, &last);
 			if (err == 0) {
-				struct tcf_proto *next = rtnl_dereference(tp->next);
-
 				tfilter_notify(net, skb, n, tp,
 					       t->tcm_handle,
 					       RTM_DELTFILTER, false);
-				if (tcf_destroy(tp, false))
-					RCU_INIT_POINTER(*back, next);
+				if (last) {
+					struct tcf_proto *next = rtnl_dereference(tp->next);
+
+					rcu_assign_pointer(*back, next);
+					tcf_destroy(tp);
+				}
 			}
 			goto errout;
 		case RTM_GETTFILTER:
@@ -366,13 +373,14 @@  static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 			      n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE);
 	if (err == 0) {
 		if (tp_created) {
-			RCU_INIT_POINTER(tp->next, rtnl_dereference(*back));
+			rcu_assign_pointer(tp->next, rtnl_dereference(*back));
 			rcu_assign_pointer(*back, tp);
+			printk(KERN_ERR "DEBUGG:SK thread-%d[cpu-%d] add/change filter by: %pf tp=%p tp->next=%p\n", current->pid, current->on_cpu, tp->ops->get, tp, tp->next);
 		}
 		tfilter_notify(net, skb, n, tp, fh, RTM_NEWTFILTER, false);
 	} else {
 		if (tp_created)
-			tcf_destroy(tp, true);
+			tcf_destroy(tp);
 	}
 
 errout: