Messages in this thread | | | Date | Wed, 1 Aug 2018 22:08:18 -0700 | From | Nathan Chancellor <> | Subject | Re: [PATCH] inet/connection_sock: prefer _THIS_IP_ to current_text_addr |
| |
On Wed, Aug 01, 2018 at 09:58:25PM -0700, Nathan Chancellor wrote: > On Wed, Aug 01, 2018 at 06:42:08PM -0700, David Miller wrote: > > From: Nick Desaulniers <ndesaulniers@google.com> > > Date: Wed, 1 Aug 2018 14:57:59 -0700 > > > > > - sk, what, when, current_text_addr()); > > > + sk, what, when, (void *)_THIS_IP_); > > > > Is a fugly macro in all caps really better than a function()? > > > > I'm really surprised that _THIS_IP_ is being chosen over > > current_text_addr(), seriously. > > Hi Dave,
Sorry for the second email so quick after the first, needed to clarify two things.
> > current_text_addr is only used in five places in the entire kernel and > this is the only non-architecture specific use. > > include/net/inet_connection_sock.h:227: sk, what, when, current_text_addr()); > arch/sh/include/asm/kexec.h:64: newregs->pc = (unsigned long)current_text_addr(); > arch/sh/kernel/dwarf.c:602: pc = (unsigned long)current_text_addr(); > arch/x86/include/asm/kexec.h:135: newregs->ip = (unsigned long)current_text_addr(); > arch/parisc/kernel/unwind.c:442: r.iaoq[0] = (unsigned long) current_text_addr(); > > We're trying to merge it with _THIS_IP_ before for most architectures,
because for most...
> it is defined nearly identically to _THIS_IP_: > > #define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; }) > > #define current_text_addr() ({ __label__ _l; _l: &&_l; }) > > * arc > * arm > * arm64 > * h8300 > * hexagon > * m68k > * mips > * nds32 > * nios2 > * openrisc > * powerpc > * riscv > * unicore32 > * xtensa > > Ultimately, we're trying to turn off a new Clang warning in _THIS_IP_ > which requires some changes to work around what is believed to be a GCC > bug (see https://lore.kernel.org/patchwork/patch/969101/).
and I should have mentioned that current_text_addr suffers from the exact same issue so we're trying not to duplicate it 14 times across the kernel.
> I guess the alternative to this patch is to just define current_text_addr > as _THIS_IP_ for all of those architectures, I am not sure how that > filters into Nick's plan (I think the goal of this was to try and avoid > getting all of the architecture folks involved). > > Thanks for your time! > Nathan
| |