From: Mikulas Patocka on
On Tue, 17 Jun 2008, David Miller wrote:

> From: Mikulas Patocka <mpatocka(a)redhat.com>
> Date: Tue, 17 Jun 2008 20:47:57 -0400 (EDT)
>
>> Wait queue waking looks like being written by a high-level maniac --- it
>> contains 8 levels of calls (none of them inlined). 7 of these calls (until
>> try_to_wake_up) do nothing but pass arguments to lower level call. And
>> each of these calls allocate at least 192 bytes of stack space. All these
>> 7 useless calls consume 1360 bytes of stack (and cause windows traps that
>> needlessly damage performance). Would you agree to inline most of the
>> calls to save stack? Or do you see another solution?
>
> Some of them could be inlined but there are a few limiting
> factors here.

I inlined three of them, I think I can inline another two. So hopefully,
I'll be able to shring 8-call depth to 3-call depth.

> Even spin lock acquisitions are function calls, limiting how
> much leaf function and tail call optimizations can be done.

Tail call optimization is not done at all if you compile kernel with stack
checking. This contributes to the stack overflow too.

> Also, wake_up_bit has this aggregate local variable "key" whose
> address is passed down to subsequent functions, which limits
> optimizations even further.
>
> It could still be improved a lot, however.
>
> But the level of recursion possible by the current device layer is
> excessive and needs to be curtained irrespective of these generic
> wakeup and sparc64 interrupt stack issues.

I fixed that too.

BTW. what's the purpose of having 192-byte stack frame? There are 16
8-byte registers being saved per function call, so 128-byte frame should
be sufficient, shoudn't? The ABI specifies that some additional entries
must be present even if unused, but I don't see reason for them. Would
something bad happen if GCC started to generate 128-byte stacks?

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on
From: Mikulas Patocka <mpatocka(a)redhat.com>
Date: Wed, 18 Jun 2008 23:24:20 -0400 (EDT)

> BTW. what's the purpose of having 192-byte stack frame? There are 16
> 8-byte registers being saved per function call, so 128-byte frame should
> be sufficient, shoudn't? The ABI specifies that some additional entries
> must be present even if unused, but I don't see reason for them. Would
> something bad happen if GCC started to generate 128-byte stacks?

The callee can pop the arguments into the area past the
register window.

So you have the 128 byte register window save area, 6
slots for incoming arguments, which gives us 176 bytes.
The rest is for some miscellaneous stack frame state,
which I don't remember the details of at the moment.
I'd have to read the sparc backend of gcc to remember.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mikulas Patocka on
On Wed, 18 Jun 2008, David Miller wrote:

> From: Mikulas Patocka <mpatocka(a)redhat.com>
> Date: Wed, 18 Jun 2008 23:24:20 -0400 (EDT)
>
>> BTW. what's the purpose of having 192-byte stack frame? There are 16
>> 8-byte registers being saved per function call, so 128-byte frame should
>> be sufficient, shoudn't? The ABI specifies that some additional entries
>> must be present even if unused, but I don't see reason for them. Would
>> something bad happen if GCC started to generate 128-byte stacks?
>
> The callee can pop the arguments into the area past the
> register window.

I see ... the callee writes arguments into caller's stack frame, if it has
variable number of arguments. That it misdesign, the callee should write
registers arguments into it's own frame like on AMD64 (then this space
would be allocated only if needed).
But nothing can be done with it since ABI was specified :-(

Mikulas

> So you have the 128 byte register window save area, 6
> slots for incoming arguments, which gives us 176 bytes.
> The rest is for some miscellaneous stack frame state,
> which I don't remember the details of at the moment.
> I'd have to read the sparc backend of gcc to remember.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on
From: Mikulas Patocka <mpatocka(a)redhat.com>
Date: Thu, 19 Jun 2008 01:17:39 -0400 (EDT)

> I see ... the callee writes arguments into caller's stack frame, if it has
> variable number of arguments. That it misdesign, the callee should write
> registers arguments into it's own frame like on AMD64 (then this space
> would be allocated only if needed).

The callee can do this even for non-variable argument lists.

It's like a set of pre-allocated stack slots for those incoming
argument registers when reloading under register pressure.

In my opinion it is better to put this onus on the callee because only
the callee knows if it needs to pop these values onto the stack to
alleviate register pressure.

I think it might be possible for the compiler to only use 176 bytes.
I'll take a look at the gcc sparc backend and the ABI specification
to see if this is the case.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mikulas Patocka on
On Wed, 18 Jun 2008, David Miller wrote:

> From: Mikulas Patocka <mpatocka(a)redhat.com>
> Date: Thu, 19 Jun 2008 01:17:39 -0400 (EDT)
>
>> I see ... the callee writes arguments into caller's stack frame, if it has
>> variable number of arguments. That it misdesign, the callee should write
>> registers arguments into it's own frame like on AMD64 (then this space
>> would be allocated only if needed).
>
> The callee can do this even for non-variable argument lists.
>
> It's like a set of pre-allocated stack slots for those incoming
> argument registers when reloading under register pressure.
>
> In my opinion it is better to put this onus on the callee because only
> the callee knows if it needs to pop these values onto the stack to
> alleviate register pressure.
>
> I think it might be possible for the compiler to only use 176 bytes.
> I'll take a look at the gcc sparc backend and the ABI specification
> to see if this is the case.

Yes, it could be shrunk to 176 bytes. Maybe there could be some
performance problems if the spills are cacheline-unaligned. Or better ---
make special -mkernel-abi function to gcc that will drop this area at all
and make 128-byte frames. In kernel it wouldn't matter that ABI is
incompatible.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/