Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1186203

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

Show all headers | View raw

On Thu, 2015-07-16 at 11:17 -0700, David Miller wrote:
> From: Tom Herbert <tom@herbertland.com>
> Date: Thu, 16 Jul 2015 08:43:25 -0700
> 
> > On Thu, Jul 16, 2015 at 5:40 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
> >> 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.
> 
> And frankly that makes your patches quite tiring to deal with.
> 
> Your changes potentially have large performance implications, yet you
> don't put any effort into considering that aspect at all.

It might be useful to have these performance impacting
changes guarded by something like CONFIG_CC_OPTIMIZE_FOR_SIZE
with another static __always_inline __<func> and a function &
EXPORT_SYMBOL or just a static inline so that where code size
is critical it's uninlined.

Though even for tiny embedded uses, the additional code
complexity might not be worth it.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Tom Herbert <tom@herbertland.com> - 2015-07-16 17:50 +0200
  Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords David Miller <davem@davemloft.net> - 2015-07-16 20:20 +0200
    Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Joe Perches <joe@perches.com> - 2015-07-16 21:30 +0200
      Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Hagen Paul Pfeifer <hagen@jauu.net> - 2015-07-17 15:50 +0200
        Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Joe Perches <joe@perches.com> - 2015-07-18 01:00 +0200
    Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords David Miller <davem@davemloft.net> - 2015-07-19 20:50 +0200

csiph-web