linux-kernel - Re: [PATCH] arm64:align function __arch_clear

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: <20210423153701.GP18757@arm.com>
Date:   Fri, 23 Apr 2021 16:37:02 +0100
From:   Catalin Marinas <catalin.marinas@....com>
To:     Kai Shen <shenkai8@...wei.com>
Cc:     will@...nel.org, linux-arm-kernel@...ts.infradead.org,
        LKML <linux-kernel@...r.kernel.org>, xuwei5@...ilicon.com,
        hewenliang4@...wei.com, wuxu.wu@...wei.com
Subject: Re: [PATCH] arm64:align function __arch_clear_user

On Mon, Apr 19, 2021 at 10:05:16AM +0800, Kai Shen wrote:
> On 2021/4/14 18:41, Catalin Marinas wrote:
> > On Wed, Apr 14, 2021 at 05:25:43PM +0800, Kai Shen wrote:
> > > Performance decreases happen in __arch_clear_user when this
> > > function is not correctly aligned on HISI-HIP08 arm64 SOC which
> > > fetches 32 bytes (8 instructions) from icache with a 32-bytes
> > > aligned end address. As a result, if the hot loop is not 32-bytes
> > > aligned, it may take more icache fetches which leads to decrease
> > > in performance.
> > > Dump of assembler code for function __arch_clear_user:
> > >         0xffff0000809e3f10 :    nop
> > >         0xffff0000809e3f14 :    mov x2, x1
> > >         0xffff0000809e3f18 :    subs x1, x1, #0x8
> > >         0xffff0000809e3f1c :    b.mi 0xffff0000809e3f30 <__arch_clear_user+3
> > > -----  0xffff0000809e3f20 :    str    xzr, [x0],#8
> > > hot    0xffff0000809e3f24 :    nop
> > > loop   0xffff0000809e3f28 :    subs x1, x1, #0x8
> > > -----  0xffff0000809e3f2c :    b.pl  0xffff0000809e3f20 <__arch_clear_user+1
> > > The hot loop above takes one icache fetch as the code is in one
> > > 32-bytes aligned area and the loop takes one more icache fetch
> > > when it is not aligned like below.
> > >         0xffff0000809e4178 :   str    xzr, [x0],#8
> > >         0xffff0000809e417c :   nop
> > >         0xffff0000809e4180 :   subs x1, x1, #0x8
> > >         0xffff0000809e4184 :   b.pl  0xffff0000809e4178 <__arch_clear_user+
> > > Data collected by perf:
> > >                           aligned   not aligned
> > >            instructions   57733790     57739065
> > >         L1-dcache-store   14938070     13718242
> > > L1-dcache-store-misses     349280       349869
> > >         L1-icache-loads   15380895     28500665
> > > As we can see, L1-icache-loads almost double when the loop is not
> > > aligned.
> > > This problem is found in linux 4.19 on HISI-HIP08 arm64 SOC.
> > > Not sure what the case is on other arm64 SOC, but it should do
> > > no harm.
> > > Signed-off-by: Kai Shen <shenkai8@...wei.com>
> > 
> > Do you have a real world workload that's affected by this function?
> > 
> > I'm against adding alignments and nops for specific hardware
> > implementations. What about lots of other loops that the compiler may
> > generate or that we wrote in asm?
>
> The benchmark we used which suffer performance decrease:
>     https://github.com/redhat-performance/libMicro
>     pread $OPTS -N "pread_z1k"    -s 1k    -I 300  -f /dev/zero
>     pread $OPTS -N "pread_z10k"    -s 10k    -I 1000 -f /dev/zero
>     pread $OPTS -N "pread_z100k"    -s 100k    -I 2000 -f /dev/zero

Is there any real world use-case that would benefit from this
optimisation? Reading /dev/zero in a loop hardly counts as a practical
workload.

-- 
Catalin