Re: [PATCH] parisc: prefer _THIS_IP_ and _RET_IP_ statement expressions
From: Nick Desaulniers
Date: Tue Aug 07 2018 - 17:29:28 EST
On Tue, Aug 7, 2018 at 1:30 PM Helge Deller <deller@xxxxxx> wrote:
>
> On 07.08.2018 20:11, Nick Desaulniers wrote:
> > On Fri, Aug 3, 2018 at 3:34 PM Helge Deller <deller@xxxxxx> wrote:
> >> So, your patch is basically OK and doesn't break anything.
> >> But I agree with Dave and Andrew, that THIS_IP is ugly.
> >
> > I don't disagree, and other maintainers have remarked on _THIS_IP_
> > being ugly, but renaming it en masse is a tree wide change, which I'm
> > trying to avoid at the moment.
>
> Understandable.
>
> > It sounds like we have a working patch? Are there 64b parisc machines
> > to test on, or can this get merged?
>
> Go ahead and merge it.
Thank you, but I was under the impression this would go up through the
parisc tree?
https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/
right? Or is there a different tree/maintainer I should ask?
> In addition, somehow I'd prefer if there would be a way to add to include/linux/kernel.h:
> +#if !defined(_THIS_IP_)
> #define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
> +#endif
>
> That way it would somehow be possible to replace that label calulation by the
> preferable inline assembly of current_text_address()...
I'm asserting that there is no need within the entire kernel at the
moment to have inline assembly to get the instruction pointer. If
there are no call sites of the inline assembly version, there is no
need to define per arch inline assembly versions when C (with GNU
extensions) will suffice.
And if in the future there are, either those call sites can have a
local implementation (as in the second diff I sent in this thread), or
some other change to _THIS_IP_ (as you propose) can be made. But
until then...
YAGNI: "You Ain't Gonna Need It"
Once this patch and the other 3 outstanding ones are merged, we'll be
sending patches to delete all arch specific assembly implementations
as they will be dead code (no callers, kernel-wide).
--
Thanks,
~Nick Desaulniers