| Message ID | 1366726763.26911.417.camel@localhost |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Apr 23, 2013 at 04:19:23PM +0200, Jesper Dangaard Brouer wrote: > Yes, traffic patterns do affect the results, BUT you have to be really > careful profiling this: > > Notice, that inet_frag_find() also indirectly takes the LRU lock, and > the perf tool will blame inet_frag_find(). This is very subtle and > happens with a traffic pattern that want to create new frag queues (e.g. > not found in the hash list). > The problem is that inet_frag_find() calls inet_frag_create() (if q is > not found) which calls inet_frag_intern() which calls > inet_frag_lru_add() taking the LRU lock. All of these functions gets > inlined by the compiler, thus inet_frag_find() gets the blame. > > > To avoid pissing people off: Ah, come on. I don't think you do that. ;) > Yes, having a long list in the hash bucket is obviously also contributes > significantly. Yes, we still should increase the hash bucket size. I'm > just pointing out be careful about what you actually profile ;-) > > > Please see below, profiling of current next-next, with "noinline" added > to inet_frag_intern, inet_frag_alloc and inet_frag_create. Run under > test 20G3F+MQ. I hope you can see my point with the LRU list lock, > please let me know if I have missed something. I have no objections. My ipv6 test case simply does not get the memory usage of the fragment cache over the thresholds, so I have no contention there, just dropping because of the 128 list length limit. As soon as I would fill up the fragment cache the contention will be on the lru lock as you showed here. Greetings, Hannes -- 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
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c index e97d66a..d899bba 100644 --- a/net/ipv4/inet_fragment.c +++ b/net/ipv4/inet_fragment.c @@ -241,7 +241,7 @@ int inet_frag_evictor(struct netns_frags *nf, struct inet_frags *f, bool force) } EXPORT_SYMBOL(inet_frag_evictor); -static struct inet_frag_queue *inet_frag_intern(struct netns_frags *nf, +static noinline struct inet_frag_queue *inet_frag_intern(struct netns_frags *nf, struct inet_frag_queue *qp_in, struct inet_frags *f, void *arg) { @@ -289,7 +289,7 @@ static struct inet_frag_queue *inet_frag_intern(struct netns_frags *nf, return qp; } -static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf, +static noinline struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf, struct inet_frags *f, void *arg) { struct inet_frag_queue *q; @@ -309,7 +309,7 @@ static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf, return q; } -static struct inet_frag_queue *inet_frag_create(struct netns_frags *nf, +static noinline struct inet_frag_queue *inet_frag_create(struct netns_frags *nf, struct inet_frags *f, void *arg) { struct inet_frag_queue *q;