Commit 143976ce authored by WANG Cong's avatar WANG Cong Committed by David S. Miller
Browse files

net_sched: remove tc class reference counting 



For TC classes, their ->get() and ->put() are always paired, and the
reference counting is completely useless, because:

1) For class modification and dumping paths, we already hold RTNL lock,
   so all of these ->get(),->change(),->put() are atomic.

2) For filter bindiing/unbinding, we use other reference counter than
   this one, and they should have RTNL lock too.

3) For ->qlen_notify(), it is special because it is called on ->enqueue()
   path, but we already hold qdisc tree lock there, and we hold this
   tree lock when graft or delete the class too, so it should not be gone
   or changed until we release the tree lock.

Therefore, this patch removes ->get() and ->put(), but:

1) Adds a new ->find() to find the pointer to a class by classid, no
   refcnt.

2) Move the original class destroy upon the last refcnt into ->delete(),
   right after releasing tree lock. This is fine because the class is
   already removed from hash when holding the lock.

For those who also use ->put() as ->unbind(), just rename them to reflect
this change.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Acked-by: default avatarJiri Pirko <jiri@mellanox.com>
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 14546ba1

include/net/sch_generic.h

+1 −2
Original line number Diff line number Diff line
@@ -147,8 +147,7 @@ struct Qdisc_class_ops { 
	void			(*qlen_notify)(struct Qdisc *, unsigned long);



	/* Class manipulation routines */

	unsigned long		(*get)(struct Qdisc *, u32 classid);

	void			(*put)(struct Qdisc *, unsigned long);

	unsigned long		(*find)(struct Qdisc *, u32 classid);

	int			(*change)(struct Qdisc *, u32, u32,

					struct nlattr **, unsigned long *);

	int			(*delete)(struct Qdisc *, unsigned long);

net/sched/cls_api.c

+6 −11
Original line number Diff line number Diff line
@@ -586,7 +586,7 @@ replay: 


	/* Do we search for filter, attached to class? */

	if (TC_H_MIN(parent)) {

		cl = cops->get(q, parent);

		cl = cops->find(q, parent);

		if (cl == 0)

			return -ENOENT;

	}
@@ -716,8 +716,6 @@ replay: 
errout:

	if (chain)

		tcf_chain_put(chain);

	if (cl)

		cops->put(q, cl);

	if (err == -EAGAIN)

		/* Replay the request. */

		goto replay;
@@ -822,17 +820,17 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb) 
		goto out;

	cops = q->ops->cl_ops;

	if (!cops)

		goto errout;

		goto out;

	if (!cops->tcf_block)

		goto errout;

		goto out;

	if (TC_H_MIN(tcm->tcm_parent)) {

		cl = cops->get(q, tcm->tcm_parent);

		cl = cops->find(q, tcm->tcm_parent);

		if (cl == 0)

			goto errout;

			goto out;

	}

	block = cops->tcf_block(q, cl);

	if (!block)

		goto errout;

		goto out;



	index_start = cb->args[0];

	index = 0;
@@ -847,9 +845,6 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb) 


	cb->args[0] = index;



errout:

	if (cl)

		cops->put(q, cl);

out:

	return skb->len;

}

net/sched/sch_api.c

+8 −13
Original line number Diff line number Diff line
@@ -153,7 +153,7 @@ int register_qdisc(struct Qdisc_ops *qops) 
	if (qops->cl_ops) {

		const struct Qdisc_class_ops *cops = qops->cl_ops;



		if (!(cops->get && cops->put && cops->walk && cops->leaf))

		if (!(cops->find && cops->walk && cops->leaf))

			goto out_einval;



		if (cops->tcf_block && !(cops->bind_tcf && cops->unbind_tcf))
@@ -320,12 +320,11 @@ static struct Qdisc *qdisc_leaf(struct Qdisc *p, u32 classid) 


	if (cops == NULL)

		return NULL;

	cl = cops->get(p, classid);

	cl = cops->find(p, classid);



	if (cl == 0)

		return NULL;

	leaf = cops->leaf(p, cl);

	cops->put(p, cl);

	return leaf;

}
@@ -756,9 +755,8 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, unsigned int n, 
		}

		cops = sch->ops->cl_ops;

		if (notify && cops->qlen_notify) {

			cl = cops->get(sch, parentid);

			cl = cops->find(sch, parentid);

			cops->qlen_notify(sch, cl);

			cops->put(sch, cl);

		}

		sch->q.qlen -= n;

		sch->qstats.backlog -= len;
@@ -955,11 +953,11 @@ skip: 


		err = -EOPNOTSUPP;

		if (cops && cops->graft) {

			unsigned long cl = cops->get(parent, classid);

			if (cl) {

			unsigned long cl = cops->find(parent, classid);



			if (cl)

				err = cops->graft(parent, cl, new, &old);

				cops->put(parent, cl);

			} else

			else

				err = -ENOENT;

		}

		if (!err)
@@ -1739,7 +1737,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, 
		clid = TC_H_MAKE(qid, clid);



	if (clid)

		cl = cops->get(q, clid);

		cl = cops->find(q, clid);



	if (cl == 0) {

		err = -ENOENT;
@@ -1773,9 +1771,6 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, 
		tclass_notify(net, skb, n, q, new_cl, RTM_NEWTCLASS);



out:

	if (cl)

		cops->put(q, cl);



	return err;

}

net/sched/sch_atm.c

+16 −14
Original line number Diff line number Diff line
@@ -108,23 +108,29 @@ static struct Qdisc *atm_tc_leaf(struct Qdisc *sch, unsigned long cl) 
	return flow ? flow->q : NULL;

}



static unsigned long atm_tc_get(struct Qdisc *sch, u32 classid)

static unsigned long atm_tc_find(struct Qdisc *sch, u32 classid)

{

	struct atm_qdisc_data *p __maybe_unused = qdisc_priv(sch);

	struct atm_flow_data *flow;



	pr_debug("atm_tc_get(sch %p,[qdisc %p],classid %x)\n", sch, p, classid);

	pr_debug("%s(sch %p,[qdisc %p],classid %x)\n", __func__, sch, p, classid);

	flow = lookup_flow(sch, classid);

	if (flow)

		flow->ref++;

	pr_debug("atm_tc_get: flow %p\n", flow);

	pr_debug("%s: flow %p\n", __func__, flow);

	return (unsigned long)flow;

}



static unsigned long atm_tc_bind_filter(struct Qdisc *sch,

					unsigned long parent, u32 classid)

{

	return atm_tc_get(sch, classid);

	struct atm_qdisc_data *p __maybe_unused = qdisc_priv(sch);

	struct atm_flow_data *flow;



	pr_debug("%s(sch %p,[qdisc %p],classid %x)\n", __func__, sch, p, classid);

	flow = lookup_flow(sch, classid);

	if (flow)

		flow->ref++;

	pr_debug("%s: flow %p\n", __func__, flow);

	return (unsigned long)flow;

}



/*
@@ -234,7 +240,7 @@ static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent, 
		excess = NULL;

	else {

		excess = (struct atm_flow_data *)

			atm_tc_get(sch, nla_get_u32(tb[TCA_ATM_EXCESS]));

			atm_tc_find(sch, nla_get_u32(tb[TCA_ATM_EXCESS]));

		if (!excess)

			return -ENOENT;

	}
@@ -262,10 +268,9 @@ static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent, 


		for (i = 1; i < 0x8000; i++) {

			classid = TC_H_MAKE(sch->handle, 0x8000 | i);

			cl = atm_tc_get(sch, classid);

			cl = atm_tc_find(sch, classid);

			if (!cl)

				break;

			atm_tc_put(sch, cl);

		}

	}

	pr_debug("atm_tc_change: new id %x\n", classid);
@@ -305,8 +310,6 @@ static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent, 
	*arg = (unsigned long)flow;

	return 0;

err_out:

	if (excess)

		atm_tc_put(sch, (unsigned long)excess);

	sockfd_put(sock);

	return error;

}
@@ -377,7 +380,7 @@ static int atm_tc_enqueue(struct sk_buff *skb, struct Qdisc *sch, 
	result = TC_ACT_OK;	/* be nice to gcc */

	flow = NULL;

	if (TC_H_MAJ(skb->priority) != sch->handle ||

	    !(flow = (struct atm_flow_data *)atm_tc_get(sch, skb->priority))) {

	    !(flow = (struct atm_flow_data *)atm_tc_find(sch, skb->priority))) {

		struct tcf_proto *fl;



		list_for_each_entry(flow, &p->flows, list) {
@@ -655,8 +658,7 @@ static int atm_tc_dump(struct Qdisc *sch, struct sk_buff *skb) 
static const struct Qdisc_class_ops atm_class_ops = {

	.graft		= atm_tc_graft,

	.leaf		= atm_tc_leaf,

	.get		= atm_tc_get,

	.put		= atm_tc_put,

	.find		= atm_tc_find,

	.change		= atm_tc_change,

	.delete		= atm_tc_delete,

	.walk		= atm_tc_walk,

net/sched/sch_cbq.c

+4 −37
Original line number Diff line number Diff line
@@ -129,7 +129,6 @@ struct cbq_class { 
	struct tcf_proto __rcu	*filter_list;

	struct tcf_block	*block;



	int			refcnt;

	int			filters;



	struct cbq_class	*defaults[TC_PRIO_MAX + 1];
@@ -1155,7 +1154,6 @@ static int cbq_init(struct Qdisc *sch, struct nlattr *opt) 
	if (err < 0)

		goto put_rtab;



	q->link.refcnt = 1;

	q->link.sibling = &q->link;

	q->link.common.classid = sch->handle;

	q->link.qdisc = sch;
@@ -1388,16 +1386,11 @@ static void cbq_qlen_notify(struct Qdisc *sch, unsigned long arg) 
	cbq_deactivate_class(cl);

}



static unsigned long cbq_get(struct Qdisc *sch, u32 classid)

static unsigned long cbq_find(struct Qdisc *sch, u32 classid)

{

	struct cbq_sched_data *q = qdisc_priv(sch);

	struct cbq_class *cl = cbq_class_lookup(q, classid);



	if (cl) {

		cl->refcnt++;

		return (unsigned long)cl;

	}

	return 0;

	return (unsigned long)cbq_class_lookup(q, classid);

}



static void cbq_destroy_class(struct Qdisc *sch, struct cbq_class *cl)
@@ -1443,25 +1436,6 @@ static void cbq_destroy(struct Qdisc *sch) 
	qdisc_class_hash_destroy(&q->clhash);

}



static void cbq_put(struct Qdisc *sch, unsigned long arg)

{

	struct cbq_class *cl = (struct cbq_class *)arg;



	if (--cl->refcnt == 0) {

#ifdef CONFIG_NET_CLS_ACT

		spinlock_t *root_lock = qdisc_root_sleeping_lock(sch);

		struct cbq_sched_data *q = qdisc_priv(sch);



		spin_lock_bh(root_lock);

		if (q->rx_class == cl)

			q->rx_class = NULL;

		spin_unlock_bh(root_lock);

#endif



		cbq_destroy_class(sch, cl);

	}

}



static int

cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **tca,

		 unsigned long *arg)
@@ -1608,7 +1582,6 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t 


	cl->R_tab = rtab;

	rtab = NULL;

	cl->refcnt = 1;

	cl->q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, classid);

	if (!cl->q)

		cl->q = &noop_qdisc;
@@ -1689,12 +1662,7 @@ static int cbq_delete(struct Qdisc *sch, unsigned long arg) 
	cbq_rmprio(q, cl);

	sch_tree_unlock(sch);



	BUG_ON(--cl->refcnt == 0);

	/*

	 * This shouldn't happen: we "hold" one cops->get() when called

	 * from tc_ctl_tclass; the destroy method is done from cops->put().

	 */



	cbq_destroy_class(sch, cl);

	return 0;

}
@@ -1760,8 +1728,7 @@ static const struct Qdisc_class_ops cbq_class_ops = { 
	.graft		=	cbq_graft,

	.leaf		=	cbq_leaf,

	.qlen_notify	=	cbq_qlen_notify,

	.get		=	cbq_get,

	.put		=	cbq_put,

	.find		=	cbq_find,

	.change		=	cbq_change_class,

	.delete		=	cbq_delete,

	.walk		=	cbq_walk,

CRA Git | Maintained and supported by SUSTech CRA and CCSE