move WARN_ON() out of line
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):
text data bss dec hex filename
3096493 293455 2760704 6150652 5dd9fc vmlinux.before
3090006 293455 2760704 6144165 5dc0a5 vmlinux.after
--- include/asm-generic/bug.h | 13 ++++--------- kernel/panic.c | 13 +++++++++++++ 2 files changed, 17 insertions(+), 9 deletions(-)
include/asm-generic/bug.h +4 -9
@@ -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 */
kernel/panic.c +13 -0
@@ -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
Quoted 19 lines · 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_ONin the kernel. Currently,WARN_ONis 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 toBUG()(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).
Quoted 8 lines
The code size reduction of this patch was about 6.5Kb (on a distro style .config):
text data bss dec hex filename
3096493 293455 2760704 6150652 5dd9fc vmlinux.before
3090006 293455 2760704 6144165 5dc0a5 vmlinux.afterSigned-off-by Arjan van de Ven <[email protected]>
I hate the do_foo naming scheme (how about __warn_on?), but otherwise:
kernel/panic.c void oops_exit(void) · line 302
Quoted 3 lines
kernel/panic.c +3
-0+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.
Signature · 1 line
Mathematics is the supreme nostalgia of our time.
Quoted 10 lines · 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_ONin the kernel. Currently,WARN_ONis pretty much entirely inlined, which makes it hard to enhance it without growing the size of the kernel (and getting Andrew unhappy).
Quoted 8 lines
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 toBUG()(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
include/asm-generic/bug.h struct bug_entry { · line 38
Quoted 2 lines · Arjan van de Ven
include/asm-generic/bug.h +2
-0+#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
Hi Arjan,
kernel/panic.c void oops_exit(void) · line 301
Quoted 6 lines · Arjan van de Ven
kernel/panic.c +6
-0+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...
Quoted 8 lines · Ingo Molnar
include/asm-generic/bug.h struct bug_entry { · line 38
Quoted 2 lines · Arjan van de Ven
include/asm-generic/bug.h +2
-0+#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
Quoted 1 line
Secondly, we could put
__FILE__and__LINE__into a text section and key it up to the return address fromdo_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.
Quoted 2 lines
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...
Quoted 26 lines · Olof Johansson
Quoted 10 lines · 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_ONin the kernel. Currently,WARN_ONis pretty much entirely inlined, which makes it hard to enhance it without growing the size of the kernel (and getting Andrew unhappy).Quoted 8 lines
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 toBUG()(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 toBUG()).
Quoted 9 lines
The two patches in question are:
bugh-remove-have_arch_bug--have_arch_warn.patch
powerpc-switch-to-generic-warn_on-bug_on.patchCare to do this incrementally on top of that instead? I.e. call
do_warn_on()from theasm-generic/bug.h__WARN()instead.
ok just did that; will post shortly
Quoted 4 lines · Matt Mackall
I hate the
do_foonaming 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()
Quoted 7 lines
kernel/panic.c void oops_exit(void) · line 302
Quoted 4 lines
kernel/panic.c +3
-0+printk (KERN_WARNING "WARNING: at %s:%d %s()\n" , + __FILE__, __LINE__, __FUNCTION__); +dump_stack ();While we're here, I'll mention that
dump_stackprobably 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...
Quoted 1 line
Quoted 7 lines · Arjan van de Ven
Quoted 3 lines
While we're here, I'll mention that
dump_stackprobably 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,
Signature · 4 lines
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.txtArjan van de Ven пишет:
Quoted 71 lines
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_ONin the kernel. Currently,WARN_ONis 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 toBUG()(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 3096493 293455 2760704 6150652 5dd9fc vmlinux.before 3090006 293455 2760704 6144165 5dc0a5 vmlinux.afterSigned-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
===================================================================include/asm-generic/bug.h +4
-9@@ -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
===================================================================kernel/panic.c +1
-0@@ -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;
kernel/panic.c void oops_exit(void) · line 301
Quoted 9 lines
kernel/panic.c +6
-0@@ -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:
dmvo@cipher:/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;
}
dmvo@cipher:/tmp$ cc c.c
dmvo@cipher:/tmp$ ./a.out
WARNING: at c.c:11 do_warn_on()
dmvo@cipher:/tmp$I think that your intention was to propagate the parameters to the do_warn_on() routine to the printk() call, right?
Quoted 15 lines
kernel/panic.c +6
-0+dump_stack (); + } +return !!condition; +} +EXPORT_SYMBOL (do_warn_on); +#ifdef CONFIG_CC_STACKPROTECTOR /* * Called when gcc's -fstack-protector feature is used, and Signature · 4 lines
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/
Quoted 9 lines · Arjan van de Ven
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 toBUG()(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
Quoted 9 lines · Ingo Molnar
include/asm-generic/bug.h struct bug_entry { · line 38
Quoted 3 lines · Arjan van de Ven
include/asm-generic/bug.h +2
-0+#define WARN_ON (condition) do_warn_on((unsigned long)(condition), __FILE__, \ + __LINE__, __FUNCTION__)hm. This passes in 4 arguments to
do_warn_on().
Quoted 6 lines
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 fromdo_warn_on().
BUG_ON already does this, and WARN_ON can reuse all the same machinery.
J
Quoted 11 lines · Jeremy Fitzhardinge
Quoted 9 lines · Arjan van de Ven
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 toBUG()(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).
Quoted 5 lines
Power implements
WARN_ONthis way, and all the machinery is in place to generically implementWARN_ONthat 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.
Quoted 27 lines · Arjan van de Ven
Quoted 11 lines · Jeremy Fitzhardinge
Quoted 9 lines · Arjan van de Ven
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 toBUG()(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).Quoted 5 lines
Power implements
WARN_ONthis way, and all the machinery is in place to generically implementWARN_ONthat 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 theBUG_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.
Quoted 5 lines
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....
Quoted 8 lines · Herbert Xu
Quoted 5 lines · Arjan van de Ven
Quoted 2 lines
While we're here, I'll mention that
dump_stackprobably 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.
Quoted 11 lines · Arjan van de Ven
Quoted 5 lines
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_ONpatch series. (before Ingo's suggestion)
In the new model, even at 1024 the out of lineWARN_ONfunction call is smaller than theBUG_ONmethod.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)
Quoted 34 lines · Arjan van de Ven
Quoted 12 lines · Jeremy Fitzhardinge
Quoted 10 lines · Arjan van de Ven
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 toBUG()(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).Quoted 5 lines
Power implements
WARN_ONthis way, and all the machinery is in place to generically implementWARN_ONthat 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 theBUG_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.
Quoted 5 lines
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
Quoted 5 lines · Jeremy Fitzhardinge
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.
Quoted 9 lines
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_ONwould 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 ;)
Quoted 2 lines
J