Commit d7fb60b9 authored by Cong Wang's avatar Cong Wang Committed by David S. Miller
Browse files

net_sched: get rid of tcfa_rcu 



gen estimator has been rewritten in commit 1c0d32fd
("net_sched: gen_estimator: complete rewrite of rate estimators"),
the caller is no longer needed to wait for a grace period.
So this patch gets rid of it.

This also completely closes a race condition between action free
path and filter chain add/remove path for the following patch.
Because otherwise the nested RCU callback can't be caught by
rcu_barrier().

Please see also the comments in code.

Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent da8ab578

include/net/act_api.h

+0 −2
Original line number Original line Diff line number Diff line
@@ -34,7 +34,6 @@ struct tc_action { 
	struct gnet_stats_queue		tcfa_qstats;

	struct gnet_stats_queue		tcfa_qstats;

	struct net_rate_estimator __rcu *tcfa_rate_est;

	struct net_rate_estimator __rcu *tcfa_rate_est;

	spinlock_t			tcfa_lock;

	spinlock_t			tcfa_lock;

	struct rcu_head			tcfa_rcu;

	struct gnet_stats_basic_cpu __percpu *cpu_bstats;

	struct gnet_stats_basic_cpu __percpu *cpu_bstats;

	struct gnet_stats_queue __percpu *cpu_qstats;

	struct gnet_stats_queue __percpu *cpu_qstats;

	struct tc_cookie	*act_cookie;

	struct tc_cookie	*act_cookie;
@@ -50,7 +49,6 @@ struct tc_action { 
#define tcf_qstats	common.tcfa_qstats

#define tcf_qstats	common.tcfa_qstats

#define tcf_rate_est	common.tcfa_rate_est

#define tcf_rate_est	common.tcfa_rate_est

#define tcf_lock	common.tcfa_lock

#define tcf_lock	common.tcfa_lock

#define tcf_rcu		common.tcfa_rcu





/* Update lastuse only if needed, to avoid dirtying a cache line.

/* Update lastuse only if needed, to avoid dirtying a cache line.

 * We use a temp variable to avoid fetching jiffies twice.

 * We use a temp variable to avoid fetching jiffies twice.

net/sched/act_api.c

+8 −9
Original line number Original line Diff line number Diff line
@@ -53,10 +53,13 @@ static void tcf_action_goto_chain_exec(const struct tc_action *a, 
	res->goto_tp = rcu_dereference_bh(chain->filter_chain);

	res->goto_tp = rcu_dereference_bh(chain->filter_chain);

}

}





static void free_tcf(struct rcu_head *head)

/* XXX: For standalone actions, we don't need a RCU grace period either, because

 * actions are always connected to filters and filters are already destroyed in

 * RCU callbacks, so after a RCU grace period actions are already disconnected

 * from filters. Readers later can not find us.

 */

static void free_tcf(struct tc_action *p)

{

{

	struct tc_action *p = container_of(head, struct tc_action, tcfa_rcu);



	free_percpu(p->cpu_bstats);

	free_percpu(p->cpu_bstats);

	free_percpu(p->cpu_qstats);

	free_percpu(p->cpu_qstats);
@@ -76,11 +79,7 @@ static void tcf_idr_remove(struct tcf_idrinfo *idrinfo, struct tc_action *p) 
	idr_remove_ext(&idrinfo->action_idr, p->tcfa_index);

	idr_remove_ext(&idrinfo->action_idr, p->tcfa_index);

	spin_unlock_bh(&idrinfo->lock);

	spin_unlock_bh(&idrinfo->lock);

	gen_kill_estimator(&p->tcfa_rate_est);

	gen_kill_estimator(&p->tcfa_rate_est);

	/*

	free_tcf(p);

	 * gen_estimator est_timer() might access p->tcfa_lock

	 * or bstats, wait a RCU grace period before freeing p

	 */

	call_rcu(&p->tcfa_rcu, free_tcf);

}

}





int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)

int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
@@ -259,7 +258,7 @@ void tcf_idr_cleanup(struct tc_action *a, struct nlattr *est) 
{

{

	if (est)

	if (est)

		gen_kill_estimator(&a->tcfa_rate_est);

		gen_kill_estimator(&a->tcfa_rate_est);

	call_rcu(&a->tcfa_rcu, free_tcf);

	free_tcf(a);

}

}

EXPORT_SYMBOL(tcf_idr_cleanup);

EXPORT_SYMBOL(tcf_idr_cleanup);

CRA Git | Maintained and supported by SUSTech CRA and CCSE