| Message ID | 20170319222449.GB17015@avx2 |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2017-03-20 at 01:24 +0300, Alexey Dobriyan wrote: > Hash size can't negative so "unsigned int" is logically correct. > 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); I do not see any improvement here. What is wrong with size_t exactly ?
On Sun, Mar 19, 2017 at 04:13:41PM -0700, Eric Dumazet wrote: > On Mon, 2017-03-20 at 01:24 +0300, Alexey Dobriyan wrote: > > Hash size can't negative so "unsigned int" is logically correct. > > > > 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); > > > I do not see any improvement here. > > What is wrong with size_t exactly ? REX prefixes and sign extensions, lots of them. Before: ffffffff8505b1b5: 41 bd 01 00 00 00 mov r13d,0x1 ffffffff8505b1bb: 41 d3 e5 shl r13d,cl ffffffff8505b1be: 4d 63 ed movsxd r13,r13d ffffffff8505b1c1: 49 c1 e5 03 shl r13,0x3 ffffffff8505b1c5: e8 86 28 0a fc call __cpu_to_node ... ffffffff8505b20b: 4c 89 ee mov rsi,r13 After: ffffffff8505b1b5: 41 bd 08 00 00 00 mov r13d,0x8 ffffffff8505b1bb: 41 d3 e5 shl r13d,cl ffffffff8505b1be: e8 8d 28 0a fc call __cpu_to_node ... ffffffff8505b1c3: 44 89 ef mov edi,r13d Basically, one can do s/size_t/unsigned int/g across whole networking stack and nothing will change but the code becomes smaller (including things like sendmsg() because VFS truncates lengths at INT_MAX).
--- a/net/core/flow.c +++ b/net/core/flow.c @@ -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;
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 Total: Before=170822884, After=170822872, chg -0.00% Lookup becomes smaller and this is what matters. Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- net/core/flow.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)