All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
To: Jay Vosburgh <jay.vosburgh@canonical.com>
Cc: David Miller <davem@davemloft.net>,
	eric.dumazet@gmail.com, netdev@vger.kernel.org,
	ogerlitz@mellanox.com, pshelar@nicira.com, jesse@nicira.com,
	discuss@openvswitch.org
Subject: Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
Date: Fri, 14 Nov 2014 21:35:09 +0100	[thread overview]
Message-ID: <1415997309.15154.59.camel@localhost> (raw)
In-Reply-To: <17658.1415996115@famine>

On Fr, 2014-11-14 at 12:15 -0800, Jay Vosburgh wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> 
> >On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
> >> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> >> Date: Fri, 14 Nov 2014 16:46:18 +0100
> >> 
> >> > I would still like to see the current proposed fix getting applied and
> >> > we can do this on-top. The inline call after this patch reassembles a
> >> > direct function call, so besides the long list of clobbers, it should
> >> > still be pretty fast.
> >> 
> >> I would rather revert the change entirely until it is implemented
> >> properly.
> >> 
> >> Also, I am strongly of the opinion that this is a mis-use of the
> >> alternative call interface.  It was never intended to be used for
> >> things that can make real function calls.
> >
> >I tend to disagree. Grepping e.g. shows
> >
> >        alternative_call_2(copy_user_generic_unrolled,
> >                         copy_user_generic_string,
> >                         X86_FEATURE_REP_GOOD,
> >                         copy_user_enhanced_fast_string,
> >                         X86_FEATURE_ERMS,
> >                         ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
> >                                     "=d" (len)),
> >                         "1" (to), "2" (from), "3" (len)
> >                         : "memory", "rcx", "r8", "r9", "r10", "r11");
> >
> >
> >(it has a few less clobbers because it has more output operands)
> 
> 	As those functions (copy_user_generic_unrolled, et al) are all
> in assembly language, presumably the list of clobbered registers can be
> had via inspection.
> 
> 	For the arch_fast_hash2 case, the functions (__intel_crc4_2_hash
> and __jash2) are both written in C, so how would the clobber list be
> created?

I created it via the function calling convention documented in
arch/x86/include/asm/calling.h, so I specified each register which a
function is allowed to clobber with.

I currently cannot see how I can resolve the invalid constraints error
easily. :(

So either go with my first patch, which I puts the alternative_call
switch point into its own function without ever inlining or the patch
needs to be reverted. :/

Bye,
Hannes

  reply	other threads:[~2014-11-14 20:35 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-14 14:06 [PATCH net-next] fast_hash: clobber registers correctly for inline function use Hannes Frederic Sowa
2014-11-14 14:40 ` [PATCH net-next v2] " Hannes Frederic Sowa
2014-11-14 14:50 ` [PATCH net-next] " Eric Dumazet
2014-11-14 15:13   ` Hannes Frederic Sowa
2014-11-14 15:33     ` Eric Dumazet
2014-11-14 15:46       ` Hannes Frederic Sowa
2014-11-14 18:38         ` David Miller
2014-11-14 19:02           ` Cong Wang
2014-11-14 20:42             ` Hannes Frederic Sowa
2014-11-14 21:35               ` David Miller
2014-11-14 19:05           ` [PATCH net-next] Revert "fast_hash: avoid indirect function calls" Jay Vosburgh
2014-11-14 21:36             ` David Miller
2014-11-14 21:43             ` Hannes Frederic Sowa
2014-11-14 20:04           ` [PATCH net-next] fast_hash: clobber registers correctly for inline function use Hannes Frederic Sowa
2014-11-14 20:10             ` Hannes Frederic Sowa
2014-11-14 20:15             ` Jay Vosburgh
2014-11-14 20:35               ` Hannes Frederic Sowa [this message]
2014-11-14 22:10                 ` Jay Vosburgh
2014-11-14 22:37                   ` Hannes Frederic Sowa
2014-11-14 15:17   ` [PATCH net-next v3] " Hannes Frederic Sowa
2014-11-14 17:57     ` Jay Vosburgh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1415997309.15154.59.camel@localhost \
    --to=hannes@stressinduktion.org \
    --cc=davem@davemloft.net \
    --cc=discuss@openvswitch.org \
    --cc=eric.dumazet@gmail.com \
    --cc=jay.vosburgh@canonical.com \
    --cc=jesse@nicira.com \
    --cc=netdev@vger.kernel.org \
    --cc=ogerlitz@mellanox.com \
    --cc=pshelar@nicira.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.