Patchwork [02/18] net: use wrapper functions of net_ratelimit() to simplify code

login
register
mail settings
Submitter Kefeng Wang
Date Oct. 15, 2013, 11:44 a.m.
Message ID <1381837514-50660-3-git-send-email-wangkefeng.wang@huawei.com>
Download mbox | patch
Permalink /patch/283611/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Kefeng Wang - Oct. 15, 2013, 11:44 a.m.
Wrapper functions net_ratelimited_function() and net_XXX_ratelimited()
are called to simplify code.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 net/bridge/br_fdb.c                   |  6 ++----
 net/bridge/br_multicast.c             | 13 ++++---------
 net/bridge/br_stp_bpdu.c              | 11 ++++-------
 net/mac80211/rx.c                     |  6 ++----
 net/netfilter/ipset/ip_set_hash_gen.h |  3 +--
 5 files changed, 13 insertions(+), 26 deletions(-)
Joe Perches - Oct. 15, 2013, 4:24 p.m.
On Tue, 2013-10-15 at 19:44 +0800, Kefeng Wang wrote:
> Wrapper functions net_ratelimited_function() and net_XXX_ratelimited()
> are called to simplify code.
[]
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
[]
> @@ -465,10 +465,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  	if (likely(fdb)) {
>  		/* attempt to update an entry for a local interface */
>  		if (unlikely(fdb->is_local)) {
> -			if (net_ratelimit())
> -				br_warn(br, "received packet on %s with "
> -					"own address as source address\n",
> -					source->dev->name);
> +			net_ratelimited_function(br_warn, br, "received packet on %s "
> +				"with own address as source address\n", source->dev->name);

Hello Kefeng.

When these types of lines are changed, please coalesce the
fragmented format pieces into a single string.

It makes grep a bit easier and 80 columns limits don't
apply to formats.

I think using net_ratelimited_function is not particularly
clarifying here.

Maybe net_ratelimited_function should be removed instead
of its use sites expanded.

Perhaps adding macros like #define br_warn_ratelimited()
would be better.

This comment applies to the whole series.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kefeng Wang - Oct. 16, 2013, 3:24 a.m.
Thanks for your reply.

On 10/16 0:24, Joe Perches wrote:
> On Tue, 2013-10-15 at 19:44 +0800, Kefeng Wang wrote:
>> Wrapper functions net_ratelimited_function() and net_XXX_ratelimited()
>> are called to simplify code.
> []
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> []
>> @@ -465,10 +465,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>  	if (likely(fdb)) {
>>  		/* attempt to update an entry for a local interface */
>>  		if (unlikely(fdb->is_local)) {
>> -			if (net_ratelimit())
>> -				br_warn(br, "received packet on %s with "
>> -					"own address as source address\n",
>> -					source->dev->name);
>> +			net_ratelimited_function(br_warn, br, "received packet on %s "
>> +				"with own address as source address\n", source->dev->name);
> 
> Hello Kefeng.
> 
> When these types of lines are changed, please coalesce the
> fragmented format pieces into a single string.
> 
> It makes grep a bit easier and 80 columns limits don't
> apply to formats.

Got it, I will coalesce them, but 80 columns limits will be
broken.

> I think using net_ratelimited_function is not particularly
> clarifying here.
> 
> Maybe net_ratelimited_function should be removed instead
> of its use sites expanded.
> 
> Perhaps adding macros like #define br_warn_ratelimited()
> would be better.

yes, I found dev_emerg_ratelimited already exists. I should
use them and will add some similar mcaros.

> This comment applies to the whole series.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index ffd5874..e47dfe5 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -465,10 +465,8 @@  void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 	if (likely(fdb)) {
 		/* attempt to update an entry for a local interface */
 		if (unlikely(fdb->is_local)) {
-			if (net_ratelimit())
-				br_warn(br, "received packet on %s with "
-					"own address as source address\n",
-					source->dev->name);
+			net_ratelimited_function(br_warn, br, "received packet on %s "
+				"with own address as source address\n", source->dev->name);
 		} else {
 			/* fastpath: update of existing entry */
 			fdb->dst = source;
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index d1c5786..c00d8a2 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -530,11 +530,8 @@  static struct net_bridge_mdb_entry *br_multicast_get_group(
 	max = mdb->max;
 
 	if (unlikely(count > br->hash_elasticity && count)) {
-		if (net_ratelimit())
-			br_info(br, "Multicast hash table "
-				"chain limit reached: %s\n",
-				port ? port->dev->name : br->dev->name);
-
+		net_ratelimited_function(br_info, br, "Multicast hash table "
+			"chain limit reached: %s\n", port ? port->dev->name : br->dev->name);
 		elasticity = br->hash_elasticity;
 	}
 
@@ -554,10 +551,8 @@  disable:
 
 	if (max > mdb->max || elasticity) {
 		if (mdb->old) {
-			if (net_ratelimit())
-				br_info(br, "Multicast hash table "
-					"on fire: %s\n",
-					port ? port->dev->name : br->dev->name);
+			net_ratelimited_function(br_info, br, "Multicast hash table "
+				"on fire: %s\n", port ? port->dev->name : br->dev->name);
 			err = -EEXIST;
 			goto err;
 		}
diff --git a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
index 8660ea3..482ef11 100644
--- a/net/bridge/br_stp_bpdu.c
+++ b/net/bridge/br_stp_bpdu.c
@@ -220,13 +220,10 @@  void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
 		bpdu.forward_delay = br_get_ticks(buf+30);
 
 		if (bpdu.message_age > bpdu.max_age) {
-			if (net_ratelimit())
-				br_notice(p->br,
-					  "port %u config from %pM"
-					  " (message_age %ul > max_age %ul)\n",
-					  p->port_no,
-					  eth_hdr(skb)->h_source,
-					  bpdu.message_age, bpdu.max_age);
+			net_ratelimited_function(br_notice, p->br,
+				"port %u config from %pM (message_age %ul > max_age %ul)\n",
+				p->port_no, eth_hdr(skb)->h_source,
+				bpdu.message_age, bpdu.max_age);
 			goto out;
 		}
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 54395d7..35b8b1d 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3156,10 +3156,8 @@  static bool ieee80211_prepare_and_rx_handle(struct ieee80211_rx_data *rx,
 	if (!consume) {
 		skb = skb_copy(skb, GFP_ATOMIC);
 		if (!skb) {
-			if (net_ratelimit())
-				wiphy_debug(local->hw.wiphy,
-					"failed to copy skb for %s\n",
-					sdata->name);
+			net_ratelimited_function(wiphy_debug, local->hw.wiphy,
+				"failed to copy skb for %s\n", sdata->name);
 			return true;
 		}
 
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 707bc52..6179436 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -607,8 +607,7 @@  mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 		mtype_expire(h, NETS_LENGTH(set->family), h->dsize);
 
 	if (h->elements >= h->maxelem) {
-		if (net_ratelimit())
-			pr_warning("Set %s is full, maxelem %u reached\n",
+		net_warn_ratelimited("Set %s is full, maxelem %u reached\n",
 				   set->name, h->maxelem);
 		return -IPSET_ERR_HASH_FULL;
 	}