On Thursday December 15, davej@xxxxxxxxxx wrote:
> On Fri, Dec 16, 2005 at 01:47:40AM +0100, Adrian Bunk wrote:
>
> > > [*] Plus a few XFS ones, but that's been a lost cause wrt stack usage
> > > for a long time -- people were reporting overflows there before we
> > > enabled 4K stacks.
> >
> > I remember someone from the XFS maintainers (Nathan?) saying they
> > believe having solved all XFS stack issues.
> >
> > If there are any XFS issues left, do you have a pointer to them?
>
> The last one I saw may have been actually been more related
> to the block layer problem. iirc that was a user NFS exporting
> XFS on a raid1 array.
Yeh, I've noticed that nfsd seems to figure often in these. As nfsd
lives on the same (in-kernel) stack as the filesystem and device
drives, it will add a couple of hundred bytes to the call trace.
A typical nfsd call trace is
nfsd -> svc_process -> nfsd_dispatch -> nfsd3_proc_write ->
nfsd_write ->nfsd_vfs_write -> vfs_writev
(errr. nfsd_vfs_write is inline, large, and called twice, that ain't
good)
These add up to over 300 bytes on the stack.
Looking at each of these, I see that nfsd_write (which includes
nfsd_vfs_write) contributes 0x8c to stack usage itself!!
It turns out this is because it puts a 'struct iattr' on the stack so
it can kill suid if needed. The following patch saves about 50 bytes
off the stack in this call path.
I sometimes wish that gcc could be told to optimise for stack usage -
a lot of variables on the stack are dead at some call points, yet they
stay there using space anyway. The only way to save this space seem
to be to move the code which uses those variable into a separate
function, but we really shouldn't *have* to do these optimisations by
hand!
NeilBrown
Signed-off-by: Neil Brown <neilb@xxxxxxx>
### Diffstat output
./fs/nfsd/vfs.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff ./fs/nfsd/vfs.c~current~ ./fs/nfsd/vfs.c
--- ./fs/nfsd/vfs.c~current~ 2005-12-12 16:00:40.000000000 +1100
+++ ./fs/nfsd/vfs.c 2005-12-16 13:48:31.000000000 +1100
@@ -869,6 +869,16 @@ out:
return err;
}
+static void kill_suid(struct dentry *dentry)
+{
+ struct iattr ia;
+ ia.ia_valid = ATTR_KILL_SUID | ATTR_KILL_SGID;
+
+ down(&dentry->d_inode->i_sem);
+ notify_change(dentry, &ia);
+ up(&dentry->d_inode->i_sem);
+}
+
static inline int
nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
loff_t offset, struct kvec *vec, int vlen,
@@ -922,14 +932,8 @@ nfsd_vfs_write(struct svc_rqst *rqstp, s
}
/* clear setuid/setgid flag after write */
- if (err >= 0 && (inode->i_mode & (S_ISUID | S_ISGID))) {
- struct iattr ia;
- ia.ia_valid = ATTR_KILL_SUID | ATTR_KILL_SGID;
-
- down(&inode->i_sem);
- notify_change(dentry, &ia);
- up(&inode->i_sem);
- }
+ if (err >= 0 && (inode->i_mode & (S_ISUID | S_ISGID)))
+ kill_suid(dentry);
if (err >= 0 && stable) {
static ino_t last_ino;
|