Re: [patch 1/3] move WARN_ON() out of line
Jeremy Fitzhardinge wrote: Yeah, that seems reasonable if you're optimising for overall size. Did you count the difference of including the function name? We decided not to include it for BUG because its usefulness/size tradeoff didn't seem terribly important. in the WARN_ON case it's not there either, based on Ingo's idea we do a kallsyms lookup of __builtin_return_address(0) .. same data, less memory. But my goal was actually to reduce icache pollution, so by my reckoning code bytes were much more expensive than data ones, so putting all BUG information in a separate section makes those bytes much less significant than putting anything inline in code. Also, the trap for WARN_ON would be smaller than BUG, because it wouldn't need the spurious infinite loop needed to make gcc understand the control flow of a BUG. On the other hand, you could put the call to out of line warning function in a separate section to achieve the same effect. yeah and gcc even has a compiler option for that. Doubt it's really worth it, we're still talking a few bytes here ;) J -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: > Jeremy Fitzhardinge wrote: >> Arjan van de Ven wrote: >>> This patch moves WARN_ON() out of line entirely. I've considered >>> keeping >>> the test inline and moving only the slowpath out of line, but I decided >>> against that: an out of line test reduces the pressure on the CPUs >>> branch predictor logic and gives smaller code, while a function call >>> to a fixed location is quite fast. Likewise I've considered doing >>> something >>> similar to BUG() (eg use a trapping instruction) but that's not really >>> better (it needs the test inline again and recovering from an invalid >>> instruction isn't quite fun). >> >> Power implements WARN_ON this way, and all the machinery is in place to >> generically implement WARN_ON that way if you want. It does generate >> denser code than the call (since its just a single trapping instruction >> with no need for argument setup), and the performance cost of the trap >> shouldn't matter if warnings are rare (which one would hope). > > I just did an experiment with this to see how much is on the table. I > made > a file with 1024 WARN_ON()'s (new style, eg the out of line call) and > 1024 BUG_ON()'s, > which on i386 already use the trap. > This shows that the BUG_ON() case is 2Kb shorter in generated code. > From this 2Kb you > need to subtract all the code size that is needed to deal with the > trap and the module > merging/unmerging of trap points etc etc, so lets say that a total of > 1Kb is left on the table. > HOWEVER, if you have a module with, say, only 4 WARN_ON()/BUG_ON()'s, > you actually LOOSE > 48 bytes, because of the extra overhead of how the trap data is stored. > > So... call me unconvinced for now. There's 30 Kb on the table with the > easy, obviously safe > transform, and maybe another 1Kb with the much more tricky trapping > scenario, but only > for the vmlinux case; the module case seems to be a loss instead. Yeah, that seems reasonable if you're optimising for overall size. Did you count the difference of including the function name? We decided not to include it for BUG because its usefulness/size tradeoff didn't seem terribly important. But my goal was actually to reduce icache pollution, so by my reckoning code bytes were much more expensive than data ones, so putting all BUG information in a separate section makes those bytes much less significant than putting anything inline in code. Also, the trap for WARN_ON would be smaller than BUG, because it wouldn't need the spurious infinite loop needed to make gcc understand the control flow of a BUG. On the other hand, you could put the call to out of line warning function in a separate section to achieve the same effect. J -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: So... call me unconvinced for now. There's 30 Kb on the table with the easy, obviously safe transform, and maybe another 1Kb with the much more tricky trapping scenario, but only for the vmlinux case; the module case seems to be a loss instead. Eh I have to retract my math here; I used a slightly older version of the WARN_ON patch series. (before Ingo's suggestion) In the new model, even at 1024 the out of line WARN_ON function call is smaller than the BUG_ON method. So I think that at least for x86, it's a loss to do what you suggest if people wonder where this comes from: the BUG_ON code sequence is 13 bytes, the WARN_ON sequence is 24 bytes, so 11 bytes longer. HOWEVER, the BUG_ON approach also needs 12 bytes of data (20 on 64 bit) per bug, a nett loss of 1 byte on 32 bit x86. (plus some general overhead for storing sections as such, but that scales per ELF file, not per BUG_ON instance) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Herbert Xu wrote: Arjan van de Ven <[EMAIL PROTECTED]> wrote: While we're here, I'll mention that dump_stack probably ought to take a severity level argument. 125 files changed, 202 insertions(+), 199 deletions(-) just to get the api change done. I can hear akpm cringe from here... You don't have to change all of them at once. Having 2 is just silly though, and it'll take a long time, if ever, to do this sort of thing slowly. Better to do it all at once. The amount of change doesn't get smaller if you spread it out; it's still a question if the total amount of change is worth the added functionality. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: Jeremy Fitzhardinge wrote: Arjan van de Ven wrote: This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Power implements WARN_ON this way, and all the machinery is in place to generically implement WARN_ON that way if you want. It does generate denser code than the call (since its just a single trapping instruction with no need for argument setup), and the performance cost of the trap shouldn't matter if warnings are rare (which one would hope). I just did an experiment with this to see how much is on the table. I made a file with 1024 WARN_ON()'s (new style, eg the out of line call) and 1024 BUG_ON()'s, which on i386 already use the trap. This shows that the BUG_ON() case is 2Kb shorter in generated code. From this 2Kb you need to subtract all the code size that is needed to deal with the trap and the module merging/unmerging of trap points etc etc, so lets say that a total of 1Kb is left on the table. HOWEVER, if you have a module with, say, only 4 WARN_ON()/BUG_ON()'s, you actually LOOSE 48 bytes, because of the extra overhead of how the trap data is stored. So... call me unconvinced for now. There's 30 Kb on the table with the easy, obviously safe transform, and maybe another 1Kb with the much more tricky trapping scenario, but only for the vmlinux case; the module case seems to be a loss instead. Eh I have to retract my math here; I used a slightly older version of the WARN_ON patch series. (before Ingo's suggestion) In the new model, even at 1024 the out of line WARN_ON function call is smaller than the BUG_ON method. So I think that at least for x86, it's a loss to do what you suggest -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Jeremy Fitzhardinge wrote: Arjan van de Ven wrote: This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Power implements WARN_ON this way, and all the machinery is in place to generically implement WARN_ON that way if you want. It does generate denser code than the call (since its just a single trapping instruction with no need for argument setup), and the performance cost of the trap shouldn't matter if warnings are rare (which one would hope). I just did an experiment with this to see how much is on the table. I made a file with 1024 WARN_ON()'s (new style, eg the out of line call) and 1024 BUG_ON()'s, which on i386 already use the trap. This shows that the BUG_ON() case is 2Kb shorter in generated code. From this 2Kb you need to subtract all the code size that is needed to deal with the trap and the module merging/unmerging of trap points etc etc, so lets say that a total of 1Kb is left on the table. HOWEVER, if you have a module with, say, only 4 WARN_ON()/BUG_ON()'s, you actually LOOSE 48 bytes, because of the extra overhead of how the trap data is stored. So... call me unconvinced for now. There's 30 Kb on the table with the easy, obviously safe transform, and maybe another 1Kb with the much more tricky trapping scenario, but only for the vmlinux case; the module case seems to be a loss instead. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: Jeremy Fitzhardinge wrote: Arjan van de Ven wrote: This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Power implements WARN_ON this way, and all the machinery is in place to generically implement WARN_ON that way if you want. It does generate denser code than the call (since its just a single trapping instruction with no need for argument setup), and the performance cost of the trap shouldn't matter if warnings are rare (which one would hope). I just did an experiment with this to see how much is on the table. I made a file with 1024 WARN_ON()'s (new style, eg the out of line call) and 1024 BUG_ON()'s, which on i386 already use the trap. This shows that the BUG_ON() case is 2Kb shorter in generated code. From this 2Kb you need to subtract all the code size that is needed to deal with the trap and the module merging/unmerging of trap points etc etc, so lets say that a total of 1Kb is left on the table. HOWEVER, if you have a module with, say, only 4 WARN_ON()/BUG_ON()'s, you actually LOOSE 48 bytes, because of the extra overhead of how the trap data is stored. So... call me unconvinced for now. There's 30 Kb on the table with the easy, obviously safe transform, and maybe another 1Kb with the much more tricky trapping scenario, but only for the vmlinux case; the module case seems to be a loss instead. Eh I have to retract my math here; I used a slightly older version of the WARN_ON patch series. (before Ingo's suggestion) In the new model, even at 1024 the out of line WARN_ON function call is smaller than the BUG_ON method. So I think that at least for x86, it's a loss to do what you suggest -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Jeremy Fitzhardinge wrote: Arjan van de Ven wrote: This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Power implements WARN_ON this way, and all the machinery is in place to generically implement WARN_ON that way if you want. It does generate denser code than the call (since its just a single trapping instruction with no need for argument setup), and the performance cost of the trap shouldn't matter if warnings are rare (which one would hope). I just did an experiment with this to see how much is on the table. I made a file with 1024 WARN_ON()'s (new style, eg the out of line call) and 1024 BUG_ON()'s, which on i386 already use the trap. This shows that the BUG_ON() case is 2Kb shorter in generated code. From this 2Kb you need to subtract all the code size that is needed to deal with the trap and the module merging/unmerging of trap points etc etc, so lets say that a total of 1Kb is left on the table. HOWEVER, if you have a module with, say, only 4 WARN_ON()/BUG_ON()'s, you actually LOOSE 48 bytes, because of the extra overhead of how the trap data is stored. So... call me unconvinced for now. There's 30 Kb on the table with the easy, obviously safe transform, and maybe another 1Kb with the much more tricky trapping scenario, but only for the vmlinux case; the module case seems to be a loss instead. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Herbert Xu wrote: Arjan van de Ven [EMAIL PROTECTED] wrote: While we're here, I'll mention that dump_stack probably ought to take a severity level argument. 125 files changed, 202 insertions(+), 199 deletions(-) just to get the api change done. I can hear akpm cringe from here... You don't have to change all of them at once. Having 2 is just silly though, and it'll take a long time, if ever, to do this sort of thing slowly. Better to do it all at once. The amount of change doesn't get smaller if you spread it out; it's still a question if the total amount of change is worth the added functionality. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: So... call me unconvinced for now. There's 30 Kb on the table with the easy, obviously safe transform, and maybe another 1Kb with the much more tricky trapping scenario, but only for the vmlinux case; the module case seems to be a loss instead. Eh I have to retract my math here; I used a slightly older version of the WARN_ON patch series. (before Ingo's suggestion) In the new model, even at 1024 the out of line WARN_ON function call is smaller than the BUG_ON method. So I think that at least for x86, it's a loss to do what you suggest if people wonder where this comes from: the BUG_ON code sequence is 13 bytes, the WARN_ON sequence is 24 bytes, so 11 bytes longer. HOWEVER, the BUG_ON approach also needs 12 bytes of data (20 on 64 bit) per bug, a nett loss of 1 byte on 32 bit x86. (plus some general overhead for storing sections as such, but that scales per ELF file, not per BUG_ON instance) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: Jeremy Fitzhardinge wrote: Arjan van de Ven wrote: This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Power implements WARN_ON this way, and all the machinery is in place to generically implement WARN_ON that way if you want. It does generate denser code than the call (since its just a single trapping instruction with no need for argument setup), and the performance cost of the trap shouldn't matter if warnings are rare (which one would hope). I just did an experiment with this to see how much is on the table. I made a file with 1024 WARN_ON()'s (new style, eg the out of line call) and 1024 BUG_ON()'s, which on i386 already use the trap. This shows that the BUG_ON() case is 2Kb shorter in generated code. From this 2Kb you need to subtract all the code size that is needed to deal with the trap and the module merging/unmerging of trap points etc etc, so lets say that a total of 1Kb is left on the table. HOWEVER, if you have a module with, say, only 4 WARN_ON()/BUG_ON()'s, you actually LOOSE 48 bytes, because of the extra overhead of how the trap data is stored. So... call me unconvinced for now. There's 30 Kb on the table with the easy, obviously safe transform, and maybe another 1Kb with the much more tricky trapping scenario, but only for the vmlinux case; the module case seems to be a loss instead. Yeah, that seems reasonable if you're optimising for overall size. Did you count the difference of including the function name? We decided not to include it for BUG because its usefulness/size tradeoff didn't seem terribly important. But my goal was actually to reduce icache pollution, so by my reckoning code bytes were much more expensive than data ones, so putting all BUG information in a separate section makes those bytes much less significant than putting anything inline in code. Also, the trap for WARN_ON would be smaller than BUG, because it wouldn't need the spurious infinite loop needed to make gcc understand the control flow of a BUG. On the other hand, you could put the call to out of line warning function in a separate section to achieve the same effect. J -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Jeremy Fitzhardinge wrote: Yeah, that seems reasonable if you're optimising for overall size. Did you count the difference of including the function name? We decided not to include it for BUG because its usefulness/size tradeoff didn't seem terribly important. in the WARN_ON case it's not there either, based on Ingo's idea we do a kallsyms lookup of __builtin_return_address(0) .. same data, less memory. But my goal was actually to reduce icache pollution, so by my reckoning code bytes were much more expensive than data ones, so putting all BUG information in a separate section makes those bytes much less significant than putting anything inline in code. Also, the trap for WARN_ON would be smaller than BUG, because it wouldn't need the spurious infinite loop needed to make gcc understand the control flow of a BUG. On the other hand, you could put the call to out of line warning function in a separate section to achieve the same effect. yeah and gcc even has a compiler option for that. Doubt it's really worth it, we're still talking a few bytes here ;) J -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Ingo Molnar wrote: > * Arjan van de Ven <[EMAIL PROTECTED]> wrote: > > >> +#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, >> \ >> + __LINE__, __FUNCTION__) >> > > hm. This passes in 4 arguments to do_warn_on(). > > i think we could get away with no arguments (!), by using section > tricks. Firstly, we can get rid of __FUNCTION__ and replace it with a > ksyms lookup - that is fine enough. Secondly, we could put __FILE__ and > __LINE__ into a text section and key it up to the return address from > do_warn_on(). > BUG_ON already does this, and WARN_ON can reuse all the same machinery. J -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: > This patch moves WARN_ON() out of line entirely. I've considered keeping > the test inline and moving only the slowpath out of line, but I decided > against that: an out of line test reduces the pressure on the CPUs > branch predictor logic and gives smaller code, while a function call > to a fixed location is quite fast. Likewise I've considered doing > something > similar to BUG() (eg use a trapping instruction) but that's not really > better (it needs the test inline again and recovering from an invalid > instruction isn't quite fun). Power implements WARN_ON this way, and all the machinery is in place to generically implement WARN_ON that way if you want. It does generate denser code than the call (since its just a single trapping instruction with no need for argument setup), and the performance cost of the trap shouldn't matter if warnings are rare (which one would hope). J -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven пишет:
> Subject: move WARN_ON() out of line
> From: Arjan van de Ven <[EMAIL PROTECTED]>
> CC: Ingo Molnar <[EMAIL PROTECTED]>
> CC: Andrew Morton <[EMAIL PROTECTED]>
>
> A quick grep shows that there are currently 1145 instances of WARN_ON
> in the kernel. Currently, WARN_ON is pretty much entirely inlined,
> which makes it hard to enhance it without growing the size of the kernel
> (and getting Andrew unhappy).
>
> This patch moves WARN_ON() out of line entirely. I've considered keeping
> the test inline and moving only the slowpath out of line, but I decided
> against that: an out of line test reduces the pressure on the CPUs
> branch predictor logic and gives smaller code, while a function call
> to a fixed location is quite fast. Likewise I've considered doing something
> similar to BUG() (eg use a trapping instruction) but that's not really
> better (it needs the test inline again and recovering from an invalid
> instruction isn't quite fun).
>
> The code size reduction of this patch was about 6.5Kb (on a distro style
> .config):
>
>text databssdechexfilename
> 3096493 29345527607046150652 5dd9fcvmlinux.before
> 3090006 29345527607046144165 5dc0a5vmlinux.after
>
> Signed-off-by: Arjan van de Ven <[EMAIL PROTECTED]>
>
> ---
> include/asm-generic/bug.h | 13 -
> kernel/panic.c| 13 +
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> Index: linux-2.6.24-rc6/include/asm-generic/bug.h
> ===
> --- linux-2.6.24-rc6.orig/include/asm-generic/bug.h
> +++ linux-2.6.24-rc6/include/asm-generic/bug.h
> @@ -32,15 +32,10 @@ struct bug_entry {
> #endif
>
> #ifndef HAVE_ARCH_WARN_ON
> -#define WARN_ON(condition) ({\
> -int __ret_warn_on = !!(condition);\
> -if (unlikely(__ret_warn_on)) {\
> -printk("WARNING: at %s:%d %s()\n", __FILE__,\
> -__LINE__, __FUNCTION__);\
> -dump_stack();\
> -}\
> -unlikely(__ret_warn_on);\
> -})
> +extern int do_warn_on(const unsigned long condition, const char *file,
> +const int line, const char *function);
> +#define WARN_ON(condition) do_warn_on((unsigned long)(condition),
> __FILE__, \
> + __LINE__, __FUNCTION__)
> #endif
>
> #else /* !CONFIG_BUG */
> Index: linux-2.6.24-rc6/kernel/panic.c
> ===
> --- linux-2.6.24-rc6.orig/kernel/panic.c
> +++ linux-2.6.24-rc6/kernel/panic.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> int panic_on_oops;
> int tainted;
> @@ -292,6 +293,18 @@ void oops_exit(void)
> (unsigned long long)oops_id);
> }
>
> +int do_warn_on(const unsigned long condition, const char *file,
> +const int line, const char *function)
> +{
> +if (unlikely(condition)) {
> +printk(KERN_WARNING "WARNING: at %s:%d %s()\n",
> +__FILE__, __LINE__, __FUNCTION__);
Isn't this going to print "panic.c" for __FILE__, "do_warn_on" for __FUNCTION__
and whatever line number you have there for __LINE__? I put up a userspace
equivalent
of what you're doing here, which confirms my suspision:
[EMAIL PROTECTED]:/tmp$ cat c.c
#include
#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \
__LINE__, __FUNCTION__)
int do_warn_on(const unsigned long condition, const char *file,
const int line, const char *function)
{
if (condition) {
printf("WARNING: at %s:%d %s()\n",
__FILE__, __LINE__, __FUNCTION__);
}
return !!condition;
}
int main()
{
WARN_ON(1);
return 0;
}
[EMAIL PROTECTED]:/tmp$ cc c.c
[EMAIL PROTECTED]:/tmp$ ./a.out
WARNING: at c.c:11 do_warn_on()
[EMAIL PROTECTED]:/tmp$
I think that your intention was to propagate the parameters to the do_warn_on()
routine
to the printk() call, right?
> +dump_stack();
> +}
> +return !!condition;
> +}
> +EXPORT_SYMBOL(do_warn_on);
> +
> #ifdef CONFIG_CC_STACKPROTECTOR
> /*
> * Called when gcc's -fstack-protector feature is used, and
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven <[EMAIL PROTECTED]> wrote:
>
>> While we're here, I'll mention that dump_stack probably ought to take a
>> severity level argument.
>
> 125 files changed, 202 insertions(+), 199 deletions(-)
> just to get the api change done.
> I can hear akpm cringe from here...
You don't have to change all of them at once. Just create a new function
that does take a level and make the old dump_stack and WARN_ON call that
then slowly convert everything else across if so desired.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven [EMAIL PROTECTED] wrote:
While we're here, I'll mention that dump_stack probably ought to take a
severity level argument.
125 files changed, 202 insertions(+), 199 deletions(-)
just to get the api change done.
I can hear akpm cringe from here...
You don't have to change all of them at once. Just create a new function
that does take a level and make the old dump_stack and WARN_ON call that
then slowly convert everything else across if so desired.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven пишет:
Subject: move WARN_ON() out of line
From: Arjan van de Ven [EMAIL PROTECTED]
CC: Ingo Molnar [EMAIL PROTECTED]
CC: Andrew Morton [EMAIL PROTECTED]
A quick grep shows that there are currently 1145 instances of WARN_ON
in the kernel. Currently, WARN_ON is pretty much entirely inlined,
which makes it hard to enhance it without growing the size of the kernel
(and getting Andrew unhappy).
This patch moves WARN_ON() out of line entirely. I've considered keeping
the test inline and moving only the slowpath out of line, but I decided
against that: an out of line test reduces the pressure on the CPUs
branch predictor logic and gives smaller code, while a function call
to a fixed location is quite fast. Likewise I've considered doing something
similar to BUG() (eg use a trapping instruction) but that's not really
better (it needs the test inline again and recovering from an invalid
instruction isn't quite fun).
The code size reduction of this patch was about 6.5Kb (on a distro style
.config):
text databssdechexfilename
3096493 29345527607046150652 5dd9fcvmlinux.before
3090006 29345527607046144165 5dc0a5vmlinux.after
Signed-off-by: Arjan van de Ven [EMAIL PROTECTED]
---
include/asm-generic/bug.h | 13 -
kernel/panic.c| 13 +
2 files changed, 17 insertions(+), 9 deletions(-)
Index: linux-2.6.24-rc6/include/asm-generic/bug.h
===
--- linux-2.6.24-rc6.orig/include/asm-generic/bug.h
+++ linux-2.6.24-rc6/include/asm-generic/bug.h
@@ -32,15 +32,10 @@ struct bug_entry {
#endif
#ifndef HAVE_ARCH_WARN_ON
-#define WARN_ON(condition) ({\
-int __ret_warn_on = !!(condition);\
-if (unlikely(__ret_warn_on)) {\
-printk(WARNING: at %s:%d %s()\n, __FILE__,\
-__LINE__, __FUNCTION__);\
-dump_stack();\
-}\
-unlikely(__ret_warn_on);\
-})
+extern int do_warn_on(const unsigned long condition, const char *file,
+const int line, const char *function);
+#define WARN_ON(condition) do_warn_on((unsigned long)(condition),
__FILE__, \
+ __LINE__, __FUNCTION__)
#endif
#else /* !CONFIG_BUG */
Index: linux-2.6.24-rc6/kernel/panic.c
===
--- linux-2.6.24-rc6.orig/kernel/panic.c
+++ linux-2.6.24-rc6/kernel/panic.c
@@ -20,6 +20,7 @@
#include linux/kexec.h
#include linux/debug_locks.h
#include linux/random.h
+#include asm/bug.h
int panic_on_oops;
int tainted;
@@ -292,6 +293,18 @@ void oops_exit(void)
(unsigned long long)oops_id);
}
+int do_warn_on(const unsigned long condition, const char *file,
+const int line, const char *function)
+{
+if (unlikely(condition)) {
+printk(KERN_WARNING WARNING: at %s:%d %s()\n,
+__FILE__, __LINE__, __FUNCTION__);
Isn't this going to print panic.c for __FILE__, do_warn_on for __FUNCTION__
and whatever line number you have there for __LINE__? I put up a userspace
equivalent
of what you're doing here, which confirms my suspision:
[EMAIL PROTECTED]:/tmp$ cat c.c
#include stdio.h
#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \
__LINE__, __FUNCTION__)
int do_warn_on(const unsigned long condition, const char *file,
const int line, const char *function)
{
if (condition) {
printf(WARNING: at %s:%d %s()\n,
__FILE__, __LINE__, __FUNCTION__);
}
return !!condition;
}
int main()
{
WARN_ON(1);
return 0;
}
[EMAIL PROTECTED]:/tmp$ cc c.c
[EMAIL PROTECTED]:/tmp$ ./a.out
WARNING: at c.c:11 do_warn_on()
[EMAIL PROTECTED]:/tmp$
I think that your intention was to propagate the parameters to the do_warn_on()
routine
to the printk() call, right?
+dump_stack();
+}
+return !!condition;
+}
+EXPORT_SYMBOL(do_warn_on);
+
#ifdef CONFIG_CC_STACKPROTECTOR
/*
* Called when gcc's -fstack-protector feature is used, and
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Arjan van de Ven wrote: This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Power implements WARN_ON this way, and all the machinery is in place to generically implement WARN_ON that way if you want. It does generate denser code than the call (since its just a single trapping instruction with no need for argument setup), and the performance cost of the trap shouldn't matter if warnings are rare (which one would hope). J -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Ingo Molnar wrote: * Arjan van de Ven [EMAIL PROTECTED] wrote: +#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \ + __LINE__, __FUNCTION__) hm. This passes in 4 arguments to do_warn_on(). i think we could get away with no arguments (!), by using section tricks. Firstly, we can get rid of __FUNCTION__ and replace it with a ksyms lookup - that is fine enough. Secondly, we could put __FILE__ and __LINE__ into a text section and key it up to the return address from do_warn_on(). BUG_ON already does this, and WARN_ON can reuse all the same machinery. J -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Matt Mackall wrote: I hate the do_foo naming scheme (how about __warn_on?), but otherwise: Acked-by: Matt Mackall <[EMAIL PROTECTED]> after I moved it around based on Olof's work, I've now ended up with warn_on_slowpath() + printk(KERN_WARNING "WARNING: at %s:%d %s()\n", + __FILE__, __LINE__, __FUNCTION__); + dump_stack(); While we're here, I'll mention that dump_stack probably ought to take a severity level argument. 125 files changed, 202 insertions(+), 199 deletions(-) just to get the api change done. I can hear akpm cringe from here... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Olof Johansson wrote: On Thu, Jan 03, 2008 at 01:56:58AM +0100, Arjan van de Ven wrote: Subject: move WARN_ON() out of line From: Arjan van de Ven <[EMAIL PROTECTED]> CC: Ingo Molnar <[EMAIL PROTECTED]> CC: Andrew Morton <[EMAIL PROTECTED]> A quick grep shows that there are currently 1145 instances of WARN_ON in the kernel. Currently, WARN_ON is pretty much entirely inlined, which makes it hard to enhance it without growing the size of the kernel (and getting Andrew unhappy). This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Hi Arjan, I've got a couple of patches in -mm at the moment that introduces __WARN() and uses that (and lets architectures override __WARN, since for example powerpc does use trapping instructions similarly to BUG()). The two patches in question are: bugh-remove-have_arch_bug--have_arch_warn.patch powerpc-switch-to-generic-warn_on-bug_on.patch Care to do this incrementally on top of that instead? I.e. call do_warn_on() from the asm-generic/bug.h __WARN() instead. ok just did that; will post shortly -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Ingo Molnar wrote: * Arjan van de Ven <[EMAIL PROTECTED]> wrote: +#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \ +__LINE__, __FUNCTION__) hm. This passes in 4 arguments to do_warn_on(). i think we could get away with no arguments (!), by using section tricks. Firstly, we can get rid of __FUNCTION__ and replace it with a ksyms lookup - that is fine enough. I can see that; I'll play with that Secondly, we could put __FILE__ and __LINE__ into a text section and key it up to the return address from do_warn_on(). the asm generated for this is 2 movl instructions for immediate to register. Doing fancy tricks ... it may well end up bigger and gain nothing. the condition code should not be passed in at all i think - it creates extra function calls to do_warn_on() all the time. function calls are *CHEAP*. passing the condition is actually near free (remember we have regparm!), it's likely to be in a register already anyway. Doing the test inline makes stuff bigger, and also spreads the branch prediction pain around rather than having one nicely predictable place... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Hi Arjan,
On Jan 3, 2008 2:56 AM, Arjan van de Ven <[EMAIL PROTECTED]> wrote:
> +int do_warn_on(const unsigned long condition, const char *file,
> + const int line, const char *function)
> +{
> + if (unlikely(condition)) {
> + printk(KERN_WARNING "WARNING: at %s:%d %s()\n",
> + __FILE__, __LINE__, __FUNCTION__);
I don't think you want to use __FILE__ and friends here...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
* Arjan van de Ven <[EMAIL PROTECTED]> wrote: > +#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \ > + __LINE__, __FUNCTION__) hm. This passes in 4 arguments to do_warn_on(). i think we could get away with no arguments (!), by using section tricks. Firstly, we can get rid of __FUNCTION__ and replace it with a ksyms lookup - that is fine enough. Secondly, we could put __FILE__ and __LINE__ into a text section and key it up to the return address from do_warn_on(). the condition code should not be passed in at all i think - it creates extra function calls to do_warn_on() all the time. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
* Arjan van de Ven [EMAIL PROTECTED] wrote: +#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \ + __LINE__, __FUNCTION__) hm. This passes in 4 arguments to do_warn_on(). i think we could get away with no arguments (!), by using section tricks. Firstly, we can get rid of __FUNCTION__ and replace it with a ksyms lookup - that is fine enough. Secondly, we could put __FILE__ and __LINE__ into a text section and key it up to the return address from do_warn_on(). the condition code should not be passed in at all i think - it creates extra function calls to do_warn_on() all the time. Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Hi Arjan,
On Jan 3, 2008 2:56 AM, Arjan van de Ven [EMAIL PROTECTED] wrote:
+int do_warn_on(const unsigned long condition, const char *file,
+ const int line, const char *function)
+{
+ if (unlikely(condition)) {
+ printk(KERN_WARNING WARNING: at %s:%d %s()\n,
+ __FILE__, __LINE__, __FUNCTION__);
I don't think you want to use __FILE__ and friends here...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Ingo Molnar wrote: * Arjan van de Ven [EMAIL PROTECTED] wrote: +#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \ +__LINE__, __FUNCTION__) hm. This passes in 4 arguments to do_warn_on(). i think we could get away with no arguments (!), by using section tricks. Firstly, we can get rid of __FUNCTION__ and replace it with a ksyms lookup - that is fine enough. I can see that; I'll play with that Secondly, we could put __FILE__ and __LINE__ into a text section and key it up to the return address from do_warn_on(). the asm generated for this is 2 movl instructions for immediate to register. Doing fancy tricks ... it may well end up bigger and gain nothing. the condition code should not be passed in at all i think - it creates extra function calls to do_warn_on() all the time. function calls are *CHEAP*. passing the condition is actually near free (remember we have regparm!), it's likely to be in a register already anyway. Doing the test inline makes stuff bigger, and also spreads the branch prediction pain around rather than having one nicely predictable place... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Olof Johansson wrote: On Thu, Jan 03, 2008 at 01:56:58AM +0100, Arjan van de Ven wrote: Subject: move WARN_ON() out of line From: Arjan van de Ven [EMAIL PROTECTED] CC: Ingo Molnar [EMAIL PROTECTED] CC: Andrew Morton [EMAIL PROTECTED] A quick grep shows that there are currently 1145 instances of WARN_ON in the kernel. Currently, WARN_ON is pretty much entirely inlined, which makes it hard to enhance it without growing the size of the kernel (and getting Andrew unhappy). This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Hi Arjan, I've got a couple of patches in -mm at the moment that introduces __WARN() and uses that (and lets architectures override __WARN, since for example powerpc does use trapping instructions similarly to BUG()). The two patches in question are: bugh-remove-have_arch_bug--have_arch_warn.patch powerpc-switch-to-generic-warn_on-bug_on.patch Care to do this incrementally on top of that instead? I.e. call do_warn_on() from the asm-generic/bug.h __WARN() instead. ok just did that; will post shortly -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
Matt Mackall wrote: I hate the do_foo naming scheme (how about __warn_on?), but otherwise: Acked-by: Matt Mackall [EMAIL PROTECTED] after I moved it around based on Olof's work, I've now ended up with warn_on_slowpath() + printk(KERN_WARNING WARNING: at %s:%d %s()\n, + __FILE__, __LINE__, __FUNCTION__); + dump_stack(); While we're here, I'll mention that dump_stack probably ought to take a severity level argument. 125 files changed, 202 insertions(+), 199 deletions(-) just to get the api change done. I can hear akpm cringe from here... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
On Thu, Jan 03, 2008 at 01:56:58AM +0100, Arjan van de Ven wrote: > Subject: move WARN_ON() out of line > From: Arjan van de Ven <[EMAIL PROTECTED]> > CC: Ingo Molnar <[EMAIL PROTECTED]> > CC: Andrew Morton <[EMAIL PROTECTED]> > > A quick grep shows that there are currently 1145 instances of WARN_ON > in the kernel. Currently, WARN_ON is pretty much entirely inlined, > which makes it hard to enhance it without growing the size of the kernel > (and getting Andrew unhappy). > > This patch moves WARN_ON() out of line entirely. I've considered keeping > the test inline and moving only the slowpath out of line, but I decided > against that: an out of line test reduces the pressure on the CPUs > branch predictor logic and gives smaller code, while a function call > to a fixed location is quite fast. Likewise I've considered doing something > similar to BUG() (eg use a trapping instruction) but that's not really > better (it needs the test inline again and recovering from an invalid > instruction isn't quite fun). Hi Arjan, I've got a couple of patches in -mm at the moment that introduces __WARN() and uses that (and lets architectures override __WARN, since for example powerpc does use trapping instructions similarly to BUG()). The two patches in question are: bugh-remove-have_arch_bug--have_arch_warn.patch powerpc-switch-to-generic-warn_on-bug_on.patch Care to do this incrementally on top of that instead? I.e. call do_warn_on() from the asm-generic/bug.h __WARN() instead. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
On Thu, 2008-01-03 at 01:56 +0100, Arjan van de Ven wrote: > Subject: move WARN_ON() out of line > From: Arjan van de Ven <[EMAIL PROTECTED]> > CC: Ingo Molnar <[EMAIL PROTECTED]> > CC: Andrew Morton <[EMAIL PROTECTED]> > > A quick grep shows that there are currently 1145 instances of WARN_ON > in the kernel. Currently, WARN_ON is pretty much entirely inlined, > which makes it hard to enhance it without growing the size of the kernel > (and getting Andrew unhappy). > > This patch moves WARN_ON() out of line entirely. I've considered keeping > the test inline and moving only the slowpath out of line, but I decided > against that: an out of line test reduces the pressure on the CPUs > branch predictor logic and gives smaller code, while a function call > to a fixed location is quite fast. Likewise I've considered doing something > similar to BUG() (eg use a trapping instruction) but that's not really > better (it needs the test inline again and recovering from an invalid > instruction isn't quite fun). > > The code size reduction of this patch was about 6.5Kb (on a distro style > .config): > > text data bss dec hex filename > 3096493293455 2760704 6150652 5dd9fc vmlinux.before > 3090006293455 2760704 6144165 5dc0a5 vmlinux.after > > Signed-off-by: Arjan van de Ven <[EMAIL PROTECTED]> I hate the do_foo naming scheme (how about __warn_on?), but otherwise: Acked-by: Matt Mackall <[EMAIL PROTECTED]> > + printk(KERN_WARNING "WARNING: at %s:%d %s()\n", > + __FILE__, __LINE__, __FUNCTION__); > + dump_stack(); While we're here, I'll mention that dump_stack probably ought to take a severity level argument. -- Mathematics is the supreme nostalgia of our time. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 1/3] move WARN_ON() out of line
Subject: move WARN_ON() out of line
From: Arjan van de Ven <[EMAIL PROTECTED]>
CC: Ingo Molnar <[EMAIL PROTECTED]>
CC: Andrew Morton <[EMAIL PROTECTED]>
A quick grep shows that there are currently 1145 instances of WARN_ON
in the kernel. Currently, WARN_ON is pretty much entirely inlined,
which makes it hard to enhance it without growing the size of the kernel
(and getting Andrew unhappy).
This patch moves WARN_ON() out of line entirely. I've considered keeping
the test inline and moving only the slowpath out of line, but I decided
against that: an out of line test reduces the pressure on the CPUs
branch predictor logic and gives smaller code, while a function call
to a fixed location is quite fast. Likewise I've considered doing something
similar to BUG() (eg use a trapping instruction) but that's not really
better (it needs the test inline again and recovering from an invalid
instruction isn't quite fun).
The code size reduction of this patch was about 6.5Kb (on a distro style
.config):
textdata bss dec hex filename
3096493 293455 2760704 6150652 5dd9fc vmlinux.before
3090006 293455 2760704 6144165 5dc0a5 vmlinux.after
Signed-off-by: Arjan van de Ven <[EMAIL PROTECTED]>
---
include/asm-generic/bug.h | 13 -
kernel/panic.c| 13 +
2 files changed, 17 insertions(+), 9 deletions(-)
Index: linux-2.6.24-rc6/include/asm-generic/bug.h
===
--- linux-2.6.24-rc6.orig/include/asm-generic/bug.h
+++ linux-2.6.24-rc6/include/asm-generic/bug.h
@@ -32,15 +32,10 @@ struct bug_entry {
#endif
#ifndef HAVE_ARCH_WARN_ON
-#define WARN_ON(condition) ({ \
- int __ret_warn_on = !!(condition); \
- if (unlikely(__ret_warn_on)) { \
- printk("WARNING: at %s:%d %s()\n", __FILE__, \
- __LINE__, __FUNCTION__);\
- dump_stack(); \
- } \
- unlikely(__ret_warn_on);\
-})
+extern int do_warn_on(const unsigned long condition, const char *file,
+ const int line, const char *function);
+#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \
+__LINE__, __FUNCTION__)
#endif
#else /* !CONFIG_BUG */
Index: linux-2.6.24-rc6/kernel/panic.c
===
--- linux-2.6.24-rc6.orig/kernel/panic.c
+++ linux-2.6.24-rc6/kernel/panic.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
int panic_on_oops;
int tainted;
@@ -292,6 +293,18 @@ void oops_exit(void)
(unsigned long long)oops_id);
}
+int do_warn_on(const unsigned long condition, const char *file,
+ const int line, const char *function)
+{
+ if (unlikely(condition)) {
+ printk(KERN_WARNING "WARNING: at %s:%d %s()\n",
+ __FILE__, __LINE__, __FUNCTION__);
+ dump_stack();
+ }
+ return !!condition;
+}
+EXPORT_SYMBOL(do_warn_on);
+
#ifdef CONFIG_CC_STACKPROTECTOR
/*
* Called when gcc's -fstack-protector feature is used, and
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 1/3] move WARN_ON() out of line
Subject: move WARN_ON() out of line
From: Arjan van de Ven [EMAIL PROTECTED]
CC: Ingo Molnar [EMAIL PROTECTED]
CC: Andrew Morton [EMAIL PROTECTED]
A quick grep shows that there are currently 1145 instances of WARN_ON
in the kernel. Currently, WARN_ON is pretty much entirely inlined,
which makes it hard to enhance it without growing the size of the kernel
(and getting Andrew unhappy).
This patch moves WARN_ON() out of line entirely. I've considered keeping
the test inline and moving only the slowpath out of line, but I decided
against that: an out of line test reduces the pressure on the CPUs
branch predictor logic and gives smaller code, while a function call
to a fixed location is quite fast. Likewise I've considered doing something
similar to BUG() (eg use a trapping instruction) but that's not really
better (it needs the test inline again and recovering from an invalid
instruction isn't quite fun).
The code size reduction of this patch was about 6.5Kb (on a distro style
.config):
textdata bss dec hex filename
3096493 293455 2760704 6150652 5dd9fc vmlinux.before
3090006 293455 2760704 6144165 5dc0a5 vmlinux.after
Signed-off-by: Arjan van de Ven [EMAIL PROTECTED]
---
include/asm-generic/bug.h | 13 -
kernel/panic.c| 13 +
2 files changed, 17 insertions(+), 9 deletions(-)
Index: linux-2.6.24-rc6/include/asm-generic/bug.h
===
--- linux-2.6.24-rc6.orig/include/asm-generic/bug.h
+++ linux-2.6.24-rc6/include/asm-generic/bug.h
@@ -32,15 +32,10 @@ struct bug_entry {
#endif
#ifndef HAVE_ARCH_WARN_ON
-#define WARN_ON(condition) ({ \
- int __ret_warn_on = !!(condition); \
- if (unlikely(__ret_warn_on)) { \
- printk(WARNING: at %s:%d %s()\n, __FILE__, \
- __LINE__, __FUNCTION__);\
- dump_stack(); \
- } \
- unlikely(__ret_warn_on);\
-})
+extern int do_warn_on(const unsigned long condition, const char *file,
+ const int line, const char *function);
+#define WARN_ON(condition) do_warn_on((unsigned long)(condition), __FILE__, \
+__LINE__, __FUNCTION__)
#endif
#else /* !CONFIG_BUG */
Index: linux-2.6.24-rc6/kernel/panic.c
===
--- linux-2.6.24-rc6.orig/kernel/panic.c
+++ linux-2.6.24-rc6/kernel/panic.c
@@ -20,6 +20,7 @@
#include linux/kexec.h
#include linux/debug_locks.h
#include linux/random.h
+#include asm/bug.h
int panic_on_oops;
int tainted;
@@ -292,6 +293,18 @@ void oops_exit(void)
(unsigned long long)oops_id);
}
+int do_warn_on(const unsigned long condition, const char *file,
+ const int line, const char *function)
+{
+ if (unlikely(condition)) {
+ printk(KERN_WARNING WARNING: at %s:%d %s()\n,
+ __FILE__, __LINE__, __FUNCTION__);
+ dump_stack();
+ }
+ return !!condition;
+}
+EXPORT_SYMBOL(do_warn_on);
+
#ifdef CONFIG_CC_STACKPROTECTOR
/*
* Called when gcc's -fstack-protector feature is used, and
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
On Thu, 2008-01-03 at 01:56 +0100, Arjan van de Ven wrote: Subject: move WARN_ON() out of line From: Arjan van de Ven [EMAIL PROTECTED] CC: Ingo Molnar [EMAIL PROTECTED] CC: Andrew Morton [EMAIL PROTECTED] A quick grep shows that there are currently 1145 instances of WARN_ON in the kernel. Currently, WARN_ON is pretty much entirely inlined, which makes it hard to enhance it without growing the size of the kernel (and getting Andrew unhappy). This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). The code size reduction of this patch was about 6.5Kb (on a distro style .config): text data bss dec hex filename 3096493293455 2760704 6150652 5dd9fc vmlinux.before 3090006293455 2760704 6144165 5dc0a5 vmlinux.after Signed-off-by: Arjan van de Ven [EMAIL PROTECTED] I hate the do_foo naming scheme (how about __warn_on?), but otherwise: Acked-by: Matt Mackall [EMAIL PROTECTED] + printk(KERN_WARNING WARNING: at %s:%d %s()\n, + __FILE__, __LINE__, __FUNCTION__); + dump_stack(); While we're here, I'll mention that dump_stack probably ought to take a severity level argument. -- Mathematics is the supreme nostalgia of our time. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] move WARN_ON() out of line
On Thu, Jan 03, 2008 at 01:56:58AM +0100, Arjan van de Ven wrote: Subject: move WARN_ON() out of line From: Arjan van de Ven [EMAIL PROTECTED] CC: Ingo Molnar [EMAIL PROTECTED] CC: Andrew Morton [EMAIL PROTECTED] A quick grep shows that there are currently 1145 instances of WARN_ON in the kernel. Currently, WARN_ON is pretty much entirely inlined, which makes it hard to enhance it without growing the size of the kernel (and getting Andrew unhappy). This patch moves WARN_ON() out of line entirely. I've considered keeping the test inline and moving only the slowpath out of line, but I decided against that: an out of line test reduces the pressure on the CPUs branch predictor logic and gives smaller code, while a function call to a fixed location is quite fast. Likewise I've considered doing something similar to BUG() (eg use a trapping instruction) but that's not really better (it needs the test inline again and recovering from an invalid instruction isn't quite fun). Hi Arjan, I've got a couple of patches in -mm at the moment that introduces __WARN() and uses that (and lets architectures override __WARN, since for example powerpc does use trapping instructions similarly to BUG()). The two patches in question are: bugh-remove-have_arch_bug--have_arch_warn.patch powerpc-switch-to-generic-warn_on-bug_on.patch Care to do this incrementally on top of that instead? I.e. call do_warn_on() from the asm-generic/bug.h __WARN() instead. -Olof -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
