2018-07-31 22:33:50

by Nick Desaulniers

[permalink] [raw]
Subject: Getting the instruction pointer on a per arch basis

I'm currently looking into cleaning up the code duplication between
current_text_addr() and _THIS_IP_, virtually every implementation of
current_text_addr() and _THIS_IP_ itself are basically:

#define _THIS_IP_ ({ __label__ _l; _l: &&_l; })

For a few arch's, they have inline assembly instead (for
current_text_addr()). Examples:
* s390
* sh
* ia64
* x86 (um and 32b)
* c6x
* sparc

I have a patch that cuts down on the duplication, but I don't
understand why the few arch specific implementations are necessary. I
could reduce the duplication further if it's ok to just use the
statement expression.

Does anyone know why this is the case?
--
Thanks,
~Nick Desaulniers


2018-07-31 23:11:05

by Nick Desaulniers

[permalink] [raw]
Subject: Re: Getting the instruction pointer on a per arch basis

+ More maintainers and lists for visibility

On Tue, Jul 31, 2018 at 3:32 PM Nick Desaulniers
<[email protected]> wrote:
>
> I'm currently looking into cleaning up the code duplication between
> current_text_addr() and _THIS_IP_, virtually every implementation of
> current_text_addr() and _THIS_IP_ itself are basically:
>
> #define _THIS_IP_ ({ __label__ _l; _l: &&_l; })
>
> For a few arch's, they have inline assembly instead (for
> current_text_addr()). Examples:
> * s390
> * sh
> * ia64
> * x86 (um and 32b)
> * c6x
> * sparc
>
> I have a patch that cuts down on the duplication, but I don't
> understand why the few arch specific implementations are necessary. I
> could reduce the duplication further if it's ok to just use the
> statement expression.
>
> Does anyone know why this is the case?
> --
> Thanks,
> ~Nick Desaulniers

2018-08-01 01:34:43

by Randy Dunlap

[permalink] [raw]
Subject: Re: Getting the instruction pointer on a per arch basis

On 07/31/2018 04:09 PM, Nick Desaulniers wrote:
> + More maintainers and lists for visibility
>

Ideally the [email protected] mailing list would be sufficient,
but I doubt that it is. :(

--
~Randy

2018-08-01 05:42:41

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: Getting the instruction pointer on a per arch basis

On Tue, 31 Jul 2018 16:09:06 -0700
Nick Desaulniers <[email protected]> wrote:

> + More maintainers and lists for visibility
>
> On Tue, Jul 31, 2018 at 3:32 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > I'm currently looking into cleaning up the code duplication between
> > current_text_addr() and _THIS_IP_, virtually every implementation of
> > current_text_addr() and _THIS_IP_ itself are basically:
> >
> > #define _THIS_IP_ ({ __label__ _l; _l: &&_l; })
> >
> > For a few arch's, they have inline assembly instead (for
> > current_text_addr()). Examples:
> > * s390
> > * sh
> > * ia64
> > * x86 (um and 32b)
> > * c6x
> > * sparc
> >
> > I have a patch that cuts down on the duplication, but I don't
> > understand why the few arch specific implementations are necessary. I
> > could reduce the duplication further if it's ok to just use the
> > statement expression.
> >
> > Does anyone know why this is the case?

For s390 it is just that we did not know about the label trick when we
introduced the define. The inline has an advantage though, the code
generated with the label trick is using a LARL instruction which is
4 bytes, the inline assembly uses a BASR which is 2 bytes.

If I use the label method in current_text_addr() the size of vmlinux
increases by a small amount:

add/remove: 33/13 grow/shrink: 101/48 up/down: 11941/-8887 (3054)

This is acceptable though, I would not mind if _THIS_IP_ and
current_text_addr use a common definition using labels.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.


2018-08-01 18:23:01

by Nick Desaulniers

[permalink] [raw]
Subject: Re: Getting the instruction pointer on a per arch basis

On Tue, Jul 31, 2018 at 10:41 PM Martin Schwidefsky
<[email protected]> wrote:
>
> On Tue, 31 Jul 2018 16:09:06 -0700
> Nick Desaulniers <[email protected]> wrote:
>
> > + More maintainers and lists for visibility
> >
> > On Tue, Jul 31, 2018 at 3:32 PM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > I'm currently looking into cleaning up the code duplication between
> > > current_text_addr() and _THIS_IP_, virtually every implementation of
> > > current_text_addr() and _THIS_IP_ itself are basically:
> > >
> > > #define _THIS_IP_ ({ __label__ _l; _l: &&_l; })
> > >
> > > For a few arch's, they have inline assembly instead (for
> > > current_text_addr()). Examples:
> > > * s390
> > > * sh
> > > * ia64
> > > * x86 (um and 32b)
> > > * c6x
> > > * sparc
> > >
> > > I have a patch that cuts down on the duplication, but I don't
> > > understand why the few arch specific implementations are necessary. I
> > > could reduce the duplication further if it's ok to just use the
> > > statement expression.
> > >
> > > Does anyone know why this is the case?
>
> For s390 it is just that we did not know about the label trick when we
> introduced the define. The inline has an advantage though, the code
> generated with the label trick is using a LARL instruction which is
> 4 bytes, the inline assembly uses a BASR which is 2 bytes.
>
> If I use the label method in current_text_addr() the size of vmlinux
> increases by a small amount:
>
> add/remove: 33/13 grow/shrink: 101/48 up/down: 11941/-8887 (3054)

Thanks for the measurements. Was this output produced by a utility?

> This is acceptable though, I would not mind if _THIS_IP_ and
> current_text_addr use a common definition using labels.

Thank you for this feedback Martin, I appreciate it. Patches soon.

--
Thanks,
~Nick Desaulniers

2018-08-06 05:26:10

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: Getting the instruction pointer on a per arch basis

On Wed, 1 Aug 2018 11:21:13 -0700
Nick Desaulniers <[email protected]> wrote:

> On Tue, Jul 31, 2018 at 10:41 PM Martin Schwidefsky
> <[email protected]> wrote:
> >
> > On Tue, 31 Jul 2018 16:09:06 -0700
> > Nick Desaulniers <[email protected]> wrote:
> >
> > > + More maintainers and lists for visibility
> > >
> > > On Tue, Jul 31, 2018 at 3:32 PM Nick Desaulniers
> > > <[email protected]> wrote:
> > > >
> > > > I'm currently looking into cleaning up the code duplication between
> > > > current_text_addr() and _THIS_IP_, virtually every implementation of
> > > > current_text_addr() and _THIS_IP_ itself are basically:
> > > >
> > > > #define _THIS_IP_ ({ __label__ _l; _l: &&_l; })
> > > >
> > > > For a few arch's, they have inline assembly instead (for
> > > > current_text_addr()). Examples:
> > > > * s390
> > > > * sh
> > > > * ia64
> > > > * x86 (um and 32b)
> > > > * c6x
> > > > * sparc
> > > >
> > > > I have a patch that cuts down on the duplication, but I don't
> > > > understand why the few arch specific implementations are necessary. I
> > > > could reduce the duplication further if it's ok to just use the
> > > > statement expression.
> > > >
> > > > Does anyone know why this is the case?
> >
> > For s390 it is just that we did not know about the label trick when we
> > introduced the define. The inline has an advantage though, the code
> > generated with the label trick is using a LARL instruction which is
> > 4 bytes, the inline assembly uses a BASR which is 2 bytes.
> >
> > If I use the label method in current_text_addr() the size of vmlinux
> > increases by a small amount:
> >
> > add/remove: 33/13 grow/shrink: 101/48 up/down: 11941/-8887 (3054)
>
> Thanks for the measurements. Was this output produced by a utility?

Yes, ./scripts/bloat-o-meter

> > This is acceptable though, I would not mind if _THIS_IP_ and
> > current_text_addr use a common definition using labels.
>
> Thank you for this feedback Martin, I appreciate it. Patches soon.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.