2008-11-23 16:57:25

by Cyrill Gorcunov

[permalink] [raw]
Subject: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

It's usefull to catch unbalanced, missed or mixed declarations of ENTRY and
KPROBES. These macros would help a bit (at least I hope so).

For example the following code would compile without problems

ENTRY_X86(mcount)
retq
END_X86(mcount)

But if you forget and mix the following form

ENTRY_X86(mcount)
retq
END(mcount)

ENTRY_X86(ftrace_caller)

The assembler will issue the following message:
Error: ENTRY_X86/KPROBE_X86 unbalanced,missed,mixed

Actually the checking is performed at every _X86 macro
so maybe it's good idea to put ENTRY_KPROBE_FINAL_X86
at the end of .S file to be sure you didn't miss anything.

Signed-off-by: Cyrill Gorcunov <[email protected]>
---
arch/x86/include/asm/linkage.h | 60 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)

Index: linux-2.6.git/arch/x86/include/asm/linkage.h
===================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/linkage.h
+++ linux-2.6.git/arch/x86/include/asm/linkage.h
@@ -57,5 +57,65 @@
#define __ALIGN_STR ".align 16,0x90"
#endif

+/*
+ * to check ENTRY_X86/END_X86 and
+ * KPROBE_ENTRY_X86/KPROBE_END_X86
+ * unbalanced-missed-mixed appearance
+ */
+#define __set_entry_x86 .set ENTRY_X86_IN, 0
+#define __unset_entry_x86 .set ENTRY_X86_IN, 1
+#define __set_kprobe_x86 .set KPROBE_X86_IN, 0
+#define __unset_kprobe_x86 .set KPROBE_X86_IN, 1
+
+#define __macro_err_x86 .error "ENTRY_X86/KPROBE_X86 unbalanced,missed,mixed"
+
+#define __check_entry_x86 \
+ .ifdef ENTRY_X86_IN; \
+ .ifeq ENTRY_X86_IN; \
+ __macro_err_x86; \
+ .abort; \
+ .endif; \
+ .endif
+
+#define __check_kprobe_x86 \
+ .ifdef KPROBE_X86_IN; \
+ .ifeq KPROBE_X86_IN; \
+ __x86_macro_err; \
+ .abort; \
+ .endif; \
+ .endif
+
+#define __check_entry_kprobe_x86 \
+ __check_entry_x86; \
+ __check_kprobe_x86
+
+#define ENTRY_KPROBE_FINAL_X86 __check_entry_kprobe_x86
+
+#define ENTRY_X86(name) \
+ __check_entry_kprobe_x86; \
+ __set_entry_x86; \
+ .globl name; \
+ __ALIGN; \
+ name:
+
+#define END_X86(name) \
+ __unset_entry_x86; \
+ __check_entry_kprobe_x86; \
+ .size name, .-name
+
+#define KPROBE_ENTRY_X86(name) \
+ __check_entry_kprobe_x86; \
+ __set_kprobe_x86; \
+ .pushsection .kprobes.text, "ax"; \
+ .globl name; \
+ __ALIGN; \
+ name:
+
+#define KPROBE_END_X86(name) \
+ __unset_kprobe_x86; \
+ __check_entry_kprobe_x86; \
+ .size name, .-name; \
+ .popsection
+
#endif /* _ASM_X86_LINKAGE_H */


2008-11-23 17:00:27

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Cyrill Gorcunov - Sun, Nov 23, 2008 at 07:57:11PM +0300]
| It's usefull to catch unbalanced, missed or mixed declarations of ENTRY and
| KPROBES. These macros would help a bit (at least I hope so).
|
...

Ingo, don't apply it -- found a nit. Will fix.

- Cyrill -

2008-11-23 17:54:57

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

On Sun, Nov 23, 2008 at 07:57:11PM +0300, Cyrill Gorcunov wrote:
> It's usefull to catch unbalanced, missed or mixed declarations of ENTRY and
> KPROBES. These macros would help a bit (at least I hope so).
>
> For example the following code would compile without problems
>
> ENTRY_X86(mcount)
> retq
> END_X86(mcount)
>
> But if you forget and mix the following form
>
> ENTRY_X86(mcount)
> retq
> END(mcount)
>
> ENTRY_X86(ftrace_caller)
>
> The assembler will issue the following message:
> Error: ENTRY_X86/KPROBE_X86 unbalanced,missed,mixed
>
> Actually the checking is performed at every _X86 macro
> so maybe it's good idea to put ENTRY_KPROBE_FINAL_X86
> at the end of .S file to be sure you didn't miss anything.

Could we at least try this out in -next before we decide to make
this X86 only?
I am aware that binutils can be a bit fragile but -next testing should
make a good check on this.

Sam

2008-11-23 17:58:56

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Sam Ravnborg - Sun, Nov 23, 2008 at 06:51:25PM +0100]
| On Sun, Nov 23, 2008 at 07:57:11PM +0300, Cyrill Gorcunov wrote:
| > It's usefull to catch unbalanced, missed or mixed declarations of ENTRY and
| > KPROBES. These macros would help a bit (at least I hope so).
| >
| > For example the following code would compile without problems
| >
| > ENTRY_X86(mcount)
| > retq
| > END_X86(mcount)
| >
| > But if you forget and mix the following form
| >
| > ENTRY_X86(mcount)
| > retq
| > END(mcount)
| >
| > ENTRY_X86(ftrace_caller)
| >
| > The assembler will issue the following message:
| > Error: ENTRY_X86/KPROBE_X86 unbalanced,missed,mixed
| >
| > Actually the checking is performed at every _X86 macro
| > so maybe it's good idea to put ENTRY_KPROBE_FINAL_X86
| > at the end of .S file to be sure you didn't miss anything.
|
| Could we at least try this out in -next before we decide to make
| this X86 only?
| I am aware that binutils can be a bit fragile but -next testing should
| make a good check on this.
|
| Sam
|

I don't have -next tree on my laptop, neither cross-compile tools but
if someone could test it -- it would be great. But I used gas macros
here -- i doubt other architectures has the same syntax. At least
PDP-11 would beat us with ';' symbol :)

- Cyrill -

2008-11-23 18:10:23

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Cyrill Gorcunov - Sun, Nov 23, 2008 at 08:58:46PM +0300]
| [Sam Ravnborg - Sun, Nov 23, 2008 at 06:51:25PM +0100]
| | On Sun, Nov 23, 2008 at 07:57:11PM +0300, Cyrill Gorcunov wrote:
| | > It's usefull to catch unbalanced, missed or mixed declarations of ENTRY and
| | > KPROBES. These macros would help a bit (at least I hope so).
| | >
| | > For example the following code would compile without problems
| | >
| | > ENTRY_X86(mcount)
| | > retq
| | > END_X86(mcount)
| | >
| | > But if you forget and mix the following form
| | >
| | > ENTRY_X86(mcount)
| | > retq
| | > END(mcount)
| | >
| | > ENTRY_X86(ftrace_caller)
| | >
| | > The assembler will issue the following message:
| | > Error: ENTRY_X86/KPROBE_X86 unbalanced,missed,mixed
| | >
| | > Actually the checking is performed at every _X86 macro
| | > so maybe it's good idea to put ENTRY_KPROBE_FINAL_X86
| | > at the end of .S file to be sure you didn't miss anything.
| |
| | Could we at least try this out in -next before we decide to make
| | this X86 only?
| | I am aware that binutils can be a bit fragile but -next testing should
| | make a good check on this.
| |
| | Sam
| |
|
| I don't have -next tree on my laptop, neither cross-compile tools but
| if someone could test it -- it would be great. But I used gas macros
| here -- i doubt other architectures has the same syntax. At least
| PDP-11 would beat us with ';' symbol :)
|
| - Cyrill -

On the other hand, if this feature show 'good' behaviour on x86 we could
propagate it on other arch's. If we just turn it on by default -- lots of
errors will be.

- Cyrill -

2008-11-23 18:11:35

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

On Sun, Nov 23, 2008 at 08:58:46PM +0300, Cyrill Gorcunov wrote:
> [Sam Ravnborg - Sun, Nov 23, 2008 at 06:51:25PM +0100]
> | On Sun, Nov 23, 2008 at 07:57:11PM +0300, Cyrill Gorcunov wrote:
> | > It's usefull to catch unbalanced, missed or mixed declarations of ENTRY and
> | > KPROBES. These macros would help a bit (at least I hope so).
> | >
> | > For example the following code would compile without problems
> | >
> | > ENTRY_X86(mcount)
> | > retq
> | > END_X86(mcount)
> | >
> | > But if you forget and mix the following form
> | >
> | > ENTRY_X86(mcount)
> | > retq
> | > END(mcount)
> | >
> | > ENTRY_X86(ftrace_caller)
> | >
> | > The assembler will issue the following message:
> | > Error: ENTRY_X86/KPROBE_X86 unbalanced,missed,mixed
> | >
> | > Actually the checking is performed at every _X86 macro
> | > so maybe it's good idea to put ENTRY_KPROBE_FINAL_X86
> | > at the end of .S file to be sure you didn't miss anything.
> |
> | Could we at least try this out in -next before we decide to make
> | this X86 only?
> | I am aware that binutils can be a bit fragile but -next testing should
> | make a good check on this.
> |
> | Sam
> |
>
> I don't have -next tree on my laptop, neither cross-compile tools but
> if someone could test it -- it would be great. But I used gas macros
> here -- i doubt other architectures has the same syntax. At least
> PDP-11 would beat us with ';' symbol :)

If we include this in any of the 100+ trees that Stephen sucks
into -next we will get it tried out.

Ingo has so and so does others so getting it into -next
is rather easy. Then the automated builds will tell of if
it fails on any of the toolchains used there.

Sam

2008-11-23 18:21:20

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
...
| >
| > I don't have -next tree on my laptop, neither cross-compile tools but
| > if someone could test it -- it would be great. But I used gas macros
| > here -- i doubt other architectures has the same syntax. At least
| > PDP-11 would beat us with ';' symbol :)
|
| If we include this in any of the 100+ trees that Stephen sucks
| into -next we will get it tried out.
|
| Ingo has so and so does others so getting it into -next
| is rather easy. Then the automated builds will tell of if
| it fails on any of the toolchains used there.
|
| Sam
|

Sam, to be clear, you mean that I could put this stuff into general
include/linux/linkage.h with general names as ENTRY/END and the same
for KPROBE so it could be merged into -next tree for testing? If yes,
that as I said there will be a lot of errors so build will stuck in
a moment 'cause of unbalanced ENTRY. Not sure if it's a good idea :)

- Cyrill -

2008-11-23 18:43:56

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Cyrill Gorcunov - Sun, Nov 23, 2008 at 09:21:03PM +0300]
| [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
| ...
| | >
| | > I don't have -next tree on my laptop, neither cross-compile tools but
| | > if someone could test it -- it would be great. But I used gas macros
| | > here -- i doubt other architectures has the same syntax. At least
| | > PDP-11 would beat us with ';' symbol :)
| |
| | If we include this in any of the 100+ trees that Stephen sucks
| | into -next we will get it tried out.
| |
| | Ingo has so and so does others so getting it into -next
| | is rather easy. Then the automated builds will tell of if
| | it fails on any of the toolchains used there.
| |
| | Sam
| |
|
| Sam, to be clear, you mean that I could put this stuff into general
| include/linux/linkage.h with general names as ENTRY/END and the same
| for KPROBE so it could be merged into -next tree for testing? If yes,
| that as I said there will be a lot of errors so build will stuck in
| a moment 'cause of unbalanced ENTRY. Not sure if it's a good idea :)
|
| - Cyrill -

Did I understand you right? It's highly possible I could translate
message the wrong way somime.

- Cyrill -

Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

On Sun, Nov 23, 2008 at 09:21:03PM +0300, Cyrill Gorcunov wrote:
> [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
> ...
> | >
> | > I don't have -next tree on my laptop, neither cross-compile tools but
> | > if someone could test it -- it would be great. But I used gas macros
> | > here -- i doubt other architectures has the same syntax. At least
> | > PDP-11 would beat us with ';' symbol :)
> |
> | If we include this in any of the 100+ trees that Stephen sucks
> | into -next we will get it tried out.
> |
> | Ingo has so and so does others so getting it into -next
> | is rather easy. Then the automated builds will tell of if
> | it fails on any of the toolchains used there.
> |
> | Sam
> |
>
> Sam, to be clear, you mean that I could put this stuff into general
> include/linux/linkage.h with general names as ENTRY/END and the same
> for KPROBE so it could be merged into -next tree for testing? If yes,
> that as I said there will be a lot of errors so build will stuck in
> a moment 'cause of unbalanced ENTRY. Not sure if it's a good idea :)

Hi Cyrill,

Aborting might be a bit to heavyhanded for -next. Maybe you should
prepare a version that only gives a warning for the first problem
it encounters per file? That would be fine in -next.

Greetings,
Alexander

> - Cyrill -

2008-11-23 18:54:42

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration


* Cyrill Gorcunov <[email protected]> wrote:

> [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
> ...
> | >
> | > I don't have -next tree on my laptop, neither cross-compile tools but
> | > if someone could test it -- it would be great. But I used gas macros
> | > here -- i doubt other architectures has the same syntax. At least
> | > PDP-11 would beat us with ';' symbol :)
> |
> | If we include this in any of the 100+ trees that Stephen sucks
> | into -next we will get it tried out.
> |
> | Ingo has so and so does others so getting it into -next
> | is rather easy. Then the automated builds will tell of if
> | it fails on any of the toolchains used there.
> |
> | Sam
> |
>
> Sam, to be clear, you mean that I could put this stuff into general
> include/linux/linkage.h with general names as ENTRY/END and the same
> for KPROBE so it could be merged into -next tree for testing? If
> yes, that as I said there will be a lot of errors so build will
> stuck in a moment 'cause of unbalanced ENTRY. Not sure if it's a
> good idea :)

neither do i think it's a particularly good idea. Lets first prototype
it on x86, see how it works out in practice, and then see whether it
can be generic. Then it can just be lifted into the generic linkage.h
separately, and we can then see whether it causes new problems.

Ingo

2008-11-23 18:57:36

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Ingo Molnar - Sun, Nov 23, 2008 at 07:54:17PM +0100]
|
| * Cyrill Gorcunov <[email protected]> wrote:
|
| > [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
| > ...
| > | >
| > | > I don't have -next tree on my laptop, neither cross-compile tools but
| > | > if someone could test it -- it would be great. But I used gas macros
| > | > here -- i doubt other architectures has the same syntax. At least
| > | > PDP-11 would beat us with ';' symbol :)
| > |
| > | If we include this in any of the 100+ trees that Stephen sucks
| > | into -next we will get it tried out.
| > |
| > | Ingo has so and so does others so getting it into -next
| > | is rather easy. Then the automated builds will tell of if
| > | it fails on any of the toolchains used there.
| > |
| > | Sam
| > |
| >
| > Sam, to be clear, you mean that I could put this stuff into general
| > include/linux/linkage.h with general names as ENTRY/END and the same
| > for KPROBE so it could be merged into -next tree for testing? If
| > yes, that as I said there will be a lot of errors so build will
| > stuck in a moment 'cause of unbalanced ENTRY. Not sure if it's a
| > good idea :)
|
| neither do i think it's a particularly good idea. Lets first prototype
| it on x86, see how it works out in practice, and then see whether it
| can be generic. Then it can just be lifted into the generic linkage.h
| separately, and we can then see whether it causes new problems.
|
| Ingo
|

So be it :) Btw I think Alexander is right -- better to use .warning
instead of .error (and without .abort) even on x86. Could you update
Ingo?

- Cyrill -

2008-11-23 19:00:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration


* Cyrill Gorcunov <[email protected]> wrote:

> [Ingo Molnar - Sun, Nov 23, 2008 at 07:54:17PM +0100]
> |
> | * Cyrill Gorcunov <[email protected]> wrote:
> |
> | > [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
> | > ...
> | > | >
> | > | > I don't have -next tree on my laptop, neither cross-compile tools but
> | > | > if someone could test it -- it would be great. But I used gas macros
> | > | > here -- i doubt other architectures has the same syntax. At least
> | > | > PDP-11 would beat us with ';' symbol :)
> | > |
> | > | If we include this in any of the 100+ trees that Stephen sucks
> | > | into -next we will get it tried out.
> | > |
> | > | Ingo has so and so does others so getting it into -next
> | > | is rather easy. Then the automated builds will tell of if
> | > | it fails on any of the toolchains used there.
> | > |
> | > | Sam
> | > |
> | >
> | > Sam, to be clear, you mean that I could put this stuff into general
> | > include/linux/linkage.h with general names as ENTRY/END and the same
> | > for KPROBE so it could be merged into -next tree for testing? If
> | > yes, that as I said there will be a lot of errors so build will
> | > stuck in a moment 'cause of unbalanced ENTRY. Not sure if it's a
> | > good idea :)
> |
> | neither do i think it's a particularly good idea. Lets first prototype
> | it on x86, see how it works out in practice, and then see whether it
> | can be generic. Then it can just be lifted into the generic linkage.h
> | separately, and we can then see whether it causes new problems.
> |
> | Ingo
> |
>
> So be it :) Btw I think Alexander is right -- better to use .warning
> instead of .error (and without .abort) even on x86. Could you update
> Ingo?

.error is perfectly fine because that way automated tests that we do
on -tip will catch any bugs, we really dont want to mis-annotate these
things. Warnings tend to only pile up and rarely get fixed - without
enforcement mechanism that causes people to fix them.

Ingo

2008-11-23 19:04:39

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Ingo Molnar - Sun, Nov 23, 2008 at 08:00:25PM +0100]
...
| >
| > So be it :) Btw I think Alexander is right -- better to use .warning
| > instead of .error (and without .abort) even on x86. Could you update
| > Ingo?
|
| .error is perfectly fine because that way automated tests that we do
| on -tip will catch any bugs, we really dont want to mis-annotate these
| things. Warnings tend to only pile up and rarely get fixed - without
| enforcement mechanism that causes people to fix them.
|
| Ingo
|

ok, let me try to play with this feature a bit. Will report the results.

- Cyrill -

2008-11-23 19:22:54

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Ingo Molnar - Sun, Nov 23, 2008 at 08:00:25PM +0100]
|
| * Cyrill Gorcunov <[email protected]> wrote:
|
| > [Ingo Molnar - Sun, Nov 23, 2008 at 07:54:17PM +0100]
| > |
| > | * Cyrill Gorcunov <[email protected]> wrote:
| > |
| > | > [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
| > | > ...
| > | > | >
| > | > | > I don't have -next tree on my laptop, neither cross-compile tools but
| > | > | > if someone could test it -- it would be great. But I used gas macros
| > | > | > here -- i doubt other architectures has the same syntax. At least
| > | > | > PDP-11 would beat us with ';' symbol :)
| > | > |
| > | > | If we include this in any of the 100+ trees that Stephen sucks
| > | > | into -next we will get it tried out.
| > | > |
| > | > | Ingo has so and so does others so getting it into -next
| > | > | is rather easy. Then the automated builds will tell of if
| > | > | it fails on any of the toolchains used there.
| > | > |
| > | > | Sam
| > | > |
| > | >
| > | > Sam, to be clear, you mean that I could put this stuff into general
| > | > include/linux/linkage.h with general names as ENTRY/END and the same
| > | > for KPROBE so it could be merged into -next tree for testing? If
| > | > yes, that as I said there will be a lot of errors so build will
| > | > stuck in a moment 'cause of unbalanced ENTRY. Not sure if it's a
| > | > good idea :)
| > |
| > | neither do i think it's a particularly good idea. Lets first prototype
| > | it on x86, see how it works out in practice, and then see whether it
| > | can be generic. Then it can just be lifted into the generic linkage.h
| > | separately, and we can then see whether it causes new problems.
| > |
| > | Ingo
| > |
| >
| > So be it :) Btw I think Alexander is right -- better to use .warning
| > instead of .error (and without .abort) even on x86. Could you update
| > Ingo?
|
| .error is perfectly fine because that way automated tests that we do
| on -tip will catch any bugs, we really dont want to mis-annotate these
| things. Warnings tend to only pile up and rarely get fixed - without
| enforcement mechanism that causes people to fix them.
|
| Ingo
|

Just got an error in implementation -- we have to support nested
ENTRY without problem. Will check. What a surprise :-)

- Cyrill -

2008-11-23 19:31:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration


* Cyrill Gorcunov <[email protected]> wrote:

> [Ingo Molnar - Sun, Nov 23, 2008 at 08:00:25PM +0100]
> |
> | * Cyrill Gorcunov <[email protected]> wrote:
> |
> | > [Ingo Molnar - Sun, Nov 23, 2008 at 07:54:17PM +0100]
> | > |
> | > | * Cyrill Gorcunov <[email protected]> wrote:
> | > |
> | > | > [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
> | > | > ...
> | > | > | >
> | > | > | > I don't have -next tree on my laptop, neither cross-compile tools but
> | > | > | > if someone could test it -- it would be great. But I used gas macros
> | > | > | > here -- i doubt other architectures has the same syntax. At least
> | > | > | > PDP-11 would beat us with ';' symbol :)
> | > | > |
> | > | > | If we include this in any of the 100+ trees that Stephen sucks
> | > | > | into -next we will get it tried out.
> | > | > |
> | > | > | Ingo has so and so does others so getting it into -next
> | > | > | is rather easy. Then the automated builds will tell of if
> | > | > | it fails on any of the toolchains used there.
> | > | > |
> | > | > | Sam
> | > | > |
> | > | >
> | > | > Sam, to be clear, you mean that I could put this stuff into general
> | > | > include/linux/linkage.h with general names as ENTRY/END and the same
> | > | > for KPROBE so it could be merged into -next tree for testing? If
> | > | > yes, that as I said there will be a lot of errors so build will
> | > | > stuck in a moment 'cause of unbalanced ENTRY. Not sure if it's a
> | > | > good idea :)
> | > |
> | > | neither do i think it's a particularly good idea. Lets first prototype
> | > | it on x86, see how it works out in practice, and then see whether it
> | > | can be generic. Then it can just be lifted into the generic linkage.h
> | > | separately, and we can then see whether it causes new problems.
> | > |
> | > | Ingo
> | > |
> | >
> | > So be it :) Btw I think Alexander is right -- better to use .warning
> | > instead of .error (and without .abort) even on x86. Could you update
> | > Ingo?
> |
> | .error is perfectly fine because that way automated tests that we do
> | on -tip will catch any bugs, we really dont want to mis-annotate these
> | things. Warnings tend to only pile up and rarely get fixed - without
> | enforcement mechanism that causes people to fix them.
> |
> | Ingo
> |
>
> Just got an error in implementation -- we have to support nested
> ENTRY without problem. Will check. What a surprise :-)

do you mean:

ENTRY(system_call)
ENTRY(system_call_after_swapgs)
...
END(system_call)

that's more of a bug - system_call_after_swapgs is not a real entry
point, we just need the label of it. Perhaps something like __ENTRY()
for that case would be enough.

nor is this one real:

ENTRY(interrupt)
ENTRY(irq_entries_start)
...
END(irq_entries_start)
END(interrupt)

do we really need .irq_entries_start?

I think in general we should define a flat hierarchy of entries.

Ingo

2008-11-23 19:48:39

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

[Ingo Molnar - Sun, Nov 23, 2008 at 08:31:34PM +0100]
...
| >
| > Just got an error in implementation -- we have to support nested
| > ENTRY without problem. Will check. What a surprise :-)
|
| do you mean:
|
| ENTRY(system_call)
| ENTRY(system_call_after_swapgs)
| ...
| END(system_call)
|
| that's more of a bug - system_call_after_swapgs is not a real entry
| point, we just need the label of it. Perhaps something like __ENTRY()
| for that case would be enough.
|
| nor is this one real:
|
| ENTRY(interrupt)
| ENTRY(irq_entries_start)
| ...
| END(irq_entries_start)
| END(interrupt)
|
| do we really need .irq_entries_start?
|
| I think in general we should define a flat hierarchy of entries.
|
| Ingo
|

Yeah, I meant these cases. I don't think we really need irq_entries_start
(didn't find any mention of them in tree). In case of system_call_after_swapgs
I'm not that sure, but since xen use it as a plain jmp (at least now) it
could be converted to a plain label. Ingo, I'll continue tomorrow evening --
have some other things to be done :)

- Cyrill -

2008-11-23 19:50:07

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

On Sun, Nov 23, 2008 at 07:54:17PM +0100, Ingo Molnar wrote:
>
> * Cyrill Gorcunov <[email protected]> wrote:
>
> > [Sam Ravnborg - Sun, Nov 23, 2008 at 07:12:48PM +0100]
> > ...
> > | >
> > | > I don't have -next tree on my laptop, neither cross-compile tools but
> > | > if someone could test it -- it would be great. But I used gas macros
> > | > here -- i doubt other architectures has the same syntax. At least
> > | > PDP-11 would beat us with ';' symbol :)
> > |
> > | If we include this in any of the 100+ trees that Stephen sucks
> > | into -next we will get it tried out.
> > |
> > | Ingo has so and so does others so getting it into -next
> > | is rather easy. Then the automated builds will tell of if
> > | it fails on any of the toolchains used there.
> > |
> > | Sam
> > |
> >
> > Sam, to be clear, you mean that I could put this stuff into general
> > include/linux/linkage.h with general names as ENTRY/END and the same
> > for KPROBE so it could be merged into -next tree for testing? If
> > yes, that as I said there will be a lot of errors so build will
> > stuck in a moment 'cause of unbalanced ENTRY. Not sure if it's a
> > good idea :)
>
> neither do i think it's a particularly good idea. Lets first prototype
> it on x86, see how it works out in practice, and then see whether it
> can be generic. Then it can just be lifted into the generic linkage.h
> separately, and we can then see whether it causes new problems.

OK - I assume you guys will take action on this if we succeed in x86
with this nice build time check.

Sam

2008-11-23 22:36:17

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration


* Cyrill Gorcunov <[email protected]> wrote:

> [Ingo Molnar - Sun, Nov 23, 2008 at 08:31:34PM +0100]
> ...
> | >
> | > Just got an error in implementation -- we have to support nested
> | > ENTRY without problem. Will check. What a surprise :-)
> |
> | do you mean:
> |
> | ENTRY(system_call)
> | ENTRY(system_call_after_swapgs)
> | ...
> | END(system_call)
> |
> | that's more of a bug - system_call_after_swapgs is not a real entry
> | point, we just need the label of it. Perhaps something like __ENTRY()
> | for that case would be enough.
> |
> | nor is this one real:
> |
> | ENTRY(interrupt)
> | ENTRY(irq_entries_start)
> | ...
> | END(irq_entries_start)
> | END(interrupt)
> |
> | do we really need .irq_entries_start?
> |
> | I think in general we should define a flat hierarchy of entries.
> |
> | Ingo
> |
>
> Yeah, I meant these cases. I don't think we really need
> irq_entries_start (didn't find any mention of them in tree). In case
> of system_call_after_swapgs I'm not that sure, but since xen use it
> as a plain jmp (at least now) it could be converted to a plain
> label. [...]

system_call_after_swapgs is a slowpath and should be converted to a
simple:

.globl system_call_after_swapgs
system_call_after_swapgs:

symbol definition - with no particular jump target alignment tweaks.

(the above sequence should be generalized as an __ENTRY() macro - i.e.
raw global symbol definition without any alignment tweaks)

Ingo

Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

On Sun, Nov 23, 2008 at 10:48:28PM +0300, Cyrill Gorcunov wrote:
> [Ingo Molnar - Sun, Nov 23, 2008 at 08:31:34PM +0100]
> ...
> | >
> | > Just got an error in implementation -- we have to support nested
> | > ENTRY without problem. Will check. What a surprise :-)
> |
> | do you mean:
> |
> | ENTRY(system_call)
> | ENTRY(system_call_after_swapgs)
> | ...
> | END(system_call)
> |
> | that's more of a bug - system_call_after_swapgs is not a real entry
> | point, we just need the label of it. Perhaps something like __ENTRY()
> | for that case would be enough.
> |
> | nor is this one real:
> |
> | ENTRY(interrupt)
> | ENTRY(irq_entries_start)
> | ...
> | END(irq_entries_start)
> | END(interrupt)
> |
> | do we really need .irq_entries_start?
> |
> | I think in general we should define a flat hierarchy of entries.
> |
> | Ingo
> |
>
> Yeah, I meant these cases. I don't think we really need irq_entries_start
> (didn't find any mention of them in tree). In case of system_call_after_swapgs
> I'm not that sure, but since xen use it as a plain jmp (at least now) it
> could be converted to a plain label. Ingo, I'll continue tomorrow evening --
> have some other things to be done :)

The problem is that ENTRY(interrupt) is done in init.rodata, and
ENTRY(irq_entries_start) is done in .text. So inside the .S-file,
they are nested, but in the .o-file they are separate. Instead of
removing ENTRY(irq_entries_start), I think we should just expand to:


.section .init.rodata,"a"
.p2align 5
.global interrupt
interrupt:

and

size interrupt, .-interrupt

But the only importance I can think of is that this keeps both
the "interrupt" array and irq_entries_start visible in debugging
information.

Alternatively, we could probably do away with the interrupt
array entirely. We _know_ how the irq stubs are structured and
irq_entries_start is in principle enough information to reconstruct
all information in interrupt.

Anyhow, I too think that supporting nested ENTRY/END in .S files is
unnecessary.

Alexander

> - Cyrill -

2008-11-24 18:05:30

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [RFC -tip] x86: introduce ENTRY(KPROBE)_X86 assembly helpers to catch unbalanced declaration

Alexander van Heukelum wrote:
>
> The problem is that ENTRY(interrupt) is done in init.rodata, and
> ENTRY(irq_entries_start) is done in .text. So inside the .S-file,
> they are nested, but in the .o-file they are separate. Instead of
> removing ENTRY(irq_entries_start), I think we should just expand to:
>
> .section .init.rodata,"a"
> .p2align 5
> .global interrupt
> interrupt:
>
> and
>
> size interrupt, .-interrupt
>
> But the only importance I can think of is that this keeps both
> the "interrupt" array and irq_entries_start visible in debugging
> information.
>
> Alternatively, we could probably do away with the interrupt
> array entirely. We _know_ how the irq stubs are structured and
> irq_entries_start is in principle enough information to reconstruct
> all information in interrupt.
>

I'd rather not get rid of the interrupt array. But more fundamentally,
interrupt is a data symbol, not an entry point.

-hpa