Date:   Fri, 6 Nov 2020 10:44:14 +0800
From:   Guo Ren <guoren@...nel.org>
To:     Atish Patra <atishp@...shpatra.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>, Zong Li <zong.li@...ive.com>,
        Paul McKenney <paulmck@...nel.org>, josh@...htriplett.org,
        mathieu.desnoyers@...icios.com, jiangshanlai@...il.com,
        joel@...lfernandes.org, vincent.whitchurch@...s.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmerdabbelt@...gle.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Greentime Hu <greentime.hu@...ive.com>,
        Colin Ian King <colin.king@...onical.com>,
        rcu@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>
Subject: Re: [PATCH] stop_machine: Mark functions as notrace

On Tue, Nov 3, 2020 at 11:33 PM Atish Patra <atishp@...shpatra.org> wrote:
>
> On Sat, Oct 31, 2020 at 12:42 AM Guo Ren <guoren@...nel.org> wrote:
> >
> > On Sat, Oct 31, 2020 at 8:28 AM Steven Rostedt <rostedt@...dmis.org> wrote:
> > >
> > > On Fri, 30 Oct 2020 14:47:56 -0700
> > > Atish Patra <atishp@...shpatra.org> wrote:
> > >
> > > > > Look at arm64, they __kprobes flag and I guess it would also prevent
> > > > > ftrace call site.
> > > > >
> > > >
> > > > Are you sure about that ? __kprobes puts the code in .kprobes.text section
> > > > which is under whitelist sections in recordmcount.pl & recordmcount.c.
> > >
> > > Correct, ftrace can trace functions marked with __kprobes. That said,
> > I guess wrong, thx for correct me.
> >
> > > the instruction you are looking at here, is in a file that is
> > > blacklisted from recordmcount.
> > >
> > >   CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE)
> > >
> > > All ftrace flags are removed from the compiling of insn.c, and every
> > > function in that file will not be traced.
> > Yes, arm64 prevents the whole file from ftrace. My patch just use
> > notrace flag setting on some functions.
> >
> > @Atish How do think:
> > CFLAGS_REMOVE_cacheflush.o = $(CC_FLAGS_FTRACE)
> > CFLAGS_REMOVE_sbi.o = $(CC_FLAGS_FTRACE)
> >
>
> Looks good to me. What should be done for copy_to_kernel_nofault ?
> That is also in the calling path.

There is no nops' entry in the prologue of copy_to_kernel_nofault.
>>>>
000000000000007c <.LVL6>:
}
  7c:   6105                    addi    sp,sp,32
  7e:   8082                    ret

0000000000000080 <copy_to_user_nofault>:
 *
 * Safely write to address @dst from the buffer at @src.  If a kernel fault
 * happens, handle that and return -EFAULT.
 */
long copy_to_user_nofault(void __user *dst, const void *src, size_t size)
{
  80:   1101                    addi    sp,sp,-32
  82:   e822                    sd      s0,16(sp)
  84:   ec06                    sd      ra,24(sp)
  86:   e426                    sd      s1,8(sp)
  88:   e04a                    sd      s2,0(sp)
  8a:   1000                    addi    s0,sp,32
<<<<
>>>>
cmd_mm/maccess.o :=
/root/source/riscv-tools/install_64gc/bin/riscv64-unknown-linux-gnu-gcc
-Wp,-MMD,mm/.maccess.o.d  -nostdinc -isystem
/root/source/riscv-tools/install_64gc/bin/../lib/gcc/riscv64-unknown-linux-gnu/8.4.0/include
-I./arch/riscv/include -I./arch/riscv/include/generated  -I./include
-I./arch/riscv/include/uapi -I./arch/riscv/include/generated/uapi
-I./include/uapi -I./include/generated/uapi -include
./include/linux/kconfig.h -include ./include/linux/compiler_types.h
-D__KERNEL__ ***-DCC_USING_PATCHABLE_FUNCTION_ENTRY*** -Wall -Wundef
-Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing
-fno-common -fshort-wchar -fno-PIE
-Werror=implicit-function-declaration -Werror=implicit-int
-Wno-format-security -std=gnu89 -mabi=lp64 -march=rv64imac
-mno-save-restore -DCONFIG_PAGE_OFFSET=0xffffffe000000000
-mcmodel=medany -fno-omit-frame-pointer -mstrict-align
-fno-delete-null-pointer-checks -Wno-frame-address
-Wno-format-truncation -Wno-format-overflow -O2
--param=allow-store-data-races=0 -Wframe-larger-than=2048
-fstack-protector-strong -Wno-unused-but-set-variable
-Wimplicit-fallthrough -Wno-unused-const-variable
-fno-omit-frame-pointer -fno-optimize-sibling-calls
-fno-var-tracking-assignments -g ***-fpatchable-function-entry=8***
-Wdeclaration-after-statement -Wvla -Wno-pointer-sign
-Wno-stringop-truncation -Wno-array-bounds -Wno-stringop-overflow
-Wno-restrict -Wno-maybe-uninitialized -fno-strict-overflow
-fno-merge-all-constants -fmerge-constants -fno-stack-check
-fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types
-Werror=designated-init -fmacro-prefix-map=./= -Wno-packed-not-aligned
   -DKBUILD_MODFILE='"mm/maccess"' -DKBUILD_BASENAME='"maccess"'
-DKBUILD_MODNAME='"maccess"' -c -o mm/maccess.o mm/maccess.c
<<<<

But copy_from_user_nofault has:
000000000000007c <.LVL6>:
}
  7c:   6105                    addi    sp,sp,32
  7e:   8082                    ret

0000000000000080 <copy_to_user_nofault>:
 *
 * Safely write to address @dst from the buffer at @src.  If a kernel fault
 * happens, handle that and return -EFAULT.
 */
long copy_to_user_nofault(void __user *dst, const void *src, size_t size)
{
  80:   1101                    addi    sp,sp,-32
  82:   e822                    sd      s0,16(sp)
  84:   ec06                    sd      ra,24(sp)
  86:   e426                    sd      s1,8(sp)
  88:   e04a                    sd      s2,0(sp)
  8a:   1000                    addi    s0,sp,32


I think it's a gcc problem, but satisfy our ftrace requirement.





--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Powered by blists - more mailing lists