Skip to contentSkip to conversation

flowcache: make flow_key_size() return "unsigned int"

PATCH Applied Alexey Dobriyan 2 participants 6 messages Apr 2, 2017 – Apr 4, 2017

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Alexey Dobriyan

author
#0 lore raw

PATCH v2 1/3 flowcache: make flow_key_size() return "unsigned int"

Flow keys aren't 4GB+ numbers so 64-bit arithmetic is excessive.

Space savings (I'm not sure what CSWTCH is):

add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-48 (-48)
function old new delta
flow_cache_lookup 1163 1159 -4
CSWTCH 75997 75953 -44

Signed-off-by Alexey Dobriyan <[email protected]>

---

 include/net/flow.h |    2 +-
 net/core/flow.c    |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
include/net/flow.h +1 -1
@@ -202,7 +202,7 @@ static inline struct flowi *flowidn_to_flowi(struct flowidn *fldn)
 
 typedef unsigned long flow_compare_t;
 
-static inline size_t flow_key_size(u16 family)
+static inline unsigned int flow_key_size(u16 family)
 {
 	switch (family) {
 	case AF_INET:
net/core/flow.c +3 -3
@@ -161,7 +161,7 @@ static void flow_new_hash_rnd(struct flow_cache *fc,
 static u32 flow_hash_code(struct flow_cache *fc,
 			  struct flow_cache_percpu *fcp,
 			  const struct flowi *key,
-			  size_t keysize)
+			  unsigned int keysize)
 {
 	const u32 *k = (const u32 *) key;
 	const u32 length = keysize * sizeof(flow_compare_t) / sizeof(u32);

@@ -174,7 +174,7 @@ static u32 flow_hash_code(struct flow_cache *fc,
  * important assumptions that we can here, such as alignment.
  */
 static int flow_key_compare(const struct flowi *key1, const struct flowi *key2,
-			    size_t keysize)
+			    unsigned int keysize)
 {
 	const flow_compare_t *k1, *k1_lim, *k2;
 

@@ -199,7 +199,7 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
 	struct flow_cache_percpu *fcp;
 	struct flow_cache_entry *fle, *tfle;
 	struct flow_cache_object *flo;
-	size_t keysize;
+	unsigned int keysize;
 	unsigned int hash;
 
 	local_bh_disable();
author
#1 lore raw

PATCH v2 2/3 flowcache: make flow_cache_hash_size() return "unsigned int"

Hash size can't negative so "unsigned int" is logically correct.

Propagate "unsigned int" to loop counters.

Space savings:

add/remove: 0/0 grow/shrink: 2/2 up/down: 6/-18 (-12)
function old new delta
flow_cache_flush_tasklet 362 365 +3
__flow_cache_shrink 333 336 +3
flow_cache_cpu_up_prep 178 171 -7
flow_cache_lookup 1159 1148 -11

Signed-off-by Alexey Dobriyan <[email protected]>

---

 net/core/flow.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
net/core/flow.c +8 -6
@@ -47,7 +47,7 @@ struct flow_flush_info {
 
 static struct kmem_cache *flow_cachep __read_mostly;
 
-#define flow_cache_hash_size(cache)	(1 << (cache)->hash_shift)
+#define flow_cache_hash_size(cache)	(1U << (cache)->hash_shift)
 #define FLOW_HASH_RND_PERIOD		(10 * 60 * HZ)
 
 static void flow_cache_new_hashrnd(unsigned long arg)

@@ -119,9 +119,10 @@ static void __flow_cache_shrink(struct flow_cache *fc,
 	struct flow_cache_entry *fle;
 	struct hlist_node *tmp;
 	LIST_HEAD(gc_list);
-	int i, deleted = 0;
+	int deleted = 0;
 	struct netns_xfrm *xfrm = container_of(fc, struct netns_xfrm,
 						flow_cache_global);
+	unsigned int i;
 
 	for (i = 0; i < flow_cache_hash_size(fc); i++) {
 		int saved = 0;

@@ -295,9 +296,10 @@ static void flow_cache_flush_tasklet(unsigned long data)
 	struct flow_cache_entry *fle;
 	struct hlist_node *tmp;
 	LIST_HEAD(gc_list);
-	int i, deleted = 0;
+	int deleted = 0;
 	struct netns_xfrm *xfrm = container_of(fc, struct netns_xfrm,
 						flow_cache_global);
+	unsigned int i;
 
 	fcp = this_cpu_ptr(fc->percpu);
 	for (i = 0; i < flow_cache_hash_size(fc); i++) {

@@ -327,7 +329,7 @@ static void flow_cache_flush_tasklet(unsigned long data)
 static int flow_cache_percpu_empty(struct flow_cache *fc, int cpu)
 {
 	struct flow_cache_percpu *fcp;
-	int i;
+	unsigned int i;
 
 	fcp = per_cpu_ptr(fc->percpu, cpu);
 	for (i = 0; i < flow_cache_hash_size(fc); i++)

@@ -402,12 +404,12 @@ void flow_cache_flush_deferred(struct net *net)
 static int flow_cache_cpu_prepare(struct flow_cache *fc, int cpu)
 {
 	struct flow_cache_percpu *fcp = per_cpu_ptr(fc->percpu, cpu);
-	size_t sz = sizeof(struct hlist_head) * flow_cache_hash_size(fc);
+	unsigned int sz = sizeof(struct hlist_head) * flow_cache_hash_size(fc);
 
 	if (!fcp->hash_table) {
 		fcp->hash_table = kzalloc_node(sz, GFP_KERNEL, cpu_to_node(cpu));
 		if (!fcp->hash_table) {
-			pr_err("NET: failed to allocate flow cache sz %zu\n", sz);
+			pr_err("NET: failed to allocate flow cache sz %u\n", sz);
 			return -ENOMEM;
 		}
 		fcp->hash_rnd_recalc = 1;
maintainer Applied
#2 lore raw

Re: [PATCH v2 2/3] flowcache: make flow_cache_hash_size() return "unsigned int"

From: Alexey Dobriyan <[email protected]>
Date: Mon, 3 Apr 2017 00:52:29 +0300

Quoted 6 lines

Hash size can't negative so "unsigned int" is logically correct.

Propagate "unsigned int" to loop counters.

Space savings:

Quoted 8 lines

add/remove: 0/0 grow/shrink: 2/2 up/down: 6/-18 (-12)
function old new delta
flow_cache_flush_tasklet 362 365 +3
__flow_cache_shrink 333 336 +3
flow_cache_cpu_up_prep 178 171 -7
flow_cache_lookup 1159 1148 -11

Signed-off-by Alexey Dobriyan <[email protected]>

Applied.

author
#3 lore raw

PATCH v2 3/3 flowcache: more "unsigned int"

In reply to Alexey Dobriyan

Make ->hash_count, ->low_watermark and ->high_watermark unsigned int and propagate unsignedness to other variables.

This change doesn't change code generation because these fields aren't used in 64-bit contexts but make it anyway: these fields can't be negative numbers.

Signed-off-by Alexey Dobriyan <[email protected]>

---

 include/net/flowcache.h |    6 +++---
 net/core/flow.c         |   13 +++++++------
 2 files changed, 10 insertions(+), 9 deletions(-)
include/net/flowcache.h +3 -3
@@ -8,7 +8,7 @@
 
 struct flow_cache_percpu {
 	struct hlist_head		*hash_table;
-	int				hash_count;
+	unsigned int			hash_count;
 	u32				hash_rnd;
 	int				hash_rnd_recalc;
 	struct tasklet_struct		flush_tasklet;

@@ -18,8 +18,8 @@ struct flow_cache {
 	u32				hash_shift;
 	struct flow_cache_percpu __percpu *percpu;
 	struct hlist_node		node;
-	int				low_watermark;
-	int				high_watermark;
+	unsigned int			low_watermark;
+	unsigned int			high_watermark;
 	struct timer_list		rnd_timer;
 };
 #endif	/* _NET_FLOWCACHE_H */
net/core/flow.c +7 -6
@@ -99,7 +99,8 @@ static void flow_cache_gc_task(struct work_struct *work)
 }
 
 static void flow_cache_queue_garbage(struct flow_cache_percpu *fcp,
-				     int deleted, struct list_head *gc_list,
+				     unsigned int deleted,
+				     struct list_head *gc_list,
 				     struct netns_xfrm *xfrm)
 {
 	if (deleted) {

@@ -114,18 +115,18 @@ static void flow_cache_queue_garbage(struct flow_cache_percpu *fcp,
 
 static void __flow_cache_shrink(struct flow_cache *fc,
 				struct flow_cache_percpu *fcp,
-				int shrink_to)
+				unsigned int shrink_to)
 {
 	struct flow_cache_entry *fle;
 	struct hlist_node *tmp;
 	LIST_HEAD(gc_list);
-	int deleted = 0;
+	unsigned int deleted = 0;
 	struct netns_xfrm *xfrm = container_of(fc, struct netns_xfrm,
 						flow_cache_global);
 	unsigned int i;
 
 	for (i = 0; i < flow_cache_hash_size(fc); i++) {
-		int saved = 0;
+		unsigned int saved = 0;
 
 		hlist_for_each_entry_safe(fle, tmp,
 					  &fcp->hash_table[i], u.hlist) {

@@ -146,7 +147,7 @@ static void __flow_cache_shrink(struct flow_cache *fc,
 static void flow_cache_shrink(struct flow_cache *fc,
 			      struct flow_cache_percpu *fcp)
 {
-	int shrink_to = fc->low_watermark / flow_cache_hash_size(fc);
+	unsigned int shrink_to = fc->low_watermark / flow_cache_hash_size(fc);
 
 	__flow_cache_shrink(fc, fcp, shrink_to);
 }

@@ -296,7 +297,7 @@ static void flow_cache_flush_tasklet(unsigned long data)
 	struct flow_cache_entry *fle;
 	struct hlist_node *tmp;
 	LIST_HEAD(gc_list);
-	int deleted = 0;
+	unsigned int deleted = 0;
 	struct netns_xfrm *xfrm = container_of(fc, struct netns_xfrm,
 						flow_cache_global);
 	unsigned int i;
maintainer Applied
#4 lore raw

Re: [PATCH v2 3/3] flowcache: more "unsigned int"

From: Alexey Dobriyan <[email protected]>
Date: Mon, 3 Apr 2017 00:53:15 +0300

Quoted 8 lines

Make ->hash_count, ->low_watermark and ->high_watermark unsigned int and propagate unsignedness to other variables.

This change doesn't change code generation because these fields aren't used in 64-bit contexts but make it anyway: these fields can't be negative numbers.

Signed-off-by Alexey Dobriyan <[email protected]>

Applied.