Commit 67c9e627 authored by Ahmed S. Darwish's avatar Ahmed S. Darwish Committed by David S. Miller
Browse files

net: sched: Protect Qdisc::bstats with u64_stats 



The not-per-CPU variant of qdisc tc (traffic control) statistics,
Qdisc::gnet_stats_basic_packed bstats, is protected with Qdisc::running
sequence counter.

This sequence counter is used for reliably protecting bstats reads from
parallel writes. Meanwhile, the seqcount's write section covers a much
wider area than bstats update: qdisc_run_begin() => qdisc_run_end().

That read/write section asymmetry can lead to needless retries of the
read section. To prepare for removing the Qdisc::running sequence
counter altogether, introduce a u64_stats sync point inside bstats
instead.

Modify _bstats_update() to start/end the bstats u64_stats write
section.

For bisectability, and finer commits granularity, the bstats read
section is still protected with a Qdisc::running read/retry loop and
qdisc_run_begin/end() still starts/ends that seqcount write section.
Once all call sites are modified to use _bstats_update(), the
Qdisc::running seqcount will be removed and bstats read/retry loop will
be modified to utilize the internal u64_stats sync point.

Note, using u64_stats implies no sequence counter protection for 64-bit
architectures. This can lead to the statistics "packets" vs. "bytes"
values getting out of sync on rare occasions. The individual values will
still be valid.

[bigeasy: Minor commit message edits, init all gnet_stats_basic_packed.]

Signed-off-by: default avatarAhmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent f2efdb17

include/net/gen_stats.h

+2 −0
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ 
struct gnet_stats_basic_packed {

	__u64	bytes;

	__u64	packets;

	struct u64_stats_sync syncp;

};



struct gnet_stats_basic_cpu {
@@ -34,6 +35,7 @@ struct gnet_dump { 
	struct tc_stats   tc_stats;

};



void gnet_stats_basic_packed_init(struct gnet_stats_basic_packed *b);

int gnet_stats_start_copy(struct sk_buff *skb, int type, spinlock_t *lock,

			  struct gnet_dump *d, int padattr);

include/net/sch_generic.h

+2 −0
Original line number Diff line number Diff line
@@ -852,8 +852,10 @@ static inline int qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch, 
static inline void _bstats_update(struct gnet_stats_basic_packed *bstats,

				  __u64 bytes, __u32 packets)

{

	u64_stats_update_begin(&bstats->syncp);

	bstats->bytes += bytes;

	bstats->packets += packets;

	u64_stats_update_end(&bstats->syncp);

}



static inline void bstats_update(struct gnet_stats_basic_packed *bstats,

net/core/gen_estimator.c

+1 −1
Original line number Diff line number Diff line
@@ -62,7 +62,7 @@ struct net_rate_estimator { 
static void est_fetch_counters(struct net_rate_estimator *e,

			       struct gnet_stats_basic_packed *b)

{

	memset(b, 0, sizeof(*b));

	gnet_stats_basic_packed_init(b);

	if (e->stats_lock)

		spin_lock(e->stats_lock);

net/core/gen_stats.c

+12 −2
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ 
#include <linux/gen_stats.h>

#include <net/netlink.h>

#include <net/gen_stats.h>



#include <net/sch_generic.h>



static inline int

gnet_stats_copy(struct gnet_dump *d, int type, void *buf, int size, int padattr)
@@ -114,6 +114,15 @@ gnet_stats_start_copy(struct sk_buff *skb, int type, spinlock_t *lock, 
}

EXPORT_SYMBOL(gnet_stats_start_copy);



/* Must not be inlined, due to u64_stats seqcount_t lockdep key */

void gnet_stats_basic_packed_init(struct gnet_stats_basic_packed *b)

{

	b->bytes = 0;

	b->packets = 0;

	u64_stats_init(&b->syncp);

}

EXPORT_SYMBOL(gnet_stats_basic_packed_init);



static void gnet_stats_add_basic_cpu(struct gnet_stats_basic_packed *bstats,

				     struct gnet_stats_basic_cpu __percpu *cpu)

{
@@ -167,8 +176,9 @@ ___gnet_stats_copy_basic(const seqcount_t *running, 
			 struct gnet_stats_basic_packed *b,

			 int type)

{

	struct gnet_stats_basic_packed bstats = {0};

	struct gnet_stats_basic_packed bstats;



	gnet_stats_basic_packed_init(&bstats);

	gnet_stats_add_basic(running, &bstats, cpu, b);



	if (d->compat_tc_stats && type == TCA_STATS_BASIC) {

net/netfilter/xt_RATEEST.c

+1 −0
Original line number Diff line number Diff line
@@ -143,6 +143,7 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par) 
	if (!est)

		goto err1;



	gnet_stats_basic_packed_init(&est->bstats);

	strlcpy(est->name, info->name, sizeof(est->name));

	spin_lock_init(&est->lock);

	est->refcnt		= 1;