From: Nathan Chancellor <natechancellor@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: ndesaulniers@google.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] inet/connection_sock: prefer _THIS_IP_ to current_text_addr
Date: Wed, 1 Aug 2018 22:08:18 -0700 [thread overview]
Message-ID: <20180802050818.GA18742@flashbox> (raw)
In-Reply-To: <20180802045825.GA16769@flashbox>
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
next prev parent reply other threads:[~2018-08-02 5:08 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-01 21:57 [PATCH] inet/connection_sock: prefer _THIS_IP_ to current_text_addr Nick Desaulniers
2018-08-02 1:42 ` David Miller
2018-08-02 4:58 ` Nathan Chancellor
2018-08-02 5:08 ` Nathan Chancellor [this message]
2018-08-02 17:07 ` Nick Desaulniers
2018-08-02 21:42 ` David Miller
2018-08-02 22:10 ` Nick Desaulniers
2018-08-02 22:17 ` David Miller
2018-08-02 22:29 ` Nick Desaulniers
2018-08-13 21:10 ` Nick Desaulniers
2018-08-14 17:04 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180802050818.GA18742@flashbox \
--to=natechancellor@gmail.com \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=ndesaulniers@google.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).