Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <0ffb8bc11613ea082179142f7c90830d4e419491.camel@redhat.com>
Subject: Re: [PATCH net-next v2 1/4] indirect call wrappers: helpers to
 speed-up indirect calls of builtin
From:   Paolo Abeni <pabeni@redhat.com>
To:     David Woodhouse <dwmw2@infradead.org>, netdev@vger.kernel.org
Cc:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <eric.dumazet@gmail.com>,
        Paul Turner <pjt@google.com>, linux-kernel@vger.kernel.org
Date:   Tue, 11 Dec 2018 23:28:32 +0100
In-Reply-To: <2c80b9d34540dad337ed2fce6f9d16233105a2c2.camel@infradead.org>
References: <cover.1544032300.git.pabeni@redhat.com>
         <07aaed31eb0b515c173138e1358c412157e58ec2.1544032300.git.pabeni@redhat.com>
         <b72eeb67a0c83d2b497ef5b2537a6211d18f847d.camel@infradead.org>
         <f692efbbc9df09acb5f18ad16907e28cda520e4d.camel@redhat.com>
         <2c80b9d34540dad337ed2fce6f9d16233105a2c2.camel@infradead.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.30.2 (3.30.2-2.fc29) 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

I'm sorry for the long delay, I was (and still I am) diverted by some
other duty.

On Fri, 2018-12-07 at 21:46 +0000, David Woodhouse wrote:
> On Fri, 2018-12-07 at 21:46 +0100, Paolo Abeni wrote:
> > > I wonder if we can declare the common case functions as 'weak' so that
> > > the link failures don't happen when they're absent.
> > 
> > I experimented a previous version with alias. I avoided weak alias
> > usage, because I [mis?]understood not all compilers have a complete
> > support for them (e.g. clang).
> > Also, with weak ref, a coding error that is now discovered at build
> > time will result in worse performance at runtime, likely with some
> > uncommon configuration, possibly not as easily detected. I'm unsure
> > that would be better ?!?
> 
> I think everything supports weak linkage; we've been using it for
> years.

Ok, I likely was confused by some old, non first-hand info.

Anyway weak alias will turn a compile time issue in a possible run-time 
(small) regression. I think the first option would be preferable.

> > I'm sorry, I don't follow here. I think static keys can't be used for
> > the reported network case: we have different list elements each
> > contaning a different function pointer and we access/use
> > different ptr on a per packet basis.
> 
> Yes, the alternatives would be used to change the "likely" case.
> 
> We still do the "if (fn == default_fn) default_fn(); else (*fn)();"
> part; or even the variant with two (or more) common cases. 
> 
> It's just that the value of 'default_fn' can be changed at runtime
> (with patching like alternatives/static keys, since of course it has to
> be a direct call).

Thanks for clarifying. If I understood correctly, you would like some
helper for:

if (static_branch_likely(&use_default_fn_a))
    INDIRECT_CALL_1(f, default_fn_a, <args>)
else if (static_branch_likely(&use_default_fn_b))
    INDIRECT_CALL_1(f, default_fn_b, <args>)
// ...

if so, I think we can eventually add support for this kind of stuff on
top of the proposed macros. WDYT?

Thanks,

Paolo