| Message ID | 20170930025319.987-2-sergey.senozhatsky@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
On Sat 2017-09-30 11:53:13, Sergey Senozhatsky wrote: > Convert dereference_function_descriptor() to accept and return > `unsigned long'. There will be two new ARCH function for kernel > and module function pointer dereference, which will work with > `unsigned long', so the patch unifies interfaces. > > Besides, dereference_function_descriptor() mostly work with > `unsigned long': > > Convert dereference_function_descriptor() users tree-wide. I am not sure if this is a real win. If I count correctly, the net result is 6 additional casts in this patch. Many casts are still needed also in the other patches. > diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h > index 2ab2003698ef..de6bfa1ef8fb 100644 > --- a/arch/ia64/include/asm/sections.h > +++ b/arch/ia64/include/asm/sections.h > @@ -27,13 +27,13 @@ extern char __start_unwind[], __end_unwind[]; > extern char __start_ivt_text[], __end_ivt_text[]; > > #undef dereference_function_descriptor > -static inline void *dereference_function_descriptor(void *ptr) > +static inline unsigned long dereference_function_descriptor(unsigned long ptr) I would also expect that a function called "dereference*" would work with a pointer. The parameter is called ptr but it is unsigned long. > { > - struct fdesc *desc = ptr; > + struct fdesc *desc = (struct fdesc *)ptr; > void *p; > > if (!probe_kernel_address(&desc->ip, p)) > - ptr = p; > + ptr = (unsigned long)p; > return ptr; > } > > diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c > index 1ca9a2b4239f..06e1b79e2946 100644 > --- a/arch/parisc/mm/init.c > +++ b/arch/parisc/mm/init.c > @@ -389,10 +389,10 @@ static void __init setup_bootmem(void) > static int __init parisc_text_address(unsigned long vaddr) > { > static unsigned long head_ptr __initdata; > + unsigned long addr = (unsigned long)&parisc_kernel_start; > > if (!head_ptr) > - head_ptr = PAGE_MASK & (unsigned long) > - dereference_function_descriptor(&parisc_kernel_start); > + head_ptr = PAGE_MASK & dereference_function_descriptor(addr); IMHO, this is harder to read than the original. You need to search the definition of "addr" and check its manipulation to understand the meaning. > > return core_kernel_text(vaddr) || vaddr == head_ptr; > } To make it clear. All these comments are not a big deal and I do not want to invalidate all the acked-by and tested-by just because of them. But please, consider removing this change if we need to do another iteration of this patchset. IMHO, there is no real win and it would just pollute the git history. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On (10/04/17 10:24), Petr Mladek wrote: [..] > To make it clear. All these comments are not a big deal and I do > not want to invalidate all the acked-by and tested-by just because > of them. > > But please, consider removing this change if we need to do > another iteration of this patchset. IMHO, there is no real win > and it would just pollute the git history. I tend to rather keep it. would that cause any problems on your side? it saves a ton of ugly casts later in the patch set and lets us to drop some casts from the modules code/etc. the patch is here because otherwise I had to add a bunch of new casts. -ss -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu 2017-10-19 15:50:04, Sergey Senozhatsky wrote: > On (10/04/17 10:24), Petr Mladek wrote: > [..] > > To make it clear. All these comments are not a big deal and I do > > not want to invalidate all the acked-by and tested-by just because > > of them. > > > > But please, consider removing this change if we need to do > > another iteration of this patchset. IMHO, there is no real win > > and it would just pollute the git history. > > I tend to rather keep it. would that cause any problems on your side? > it saves a ton of ugly casts later in the patch set and lets us to drop > some casts from the modules code/etc. the patch is here because otherwise > I had to add a bunch of new casts. OK, let's keep it. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h index 2ab2003698ef..de6bfa1ef8fb 100644 --- a/arch/ia64/include/asm/sections.h +++ b/arch/ia64/include/asm/sections.h @@ -27,13 +27,13 @@ extern char __start_unwind[], __end_unwind[]; extern char __start_ivt_text[], __end_ivt_text[]; #undef dereference_function_descriptor -static inline void *dereference_function_descriptor(void *ptr) +static inline unsigned long dereference_function_descriptor(unsigned long ptr) { - struct fdesc *desc = ptr; + struct fdesc *desc = (struct fdesc *)ptr; void *p; if (!probe_kernel_address(&desc->ip, p)) - ptr = p; + ptr = (unsigned long)p; return ptr; } diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h index 9d13c3507ad6..59fbe0067112 100644 --- a/arch/parisc/include/asm/sections.h +++ b/arch/parisc/include/asm/sections.h @@ -6,7 +6,7 @@ #ifdef CONFIG_64BIT #undef dereference_function_descriptor -void *dereference_function_descriptor(void *); +unsigned long dereference_function_descriptor(unsigned long); #endif #endif diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c index 30f92391a93e..d350aa913acc 100644 --- a/arch/parisc/kernel/process.c +++ b/arch/parisc/kernel/process.c @@ -267,13 +267,13 @@ get_wchan(struct task_struct *p) } #ifdef CONFIG_64BIT -void *dereference_function_descriptor(void *ptr) +unsigned long dereference_function_descriptor(unsigned long ptr) { - Elf64_Fdesc *desc = ptr; + Elf64_Fdesc *desc = (Elf64_Fdesc *)ptr; void *p; if (!probe_kernel_address(&desc->addr, p)) - ptr = p; + ptr = (unsigned long)p; return ptr; } #endif diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c index 1ca9a2b4239f..06e1b79e2946 100644 --- a/arch/parisc/mm/init.c +++ b/arch/parisc/mm/init.c @@ -389,10 +389,10 @@ static void __init setup_bootmem(void) static int __init parisc_text_address(unsigned long vaddr) { static unsigned long head_ptr __initdata; + unsigned long addr = (unsigned long)&parisc_kernel_start; if (!head_ptr) - head_ptr = PAGE_MASK & (unsigned long) - dereference_function_descriptor(&parisc_kernel_start); + head_ptr = PAGE_MASK & dereference_function_descriptor(addr); return core_kernel_text(vaddr) || vaddr == head_ptr; } diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h index 7902d6358854..67379b8945e8 100644 --- a/arch/powerpc/include/asm/sections.h +++ b/arch/powerpc/include/asm/sections.h @@ -66,13 +66,13 @@ static inline int overlaps_kvm_tmp(unsigned long start, unsigned long end) #ifdef PPC64_ELF_ABI_v1 #undef dereference_function_descriptor -static inline void *dereference_function_descriptor(void *ptr) +static inline unsigned long dereference_function_descriptor(unsigned long ptr) { - struct ppc64_opd_entry *desc = ptr; + struct ppc64_opd_entry *desc = (struct ppc64_opd_entry *)ptr; void *p; if (!probe_kernel_address(&desc->funcaddr, p)) - ptr = p; + ptr = (unsigned long)p; return ptr; } #endif /* PPC64_ELF_ABI_v1 */ diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c index fc7efedbc4be..6a5a159dfb75 100644 --- a/drivers/misc/kgdbts.c +++ b/drivers/misc/kgdbts.c @@ -225,7 +225,7 @@ static unsigned long lookup_addr(char *arg) addr = (unsigned long)_do_fork; else if (!strcmp(arg, "hw_break_val")) addr = (unsigned long)&hw_break_val; - addr = (unsigned long) dereference_function_descriptor((void *)addr); + addr = dereference_function_descriptor(addr); return addr; } diff --git a/init/main.c b/init/main.c index 83bdfa4750b1..ac56f7a4501f 100644 --- a/init/main.c +++ b/init/main.c @@ -764,7 +764,7 @@ static bool __init_or_module initcall_blacklisted(initcall_t fn) if (list_empty(&blacklisted_initcalls)) return false; - addr = (unsigned long) dereference_function_descriptor(fn); + addr = dereference_function_descriptor((unsigned long)fn); sprint_symbol_no_offset(fn_name, addr); /* diff --git a/kernel/extable.c b/kernel/extable.c index 9aa1cc41ecf7..e48d6ba4ce6c 100644 --- a/kernel/extable.c +++ b/kernel/extable.c @@ -167,7 +167,7 @@ int kernel_text_address(unsigned long addr) int func_ptr_is_kernel_text(void *ptr) { unsigned long addr; - addr = (unsigned long) dereference_function_descriptor(ptr); + addr = dereference_function_descriptor((unsigned long)ptr); if (core_kernel_text(addr)) return 1; return is_module_text_address(addr); diff --git a/kernel/module.c b/kernel/module.c index de66ec825992..ea77ab13bead 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1067,7 +1067,7 @@ EXPORT_SYMBOL(__symbol_put); void symbol_put_addr(void *addr) { struct module *modaddr; - unsigned long a = (unsigned long)dereference_function_descriptor(addr); + unsigned long a = dereference_function_descriptor((unsigned long)addr); if (core_kernel_text(a)) return; diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 86c3385b9eb3..bcd906a39010 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1723,7 +1723,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, switch (*fmt) { case 'F': case 'f': - ptr = dereference_function_descriptor(ptr); + ptr = (void *)dereference_function_descriptor((unsigned long)ptr); /* Fallthrough */ case 'S': case 's':