* [PATCH 2/4] flowcache: make flow_cache_hash_size() return "unsigned int"
@ 2017-03-19 22:24 Alexey Dobriyan
2017-03-19 23:13 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Alexey Dobriyan @ 2017-03-19 22:24 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, davem, netdev
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(-)
--- 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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/4] flowcache: make flow_cache_hash_size() return "unsigned int"
2017-03-19 22:24 [PATCH 2/4] flowcache: make flow_cache_hash_size() return "unsigned int" Alexey Dobriyan
@ 2017-03-19 23:13 ` Eric Dumazet
2017-03-20 6:08 ` Alexey Dobriyan
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2017-03-19 23:13 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: steffen.klassert, herbert, davem, netdev
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 ?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/4] flowcache: make flow_cache_hash_size() return "unsigned int"
2017-03-19 23:13 ` Eric Dumazet
@ 2017-03-20 6:08 ` Alexey Dobriyan
0 siblings, 0 replies; 3+ messages in thread
From: Alexey Dobriyan @ 2017-03-20 6:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: steffen.klassert, herbert, davem, netdev
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).
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-20 6:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-19 22:24 [PATCH 2/4] flowcache: make flow_cache_hash_size() return "unsigned int" Alexey Dobriyan
2017-03-19 23:13 ` Eric Dumazet
2017-03-20 6:08 ` Alexey Dobriyan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.