lkml.org 
[lkml]   [2005]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [2.6 patch] i386: always use 4k stacks
    On Thursday December 15, davej@redhat.com 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@suse.de>

    ### 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;
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2005-12-16 04:00    [W:2.385 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site