Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Sven Schnelle <svens@linux.ibm.com>
To:     Heiko Carstens <hca@linux.ibm.com>
Cc:     Yinan Liu <yinan@linux.alibaba.com>, rostedt@goodmis.org,
        peterz@infradead.org, mark-pk.tsai@mediatek.com, mingo@redhat.com,
        linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v8] scripts: ftrace - move the sort-processing in
 ftrace_init
References: <20210911135043.16014-1-yinan@linux.alibaba.com>
        <20211212113358.34208-1-yinan@linux.alibaba.com>
        <20211212113358.34208-2-yinan@linux.alibaba.com>
        <yt9dee51ctfn.fsf@linux.ibm.com> <YeqOFHNfxKcNXNrn@osiris>
        <yt9d8rv9cpdq.fsf@linux.ibm.com>
Date:   Fri, 21 Jan 2022 12:29:21 +0100
In-Reply-To: <yt9d8rv9cpdq.fsf@linux.ibm.com> (Sven Schnelle's message of
        "Fri, 21 Jan 2022 12:14:09 +0100")
Message-ID: <yt9d4k5xcooe.fsf@linux.ibm.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Sven Schnelle <svens@linux.ibm.com> writes:

> Heiko Carstens <hca@linux.ibm.com> writes:
>
>> On Fri, Jan 21, 2022 at 10:46:36AM +0100, Sven Schnelle wrote:
>>> Hi Yinan,
>>> 
>>> Yinan Liu <yinan@linux.alibaba.com> writes:
>>> 
>>> > When the kernel starts, the initialization of ftrace takes
>>> > up a portion of the time (approximately 6~8ms) to sort mcount
>>> > addresses. We can save this time by moving mcount-sorting to
>>> > compile time.
>>> >
>>> > Signed-off-by: Yinan Liu <yinan@linux.alibaba.com>
>>> > Reported-by: kernel test robot <lkp@intel.com>
>>> > Reported-by: kernel test robot <oliver.sang@intel.com>
>>> > ---
>>> >  kernel/trace/ftrace.c   |  11 +++-
>>> >  scripts/Makefile        |   6 +-
>>> >  scripts/link-vmlinux.sh |   6 +-
>>> >  scripts/sorttable.c     |   2 +
>>> >  scripts/sorttable.h     | 120 +++++++++++++++++++++++++++++++++++++++-
>>> >  5 files changed, 137 insertions(+), 8 deletions(-)
>>> 
>>> while i like the idea, this unfortunately breaks ftrace on s390. The
>>> reason for that is that the compiler generates relocation entries for
>>> all the addresses in __mcount_loc. During boot, the s390 decompressor
>>> iterates through all the relocations and overwrites the nicely
>>> sorted list between __start_mcount_loc and __stop_mcount_loc with
>>> the unsorted list because the relocations entries are not adjusted.
>>> 
>>> Of course we could just disable that option, but that would make us
>>> different compared to x86 which i don't like. Adding code to sort the
>>> relocation would of course also fix that, but i don't think it is a good
>>> idea to rely on the order of relocations.
>>> 
>>> Any thoughts how a fix could look like, and whether that could also be a
>>> problem on other architectures?
>>
>> Sven, thanks for figuring this out. Can you confirm that reverting
>> commit 72b3942a173c ("scripts: ftrace - move the sort-processing in
>> ftrace_init") fixes the problem?
>
> Yes, reverting this commit fixes it.
>
>> This really should be addressed before rc1 is out, otherwise s390 is
>> broken if somebody enables ftrace.
>> Where "broken" translates to random crashes as soon as ftrace is
>> enabled, which again is nowadays quite common.
>
> I wasn't able to reproduce these crashes on my systems so far. For the
> readers here, we're seeing about 10-15 systems crashing every night,
> usually in the 00basic/ ftrace testcases.
>
> In most of the case it looks like register corruption, where some random
> register is or'd or parts are overwritten with 0x0004000000000000,
> sometimes 0x00f4000000000000. I haven't found yts found a commit that
> might cause this.

Thinking of it, 04 and f4 are exactly the bytes we're patching in our brcl
instructions right at the beginning of the function. So i guess that
because of this bug the ftrace code now writes those bytes to the wrong
location, sometimes hitting the register save area. I haven't verified
that, but i think there's a high likelyhood.

/Sven