All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Kosina <jikos@kernel.org>
To: Torsten Duwe <duwe@lst.de>
Cc: Miroslav Benes <mbenes@suse.cz>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	matz@suse.de, live-patching@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Disable non-ABI-compliant optimisations for live patching
Date: Thu, 23 Jun 2016 14:47:03 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LNX.2.00.1606231429280.6874@cbobk.fhfr.pm> (raw)
In-Reply-To: <alpine.LNX.2.00.1606231237470.6874@cbobk.fhfr.pm>

On Thu, 23 Jun 2016, Jiri Kosina wrote:

> > I haven't looked at the fentry solution, but the code I'm involved in saves
> > the registers so that ftrace, live patch and friends can work freely. But
> > then it restores all regs and _then_ calls the replacement, so ftrace
> > saving all regs is no gain at all.
> 
> You're right, thanks for bringing this up.
> 
> In principle we should be able to modify the trampoline so that it 
> performs its own register saving (in ftrace_regs_caller) and restoring 
> (*), completely shielding the new function from any optimization gcc might 
> have done on registers, shouldn't we?
> 
> (*) we'll have to piggy-back on ftrace_epilogue on that, i.e. making the 
>     return to the original code go through trampoline as well (the same 
>     way graph tracer works)

Okay, after looking more about how ftrace implements the return 
trampolines for graph caller, it'd be rather difficult to implement in a 
way that we neither interfere with ftrace graph tracer (the 
ftrace_ret_stack in task_struct) nor introduce a serious performance 
overhead or stack usage pressure.

I am pretty sure the overhead we'd be adding would be much worse than just 
really simply turning the IPA-RA off in CONFIG_LIVEPATCH-enabled kernels 
is the easiest way to go.

After talking to Jan Hubicka, I'd actually suggest turning off most/all 
the IPA optimizations; they are supposed to be of questionable benefit for 
kernel anyway, and they might be causing serious issues for us.

I am planning to ask our performance team to measure the impact this'd 
have.

Thanks,

-- 
Jiri Kosina
SUSE Labs

  reply	other threads:[~2016-06-23 12:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-22 14:24 [PATCH] Disable non-ABI-compliant optimisations for live patching Torsten Duwe
2016-06-22 15:19 ` Josh Poimboeuf
2016-06-23  7:45   ` Miroslav Benes
2016-06-23 10:05     ` Torsten Duwe
2016-06-23 10:45       ` Jiri Kosina
2016-06-23 12:47         ` Jiri Kosina [this message]
2016-06-26 22:39           ` Pavel Machek
2016-06-27  6:59             ` Torsten Duwe
2016-06-26 22:37 ` Pavel Machek
2016-06-27  8:13   ` Jiri Kosina
2016-06-27  8:21     ` Pavel Machek
2016-06-27  8:26       ` Jiri Kosina
2016-06-27  8:32         ` Pavel Machek
2016-06-27 11:36           ` Jiri Kosina

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=alpine.LNX.2.00.1606231429280.6874@cbobk.fhfr.pm \
    --to=jikos@kernel.org \
    --cc=duwe@lst.de \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=matz@suse.de \
    --cc=mbenes@suse.cz \
    /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.