Message ID | 46998c941d0a5664daaeb92998391aace015eddf.1620829724.git.christophe.leroy@csgroup.eu (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [1/2] kprobes: Allow architectures to override optinsn page allocation | expand |
Related | show |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (1fab3666d738e4af3a7450c44441310e4d7a7e53) |
snowpatch_ozlabs/build-ppc64le | warning | Build succeeded but added 4 new sparse warnings |
snowpatch_ozlabs/build-ppc64be | warning | Build succeeded but added 4 new sparse warnings |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 64 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Hi Christophe, I love your patch! Perhaps something to improve: [auto build test WARNING on powerpc/next] [also build test WARNING on linus/master v5.13-rc1 next-20210512] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-allyesconfig (attached as .config) compiler: powerpc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/1cc4bb503e4ee4839247b767883c8b860b26413c git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121 git checkout 1cc4bb503e4ee4839247b767883c8b860b26413c # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> arch/powerpc/kernel/optprobes.c:36:7: warning: no previous prototype for 'alloc_optinsn_page' [-Wmissing-prototypes] 36 | void *alloc_optinsn_page(void) | ^~~~~~~~~~~~~~~~~~ >> arch/powerpc/kernel/optprobes.c:44:6: warning: no previous prototype for 'free_optinsn_page' [-Wmissing-prototypes] 44 | void free_optinsn_page(void *page) | ^~~~~~~~~~~~~~~~~ vim +/alloc_optinsn_page +36 arch/powerpc/kernel/optprobes.c 35 > 36 void *alloc_optinsn_page(void) 37 { 38 if (insn_page_in_use) 39 return NULL; 40 insn_page_in_use = true; 41 return &optinsn_slot; 42 } 43 > 44 void free_optinsn_page(void *page) 45 { 46 insn_page_in_use = false; 47 } 48 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Christophe, I love your patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on linus/master v5.13-rc1 next-20210512] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-ppc64_defconfig (attached as .config) compiler: powerpc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/1cc4bb503e4ee4839247b767883c8b860b26413c git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121 git checkout 1cc4bb503e4ee4839247b767883c8b860b26413c # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> arch/powerpc/kernel/optprobes.c:36:7: error: no previous prototype for 'alloc_optinsn_page' [-Werror=missing-prototypes] 36 | void *alloc_optinsn_page(void) | ^~~~~~~~~~~~~~~~~~ >> arch/powerpc/kernel/optprobes.c:44:6: error: no previous prototype for 'free_optinsn_page' [-Werror=missing-prototypes] 44 | void free_optinsn_page(void *page) | ^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors vim +/alloc_optinsn_page +36 arch/powerpc/kernel/optprobes.c 35 > 36 void *alloc_optinsn_page(void) 37 { 38 if (insn_page_in_use) 39 return NULL; 40 insn_page_in_use = true; 41 return &optinsn_slot; 42 } 43 > 44 void free_optinsn_page(void *page) 45 { 46 insn_page_in_use = false; 47 } 48 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, 12 May 2021 14:29:27 +0000 (UTC) Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > Commit 51c9c0843993 ("powerpc/kprobes: Implement Optprobes") > implemented a powerpc specific version of optinsn in order > to workaround the 32Mb limitation for direct branches. > > Instead of implementing a dedicated powerpc version, use the > common optinsn and override the allocation and freeing functions. > > This also indirectly remove the CLANG warning about > is_kprobe_ppc_optinsn_slot() not being use, and the powerpc will > now benefit from commit 5b485629ba0d ("kprobes, extable: Identify > kprobes trampolines as kernel text area") > This looks good to me. Acked-by: Masami Hiramatsu <mhiramat@kernel.org> > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/kernel/optprobes.c | 23 +++++------------------ > 1 file changed, 5 insertions(+), 18 deletions(-) > > diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c > index cdf87086fa33..a370190cd02a 100644 > --- a/arch/powerpc/kernel/optprobes.c > +++ b/arch/powerpc/kernel/optprobes.c > @@ -31,11 +31,9 @@ > #define TMPL_END_IDX \ > (optprobe_template_end - optprobe_template_entry) > > -DEFINE_INSN_CACHE_OPS(ppc_optinsn); > - > static bool insn_page_in_use; > > -static void *__ppc_alloc_insn_page(void) > +void *alloc_optinsn_page(void) > { > if (insn_page_in_use) > return NULL; > @@ -43,20 +41,11 @@ static void *__ppc_alloc_insn_page(void) > return &optinsn_slot; > } > > -static void __ppc_free_insn_page(void *page __maybe_unused) > +void free_optinsn_page(void *page) > { > insn_page_in_use = false; > } > > -struct kprobe_insn_cache kprobe_ppc_optinsn_slots = { > - .mutex = __MUTEX_INITIALIZER(kprobe_ppc_optinsn_slots.mutex), > - .pages = LIST_HEAD_INIT(kprobe_ppc_optinsn_slots.pages), > - /* insn_size initialized later */ > - .alloc = __ppc_alloc_insn_page, > - .free = __ppc_free_insn_page, > - .nr_garbage = 0, > -}; > - > /* > * Check if we can optimize this probe. Returns NIP post-emulation if this can > * be optimized and 0 otherwise. > @@ -136,7 +125,7 @@ NOKPROBE_SYMBOL(optimized_callback); > void arch_remove_optimized_kprobe(struct optimized_kprobe *op) > { > if (op->optinsn.insn) { > - free_ppc_optinsn_slot(op->optinsn.insn, 1); > + free_optinsn_slot(op->optinsn.insn, 1); > op->optinsn.insn = NULL; > } > } > @@ -203,14 +192,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) > unsigned long nip, size; > int rc, i; > > - kprobe_ppc_optinsn_slots.insn_size = MAX_OPTINSN_SIZE; > - > nip = can_optimize(p); > if (!nip) > return -EILSEQ; > > /* Allocate instruction slot for detour buffer */ > - buff = get_ppc_optinsn_slot(); > + buff = get_optinsn_slot(); > if (!buff) > return -ENOMEM; > > @@ -297,7 +284,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) > return 0; > > error: > - free_ppc_optinsn_slot(buff, 0); > + free_optinsn_slot(buff, 0); > return -ERANGE; > > } > -- > 2.25.0 >
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c index cdf87086fa33..a370190cd02a 100644 --- a/arch/powerpc/kernel/optprobes.c +++ b/arch/powerpc/kernel/optprobes.c @@ -31,11 +31,9 @@ #define TMPL_END_IDX \ (optprobe_template_end - optprobe_template_entry) -DEFINE_INSN_CACHE_OPS(ppc_optinsn); - static bool insn_page_in_use; -static void *__ppc_alloc_insn_page(void) +void *alloc_optinsn_page(void) { if (insn_page_in_use) return NULL; @@ -43,20 +41,11 @@ static void *__ppc_alloc_insn_page(void) return &optinsn_slot; } -static void __ppc_free_insn_page(void *page __maybe_unused) +void free_optinsn_page(void *page) { insn_page_in_use = false; } -struct kprobe_insn_cache kprobe_ppc_optinsn_slots = { - .mutex = __MUTEX_INITIALIZER(kprobe_ppc_optinsn_slots.mutex), - .pages = LIST_HEAD_INIT(kprobe_ppc_optinsn_slots.pages), - /* insn_size initialized later */ - .alloc = __ppc_alloc_insn_page, - .free = __ppc_free_insn_page, - .nr_garbage = 0, -}; - /* * Check if we can optimize this probe. Returns NIP post-emulation if this can * be optimized and 0 otherwise. @@ -136,7 +125,7 @@ NOKPROBE_SYMBOL(optimized_callback); void arch_remove_optimized_kprobe(struct optimized_kprobe *op) { if (op->optinsn.insn) { - free_ppc_optinsn_slot(op->optinsn.insn, 1); + free_optinsn_slot(op->optinsn.insn, 1); op->optinsn.insn = NULL; } } @@ -203,14 +192,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) unsigned long nip, size; int rc, i; - kprobe_ppc_optinsn_slots.insn_size = MAX_OPTINSN_SIZE; - nip = can_optimize(p); if (!nip) return -EILSEQ; /* Allocate instruction slot for detour buffer */ - buff = get_ppc_optinsn_slot(); + buff = get_optinsn_slot(); if (!buff) return -ENOMEM; @@ -297,7 +284,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) return 0; error: - free_ppc_optinsn_slot(buff, 0); + free_optinsn_slot(buff, 0); return -ERANGE; }
Commit 51c9c0843993 ("powerpc/kprobes: Implement Optprobes") implemented a powerpc specific version of optinsn in order to workaround the 32Mb limitation for direct branches. Instead of implementing a dedicated powerpc version, use the common optinsn and override the allocation and freeing functions. This also indirectly remove the CLANG warning about is_kprobe_ppc_optinsn_slot() not being use, and the powerpc will now benefit from commit 5b485629ba0d ("kprobes, extable: Identify kprobes trampolines as kernel text area") Suggested-by: Masami Hiramatsu <mhiramat@kernel.org> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/kernel/optprobes.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-)