Hi Marcelo,
I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
I made 6 different patches, one for each area to pick and choose easily.
Here are the stack reductions for each these:
do_execve 320
number 100
nfs_lookup 184
__revalidate_inode 112
rpc_call_sync 144
xprt_sendmsg 120
If you have any suggestions, more cleanups or re-works, please let me
know.
Thanks,
Badari
Badari Pulavarty wrote:
> + /* Move these off of the stack for number(). This way we reduce the
> + * size of the stack and don't have to copy them every time we are called.
> + */
> +const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> +const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> +
> static char * number(char * buf, char * end, long long num, int base, int size, int precision, int type)
> {
> char c,sign,tmp[66];
> const char *digits;
> - static const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> - static const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
Is this actually correct? Since these are declared "static const", they
are not on the stack anyway, because they have to persist between calls
to this function and cannot be changed. I'd be very surprised if the
compiler was copying this data from the static data segment to the stack
on every entry to this function.
Greg-KH told me that, this patch is useless. He told me that,
this won't save any stack since "static constants" won't be
on the stack. I will take his word for it :)
Please ignore this patch.
Thanks,
Badari
Badari Pulavarty wrote:
>
> ------------------------------------------------------------------------
>
> Signed-off-by: Badari Pulavarty <[email protected]>
> --- linux-2.4.29-rc1.org/lib/vsprintf.c 2004-02-18 05:36:32.000000000 -0800
> +++ linux-2.4.29-rc1/lib/vsprintf.c 2005-01-07 07:56:00.000000000 -0800
> @@ -128,12 +128,16 @@ static int skip_atoi(const char **s)
> #define SPECIAL 32 /* 0x */
> #define LARGE 64 /* use 'ABCDEF' instead of 'abcdef' */
>
> + /* Move these off of the stack for number(). This way we reduce the
> + * size of the stack and don't have to copy them every time we are called.
> + */
> +const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> +const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> +
> static char * number(char * buf, char * end, long long num, int base, int size, int precision, int type)
> {
> char c,sign,tmp[66];
> const char *digits;
> - static const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> - static const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> int i;
>
> digits = (type & LARGE) ? large_digits : small_digits;
Yep. My bad. I got little carried away...
But I remember seeing this on earlier distros..
Thanks,
Badari
Kevin P. Fleming wrote:
> Badari Pulavarty wrote:
>
>> + /* Move these off of the stack for number(). This way we reduce the
>> + * size of the stack and don't have to copy them every time we are
>> called.
>> + */
>> +const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
>> +const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
>> +
>> static char * number(char * buf, char * end, long long num, int base,
>> int size, int precision, int type)
>> {
>> char c,sign,tmp[66];
>> const char *digits;
>> - static const char small_digits[] =
>> "0123456789abcdefghijklmnopqrstuvwxyz";
>> - static const char large_digits[] =
>> "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
>
>
> Is this actually correct? Since these are declared "static const", they
> are not on the stack anyway, because they have to persist between calls
> to this function and cannot be changed. I'd be very surprised if the
> compiler was copying this data from the static data segment to the stack
> on every entry to this function.
> -
> 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/
>
>
On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> Hi Marcelo,
>
> I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
is it really worth doing this sort of thing for 2.4 still? It's a matter
of risk versus gain... not sure this sort of thing is still worth it in
the deep-maintenance 2.4 tree
On Tue, Jan 11, 2005 at 08:39:03AM +0100, Arjan van de Ven wrote:
> On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> > Hi Marcelo,
> >
> > I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
>
> is it really worth doing this sort of thing for 2.4 still? It's a matter
> of risk versus gain... not sure this sort of thing is still worth it in
> the deep-maintenance 2.4 tree
Well it seems the s390 fellows are seeing stack overflows, which are serious
enough. Have you noticed that?
Moreover this are simple changes, not complex ones.
On Tue, 2005-01-11 at 05:49 -0200, Marcelo Tosatti wrote:
> On Tue, Jan 11, 2005 at 08:39:03AM +0100, Arjan van de Ven wrote:
> > On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> > > Hi Marcelo,
> > >
> > > I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
> >
> > is it really worth doing this sort of thing for 2.4 still? It's a matter
> > of risk versus gain... not sure this sort of thing is still worth it in
> > the deep-maintenance 2.4 tree
>
> Well it seems the s390 fellows are seeing stack overflows, which are serious
> enough. Have you noticed that?
well.. is anyone using 2.4.2X mainline on s390, or is ibm making their
s390 customers use vendor kernels instead?
(the people brave enough to not use those kernels might very well be
using 2.6 by now)
Just trying to get a feeling for who if anyone will benefit inclusion of
such patches, because if that is "just about nobody" then they might
well not be worth the risk.
On Tue, Jan 11, 2005 at 12:18:31PM +0100, Arjan van de Ven wrote:
> On Tue, 2005-01-11 at 05:49 -0200, Marcelo Tosatti wrote:
> > On Tue, Jan 11, 2005 at 08:39:03AM +0100, Arjan van de Ven wrote:
> > > On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> > > > Hi Marcelo,
> > > >
> > > > I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
> > >
> > > is it really worth doing this sort of thing for 2.4 still? It's a matter
> > > of risk versus gain... not sure this sort of thing is still worth it in
> > > the deep-maintenance 2.4 tree
> >
> > Well it seems the s390 fellows are seeing stack overflows, which are serious
> > enough. Have you noticed that?
>
> well.. is anyone using 2.4.2X mainline on s390, or is ibm making their
> s390 customers use vendor kernels instead?
> (the people brave enough to not use those kernels might very well be
> using 2.6 by now)
>
> Just trying to get a feeling for who if anyone will benefit inclusion of
> such patches, because if that is "just about nobody" then they might
> well not be worth the risk.
I understand your concern and appreciate it.
I dont expect anyone to be using v2.4 mainline on S390 (you need external
patches to get it to work anyway) either :)
But the stack growth patches are also useful for other architectures I assume,
its pretty hard to get them wrong (ie you're simple changing stack to
kmalloc()'ed memory, the code is essentially the same).
No?