Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords

From: Denys Vlasenko <redacted>
Date: Sun, 19 Jul 2015 17:14:53 +0200

On 07/16/2015 08:17 PM, David Miller wrote:

quoted

From: Tom Herbert <redacted>
Date: Thu, 16 Jul 2015 08:43:25 -0700

quoted

On Thu, Jul 16, 2015 at 5:40 AM, Denys Vlasenko [off-list ref] wrote:

quoted

This patch deinlines jhash, jhash2 and __jhash_nwords.

It also removes rhashtable_jhash2(key, length, seed)
because it was merely calling jhash2(key, length, seed).

With this .config: http://busybox.net/~vda/kernel_config,
after deinlining these functions have sizes and callsite counts
as follows:

__jhash_nwords: 72 bytes, 75 calls
jhash: 297 bytes, 111 calls
jhash2: 205 bytes, 136 calls

jhash is used in several places in the critical data path. Does the
decrease in text size justify performance impact of not inlining it?

Tom took the words right out of my mouth.

Denys, you keep making deinlining changes like this all the time, like
a robot.  But I never see you make any effort to look into the performance
nor code generation ramifications of your changes.

The performance impact of the call/ret pair on modern x86
CPUs is only 5 cycles. To make a real difference in execution
speed, the function has to be less than 100 bytes of code.

What performance metrics have you collected to assert that deinlining
doesn't matter?  What networking tests have you done which stress the
socket demux path?

You still aren't addressing any of my concerns.