I've got one stack overflow sometime ago.
Partially due to NFS stack usage.
2.6.9 uniprocessor i386 'make checkstack | grep nfs[34]*' results:
0xc01d012a nfs3_proc_create: 544
0xc01d541a _nfs4_do_open: 516
0xc01c3cbe nfs_readdir: 412
0xc01c53f3 nfs_symlink: 368
0xc01d4dc3 _nfs4_open_delegation_recall: 368
0xc01d05a2 nfs3_proc_rename: 364
0xc01d4b69 _nfs4_open_reclaim: 364
0xc01c4e54 nfs_mknod: 352
0xc01c4f34 nfs_mkdir: 352
0xc01cb57a nfs_proc_create: 344
0xc01d06f9 nfs3_proc_link: 328
0xc01c4217 nfs_lookup_revalidate: 312
0xc01c4733 nfs_lookup: 292
0xc01d85f1 _nfs4_do_setlk: 260
0xc01c6b65 nfs_statfs: 220
0xc01d082f nfs3_proc_symlink: 216
0xc01d0b96 nfs3_proc_readdir: 216
0xc01c62db nfs_sb_init: 212
0xc01cfa9b nfs3_proc_lookup: 212
0xc01d0cfe nfs3_proc_mknod: 208
0xc01cfc1a nfs3_proc_access: 192
0xc01d0992 nfs3_proc_mkdir: 192
0xc01cfda2 nfs3_proc_readlink: 176
0xc01d0428 nfs3_proc_remove: 176
0xc01d0aa7 nfs3_proc_rmdir: 176
0xc01d82e3 _nfs4_proc_unlck: 164
0xc05ad18a root_nfs_get_handle: 160
0xc01c796b __nfs_revalidate_inode: 152
0xc01d7c92 nfs4_proc_setclientid: 152
0xc01c72e1 nfs_setattr: 148
0xc01d7f7c _nfs4_proc_getlk: 148
0xc01d68d6 nfs4_proc_create: 144
0xc01e21e0 nfs_idmap_id: 116
0xc01e2409 nfs_idmap_name: 112
0xc01c4b27 nfs_cached_lookup: 108
Many of them are due to on-stack structures.
structs nfs_fh and nfs_fattr take together ~300 bytes
and are one of the most frequently used:
# cd fs/nfs; grep 'struct nfs_fh [a-z0-9_]*;' * ; grep 'struct nfs_fattr [a-z0-9_]*;' *
callback.h: struct nfs_fh fh;
callback.h: struct nfs_fh fh;
dir.c: struct nfs_fh fhandle;
dir.c: struct nfs_fh fhandle;
dir.c: struct nfs_fh fhandle;
dir.c: struct nfs_fh fhandle;
dir.c: struct nfs_fh sym_fh;
nfsroot.c: struct nfs_fh fh;
dir.c: struct nfs_fattr fattr;
dir.c: struct nfs_fattr fattr;
dir.c: struct nfs_fattr fattr;
dir.c: struct nfs_fattr fattr;
dir.c: struct nfs_fattr fattr;
dir.c: struct nfs_fattr sym_attr;
inode.c: struct nfs_fattr fattr;
inode.c: struct nfs_fattr fattr;
inode.c: struct nfs_fattr fattr;
nfs3proc.c: struct nfs_fattr res;
nfs4proc.c: struct nfs_fattr fattr;
I can convert these into kmalloc'ed variants but hesitate to do so
because of possible 'need to kmalloc in order to free memory for kmalloc'
deadlocks.
Any advice how to handle this without creating subtle bugs?
--
vda
On Oct 29, 2004 00:20 +0300, Denis Vlasenko wrote:
> I've got one stack overflow sometime ago.
> Partially due to NFS stack usage.
>
> structs nfs_fh and nfs_fattr take together ~300 bytes
> and are one of the most frequently used:
>
> # cd fs/nfs; grep 'struct nfs_fh [a-z0-9_]*;' * ; grep 'struct nfs_fattr [a-z0-9_]*;' *
> callback.h: struct nfs_fh fh;
> callback.h: struct nfs_fh fh;
> dir.c: struct nfs_fh fhandle;
> dir.c: struct nfs_fh fhandle;
> dir.c: struct nfs_fh fhandle;
> dir.c: struct nfs_fh fhandle;
> dir.c: struct nfs_fh sym_fh;
> nfsroot.c: struct nfs_fh fh;
> dir.c: struct nfs_fattr fattr;
> dir.c: struct nfs_fattr fattr;
> dir.c: struct nfs_fattr fattr;
> dir.c: struct nfs_fattr fattr;
> dir.c: struct nfs_fattr fattr;
> dir.c: struct nfs_fattr sym_attr;
> inode.c: struct nfs_fattr fattr;
> inode.c: struct nfs_fattr fattr;
> inode.c: struct nfs_fattr fattr;
> nfs3proc.c: struct nfs_fattr res;
> nfs4proc.c: struct nfs_fattr fattr;
>
> I can convert these into kmalloc'ed variants but hesitate to do so
> because of possible 'need to kmalloc in order to free memory for kmalloc'
> deadlocks.
Make the thread kmalloc unless it has PF_MEMALLOC set, in which case it
could get the structs from a small pool of reserved ones.
Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://members.shaw.ca/adilger/ http://members.shaw.ca/golinux/
On Fri, 2004-10-29 at 00:20 +0300, Denis Vlasenko wrote:
> I can convert these into kmalloc'ed variants but hesitate to do so
> because of possible 'need to kmalloc in order to free memory for kmalloc'
> deadlocks.
how about a memory pool?
It's not THE solution but I suspect the depth of callchains of these isn't too deep so it would work
fr den 29.10.2004 Klokka 11:01 (+0200) skreiv Arjan van de Ven:
> On Fri, 2004-10-29 at 00:20 +0300, Denis Vlasenko wrote:
>
> > I can convert these into kmalloc'ed variants but hesitate to do so
> > because of possible 'need to kmalloc in order to free memory for kmalloc'
> > deadlocks.
>
> how about a memory pool?
>
> It's not THE solution but I suspect the depth of callchains of these isn't too deep so it would work
I can't see that any of the callchains Denis listed can deadlock. None
of them appear to lie in the memory reclaim paths.
Cheers,
Trond
--
Trond Myklebust <[email protected]>
> > > I can convert these into kmalloc'ed variants but hesitate to do so
> > > because of possible 'need to kmalloc in order to free memory for kmalloc'
> > > deadlocks.
> >
> > how about a memory pool?
> >
> > It's not THE solution but I suspect the depth of callchains of these isn't too deep so it would work
>
> I can't see that any of the callchains Denis listed can deadlock. None
> of them appear to lie in the memory reclaim paths.
This patch reduces stack usage to below 100 bytes for
the following functions:
stack usage in 2.6.9
nfs3_proc_create: 544
_nfs4_do_open: 516
nfs_readdir: 412
nfs_symlink: 368
_nfs4_open_delegation_recall: 368
nfs3_proc_rename: 364
_nfs4_open_reclaim: 364
nfs_mknod: 352
nfs_mkdir: 352
nfs_proc_create: 344
nfs3_proc_link: 328
nfs_lookup_revalidate: 312
nfs_lookup: 292
(btw: in function nfs_readdir: local variable 'desc' seem to be
easily replaceable with &my_desc, or am I missing something?)
Compile tested only. I can't run test it until next Wednesday :(
Please review, especially for leaks on error paths.
--
vda
Denis Vlasenko wrote:
>>>>I can convert these into kmalloc'ed variants but hesitate to do so
>>>>because of possible 'need to kmalloc in order to free memory for kmalloc'
>>>>deadlocks.
>>>
>>>how about a memory pool?
>>>
>>>It's not THE solution but I suspect the depth of callchains of these isn't too deep so it would work
>>
>>I can't see that any of the callchains Denis listed can deadlock. None
>>of them appear to lie in the memory reclaim paths.
>
>
> This patch reduces stack usage to below 100 bytes for
> the following functions:
>
> stack usage in 2.6.9
> nfs3_proc_create: 544
> _nfs4_do_open: 516
> nfs_readdir: 412
> nfs_symlink: 368
> _nfs4_open_delegation_recall: 368
> nfs3_proc_rename: 364
> _nfs4_open_reclaim: 364
> nfs_mknod: 352
> nfs_mkdir: 352
> nfs_proc_create: 344
> nfs3_proc_link: 328
> nfs_lookup_revalidate: 312
> nfs_lookup: 292
>
> (btw: in function nfs_readdir: local variable 'desc' seem to be
> easily replaceable with &my_desc, or am I missing something?)
>
> Compile tested only. I can't run test it until next Wednesday :(
>
> Please review, especially for leaks on error paths.
Hi Denis,
I checked all of it. Looks right & it builds.
--
~Randy
On Saturday 30 October 2004 06:01, Randy.Dunlap wrote:
> > This patch reduces stack usage to below 100 bytes for
> > the following functions:
> >
> > stack usage in 2.6.9
> > nfs3_proc_create: 544
> > _nfs4_do_open: 516
> > nfs_readdir: 412
> > nfs_symlink: 368
> > _nfs4_open_delegation_recall: 368
> > nfs3_proc_rename: 364
> > _nfs4_open_reclaim: 364
> > nfs_mknod: 352
> > nfs_mkdir: 352
> > nfs_proc_create: 344
> > nfs3_proc_link: 328
> > nfs_lookup_revalidate: 312
> > nfs_lookup: 292
> >
> > (btw: in function nfs_readdir: local variable 'desc' seem to be
> > easily replaceable with &my_desc, or am I missing something?)
> >
> > Compile tested only. I can't run test it until next Wednesday :(
> >
> > Please review, especially for leaks on error paths.
>
> Hi Denis,
> I checked all of it. Looks right & it builds.
Thanks!
Small cleanup on top of previous: nfs_readdir():
* kill local variable 'desc' bacause loc->my_desc
evaluates to the same lvalue now, we can use it instead
* avoid checking for res<0 when we know that it is true
--
vda
lau den 30.10.2004 Klokka 00:59 (+0300) skreiv Denis Vlasenko:
> This patch reduces stack usage to below 100 bytes for
> the following functions:
...
> Please review, especially for leaks on error paths.
Looks alright, apart from the ENOMEM return for nfs_lookup_revalidate()
that I already pointed out to you.
Cheers,
Trond
--
Trond Myklebust <[email protected]>