linux-kernel - Re: [PATCH 3/3] arm64: ftrace: add cond_resched() to func ftrace_make

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181203192228.GC29028@arm.com>
Date:   Mon, 3 Dec 2018 19:22:28 +0000
From:   Will Deacon <will.deacon@....com>
To:     Anders Roxell <anders.roxell@...aro.org>
Cc:     rostedt@...dmis.org, mingo@...hat.com, catalin.marinas@....com,
        keescook@...omium.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH 3/3] arm64: ftrace: add cond_resched() to func
 ftrace_make_(call|nop)

Hi Anders,

On Fri, Nov 30, 2018 at 04:09:56PM +0100, Anders Roxell wrote:
> Both of those functions end up calling ftrace_modify_code(), which is
> expensive because it changes the page tables and flush caches.
> Microseconds add up because this is called in a loop for each dyn_ftrace
> record, and this triggers the softlockup watchdog unless we let it sleep
> occasionally.
> Rework so that we call cond_resched() before going into the
> ftrace_modify_code() function.
> 
> Co-developed-by: Arnd Bergmann <arnd@...db.de>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> Signed-off-by: Anders Roxell <anders.roxell@...aro.org>
> ---
>  arch/arm64/kernel/ftrace.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

It sounds like you're running into issues with the existing code, but I'd
like to understand a bit more about exactly what you're seeing. Which part
of the ftrace patching is proving to be expensive?

The page table manipulation only happens once per module when using PLTs,
and the cache maintenance is just a single line per patch site without an
IPI.

Is it the loop in ftrace_replace_code() that is causing the hassle?

Will