i386: enable 4k stacks by default

4k stacks have become a well-tested feature used fore a long time in Fedora and even in RHEL 4.

Quoted 2 lines · Adrian Bunk

4k stacks have become a well-tested feature used fore a long time in Fedora and even in RHEL 4.

So has anyone fixed the bugs involving ext3 and LVM snapshots on top of DM mirror?

I doubt it. Every time this comes up the problem of stacked I/O configuration being able to reliably blow the 4k stack limit comes up. Usually it's XFS that is blamed, but it seems ext3 and reiser can both suffer from the same problem. And now we can add things like unionfs/ecryptfs into the stack as well....

If this still occurs for some
combinations then the fix would be 8K + 4K IRQ stack, not just to use 8K stack

Quoted 3 lines · Alan Cox

If this still occurs for some
combinations then the fix would be 8K + 4K IRQ stack, not just to use 8K stack

Yes i've been thinking for some time doing that would be a good idea.

Quoted 6 lines · Andi Kleen

Quoted 3 lines · Alan Cox

If this still occurs for some
combinations then the fix would be 8K + 4K IRQ stack, not just to use 8K stack

Quoted 1 line

Yes i've been thinking for some time doing that would be a good idea.

Yes, the non-irqstack case should definitively go away. And 8k kernel stacks isn't that little given how much most 64bit architectures have.

Actually looking at the code it would need some fixes first:
/*
* These should really be __section__(".bss.page_aligned") as well, but
* gcc's 3.0 and earlier don't handle that correctly.
*/
static char softirq_stack[NR_CPUS * THREAD_SIZE]

                __attribute__((__aligned__(THREAD_SIZE)));

static char hardirq_stack[NR_CPUS * THREAD_SIZE]

                __attribute__((__aligned__(THREAD_SIZE)));

With 8K stacks and NR_CPUS==128 that would be 2MB statically reserved. Yuck. Really needs to be dynamically allocated. I'll take a look once the .22 big merge is done.

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 1 line · William Lee Irwin III

Here's what I did for i386 for someone concerned about blowing the stack.

Make more stack sizes configurable, adding options for deeper stacks. This is largely for differential diagnosis in cases where stack overflows are suspected of silently corrupting memory or causing other problems not detected immediately.

Quoted 4 lines

Make more stack sizes configurable, adding options for deeper stacks. This is largely for differential diagnosis in cases where stack overflows are suspected of silently corrupting memory or causing other problems not detected immediately.

I don't think we want anything bigger than 8, if you really need that for debuggin it's easy enough to hack up on demand.

Quoted 1 line · William Lee Irwin III

Here's what I did for i386 for someone concerned about blowing the stack.

Make more stack sizes configurable, adding options for deeper stacks. This is largely for differential diagnosis in cases where stack overflows are suspected of silently corrupting memory or causing other problems not detected immediately.

Signed-off-by William Irwin <[email protected]>

Index: stack-paranoia/arch/i386/Kconfig.debug
===================================================================

arch/i386/Kconfig.debug +4 -0

@@ -56,6 +56,10 @@
 	  portion of the kernel code won't be covered by a 2MB TLB anymore.
 	  If in doubt, say "N".
 
+choice
+	prompt "Stack size"
+	default 8KSTACKS
+
 config 4KSTACKS
 	bool "Use 4Kb for kernel stacks instead of 8Kb"
 	depends on DEBUG_KERNEL

These are no questions a user should ever see, and it's impossible that a user will be able to choose the right answer - that's an implementation detail, and it has to be set correctly automatically without bothering the user.
Pro of bigger stacks:

- kernel developers have to care less about stack usage
Pros of smaller stacks:
- less memory usage
- higher order allocations are more likely to fail with fragmented memory
We are currently at "works with 8k stacks and without separate IRQ stacks" and at "works in most situations with 4k stacks", so nothing > 8k should be required.

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 1 line · William Lee Irwin III

Here's what I did for i386 for someone concerned about blowing the stack.

Add a config option to vmalloc() task stacks so that stack overflows are detected without fail, and with a fatal failure mode at that.

Quoted 5 lines · William Lee Irwin III

Quoted 1 line · William Lee Irwin III

Here's what I did for i386 for someone concerned about blowing the stack.

Add a config option to vmalloc() task stacks so that stack overflows are detected without fail, and with a fatal failure mode at that.

Whee, this sounds like a really helpful although costly debugging option.

Quoted 2 lines

Add a config option to vmalloc() task stacks so that stack overflows are detected without fail, and with a fatal failure mode at that.

Whee, this sounds like a really helpful although costly debugging option.

So if you want to invest some time into getting this into mergeable shape I'd suggest you redo the patch series in the following way:
patch 1: dynamic allocated irq stacks
patch 2: make irqstacks unconditional, but allow selecting 4/8k stacks patch 3: introduce a DEBUG_STACK option that enables vmalloc'ed stacks
and adds the __pa check.

So if you want to invest some time into getting this into mergeable shape I'd suggest you redo the patch series in the following way:
patch 1: dynamic allocated irq stacks
patch 2: make irqstacks unconditional, but allow selecting 4/8k stacks patch 3: introduce a DEBUG_STACK option that enables vmalloc'ed stacks
and adds the __pa check.

So if you want to invest some time into getting this into mergeable shape I'd suggest you redo the patch series in the following way:

patch 1: dynamic allocated irq stacks

Quoted 3 lines

So if you want to invest some time into getting this into mergeable shape I'd suggest you redo the patch series in the following way:
patch 1: dynamic allocated irq stacks

Can we register them lazily at request_irq time?

These IRQ stacks are per-cpu, not per-IRQ. It may make sense to implement per-IRQ stacks, in which case dynamic allocation at the time of request_irq() will make sense.

Quoted 3 lines

These IRQ stacks are per-cpu, not per-IRQ. It may make sense to implement per-IRQ stacks, in which case dynamic allocation at the time of request_irq() will make sense.

This depends if active IRQ count exceeds active CPU count worst cases. For the big boxes it might well do but for small ones we seem to be best with per CPU.

Quoted 3 lines · Christoph Hellwig

Quoted 3 lines

So if you want to invest some time into getting this into mergeable shape I'd suggest you redo the patch series in the following way:
patch 1: dynamic allocated irq stacks

Quoted 1 line · Matt Mackall

Can we register them lazily at request_irq time?

These IRQ stacks are per-cpu, not per-IRQ. It may make sense to implement per-IRQ stacks, in which case dynamic allocation at the time of request_irq() will make sense.

Would you like me to implement per-IRQ IRQ stacks?

Quoted 1 line

Quoted 1 line

Can we register them lazily at request_irq time?

Quoted 4 lines

These IRQ stacks are per-cpu, not per-IRQ. It may make sense to implement per-IRQ stacks, in which case dynamic allocation at the time of request_irq() will make sense.
Would you like me to implement per-IRQ IRQ stacks?

It's probably the "right" thing to do, but it does have higher overhead for most systems.
But it also gives a very obvious migration path to -rt's irq threads.

Quoted 4 lines

arch/i386/Kconfig.debug +4 -0

+	  Allocates the stack physically discontiguously and from high
+	  memory. Furthermore an unmapped guard page follows the stack.
+	  This is not for end-users. It's intended to trigger fatal
+	  system errors under various forms of stack abuse.

Why is this not for end-users? Will it not trigger anything
useful unless set up properly, or is a big performace hit -- and how, or what?
All the kernel debug options are underdocumented this way -- I'd
like to have as many of them on as I can without absolutely killing performance, (or rather, *you* would) -- but I can never tell without grovelling all over for the info, which... well, I haven't done it yet, anyway.

"End-user" is just insufficently defined for anyone compiling
their own kernel. Could you add a bit more text here describing what the effect of physically discontiguous high-memory stacks is? An additional frobnitz dereference on every badda-bing badda-bang, likely to double the time it takes to dance the hokey pokey?
*shrug* Some of those debug options probably don't get set very
often on kernels that are run for more than to see if it boots.

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 1 line · William Lee Irwin III

Here's what I did for i386 for someone concerned about blowing the stack.

Divorce IRQ stack configuration from CONFIG_4KSTACKS, as it's believed to be an important safety measure regardless of stack size by some users.

Quoted 2 lines

Divorce IRQ stack configuration from CONFIG_4KSTACKS, as it's believed to be an important safety measure regardless of stack size by some users.

We should just make irqstacks the default and kill the non-irqstack code.

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 1 line · William Lee Irwin III

Here's what I did for i386 for someone concerned about blowing the stack.

Add checks to __pa() so it goes BUG() on vmallocspace addresses.

Quoted 1 line

Quoted 1 line

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 1 line

Add checks to __pa() so it goes BUG() on vmallocspace addresses.

Sorry I think that's too costly to do. __pa is pretty common

Quoted 4 lines · William Lee Irwin III

Quoted 1 line · William Lee Irwin III

Here's what I did for i386 for someone concerned about blowing the stack.

Add checks to __pa() so it goes BUG() on vmallocspace addresses.

Sorry I think that's too costly to do. __pa is pretty common

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 1 line

Quoted 1 line

Add checks to __pa() so it goes BUG() on vmallocspace addresses.

Quoted 1 line

Sorry I think that's too costly to do. __pa is pretty common

But not too costly to do if it is done solely with vmalloc the stack for
debug purposes. The bigger problem with the vmalloc approach is there are
still offenders who DMA off the kernel stack on i386 although I'd hope they are all ancient... we'll find out with this anyway

Quoted 8 lines

(quoted diff) +8 -0

+unsigned long __kvaddr_to_paddr(unsigned long kvaddr)
+{
+	if (high_memory)
+		BUG_ON(kvaddr >= VMALLOC_START);
+	else
+		BUG_ON(kvaddr >= (unsigned long)__va(MAXMEM));
+	return kvaddr - PAGE_OFFSET;
+}

Needs to be exported so that modules can use __pa. Though I suspect most modules doing so are buggy:

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 9 lines

arch/i386/kernel/irq.c +4 -4

-static char softirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__aligned__(THREAD_SIZE)));
+static DEFINE_PER_CPU(char *, softirq_stack);
+static DEFINE_PER_CPU(char *, hardirq_stack);
 
-static char hardirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__aligned__(THREAD_SIZE)));
+#ifdef CONFIG_VMALLOC_STACK
+static void * __init __alloc_irqstack(int cpu)

How about just using DEFINE_PER_CPU and allowing the percpu code dynamically allocate.

Quoted 5 lines · William Lee Irwin III

arch/i386/kernel/irq.c +2 -2

-static char softirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__aligned__(THREAD_SIZE)));
+static DEFINE_PER_CPU(char *, softirq_stack);
+static DEFINE_PER_CPU(char *, hardirq_stack);

arch/i386/kernel/irq.c line 159

Quoted 4 lines

arch/i386/kernel/irq.c +2 -2

-static char hardirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__aligned__(THREAD_SIZE)));
+#ifdef CONFIG_VMALLOC_STACK
+static void * __init __alloc_irqstack(int cpu)

How about just using DEFINE_PER_CPU and allowing the percpu code dynamically allocate.

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 9 lines

static char softirq_stack[NR_CPUS * THREAD_SIZE]

                __attribute__((__aligned__(THREAD_SIZE)));

static char hardirq_stack[NR_CPUS * THREAD_SIZE]

                __attribute__((__aligned__(THREAD_SIZE)));

With 8K stacks and NR_CPUS==128 that would be 2MB statically reserved. Yuck. Really needs to be dynamically allocated. I'll take a look once the .22 big merge is done.

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 1 line

Here's what I did for i386 for someone concerned about blowing the stack.

If we really need it, then maybe a variable like CONFIG_LOG_BUF_SHIFT could be hacked up, resulting in CONFIG_THREAD_SIZE_SHIFT, defaulting to 3 (=8KB).

Quoted 8 lines · Andi Kleen

Actually looking at the code it would need some fixes first:
/*
* These should really be __section__(".bss.page_aligned") as well, but
* gcc's 3.0 and earlier don't handle that correctly.
*/
static char softirq_stack[NR_CPUS * THREAD_SIZE]

                __attribute__((__aligned__(THREAD_SIZE)));

Quoted 6 lines

static char hardirq_stack[NR_CPUS * THREAD_SIZE]

                __attribute__((__aligned__(THREAD_SIZE)));

With 8K stacks and NR_CPUS==128 that would be 2MB statically reserved. Yuck. Really needs to be dynamically allocated. I'll take a look once the .22 big merge is done.

Here's what I did for i386 for someone concerned about blowing the stack.

Quoted 2 lines · Adrian Bunk

4k stacks have become a well-tested feature used fore a long time in Fedora and even in RHEL 4.

So has anyone fixed the bugs involving ext3 and LVM snapshots on top of DM mirror?

Our 32-bit Fedora Core 5 dual Opteron server at work crashed once a week while running rdiff-backup off a snapshot. Installing a 64-bit kernel stopped the crashes, and I believe the 64-bit still uses the bigger stacks.

Quoted 6 lines · On Saturday April 28, [email protected] wrote:

Quoted 2 lines · Adrian Bunk

4k stacks have become a well-tested feature used fore a long time in Fedora and even in RHEL 4.

So has anyone fixed the bugs involving ext3 and LVM snapshots on top of DM mirror?

Well -mm has a patch which makes stacked block devices much less of an issue, but this hasn't made it -linus yet - I think the dm developer isn't happy that dm works properly with it.

Quoted 8 lines · Neil Brown

Quoted 6 lines · On Saturday April 28, [email protected] wrote:

Quoted 2 lines · Adrian Bunk

4k stacks have become a well-tested feature used fore a long time in Fedora and even in RHEL 4.

So has anyone fixed the bugs involving ext3 and LVM snapshots on top of DM mirror?

Quoted 3 lines

Well -mm has a patch which makes stacked block devices much less of an issue, but this hasn't made it -linus yet - I think the dm developer isn't happy that dm works properly with it.

I get a little tired about this objection. If the particular dm code was racy before and this can in theory make it worse it's their problem, and putting in perfectly fine code will give them an incentive to fix their year old race.

Quoted 13 lines · Christoph Hellwig

Quoted 11 lines · Neil Brown

Quoted 6 lines · On Saturday April 28, [email protected] wrote:

Quoted 2 lines · Adrian Bunk

4k stacks have become a well-tested feature used fore a long time in Fedora and even in RHEL 4.

So has anyone fixed the bugs involving ext3 and LVM snapshots on top of DM mirror?

Well -mm has a patch which makes stacked block devices much less of an issue, but this hasn't made it -linus yet - I think the dm developer isn't happy that dm works properly with it.

Quoted 4 lines

I get a little tired about this objection. If the particular dm code was racy before and this can in theory make it worse it's their problem, and putting in perfectly fine code will give them an incentive to fix their year old race.

Agree, Neil would you mind if I merged the patch?

Quoted 14 lines · On Monday April 30, [email protected] wrote:

Quoted 13 lines · Christoph Hellwig

Quoted 11 lines · Neil Brown

Quoted 6 lines · On Saturday April 28, [email protected] wrote:

Quoted 2 lines · Adrian Bunk

4k stacks have become a well-tested feature used fore a long time in Fedora and even in RHEL 4.

So has anyone fixed the bugs involving ext3 and LVM snapshots on top of DM mirror?

Well -mm has a patch which makes stacked block devices much less of an issue, but this hasn't made it -linus yet - I think the dm developer isn't happy that dm works properly with it.

Quoted 6 lines

Quoted 4 lines

I get a little tired about this objection. If the particular dm code was racy before and this can in theory make it worse it's their problem, and putting in perfectly fine code will give them an incentive to fix their year old race.

Agree, Neil would you mind if I merged the patch?

I am perfectly happy with this patch being merged - yes.