xfs
[Top] [All Lists]

Re: [PATCH] vfs: reduce stack usage by shrinking struct kiocb

To: Denys Vlasenko <vda.linux@xxxxxxxxxxxxxx>
Subject: Re: [PATCH] vfs: reduce stack usage by shrinking struct kiocb
From: Zach Brown <zach.brown@xxxxxxxxxx>
Date: Mon, 28 Apr 2008 09:13:22 -0700
Cc: David Chinner <dgc@xxxxxxx>, Benjamin LaHaise <bcrl@xxxxxxxxx>, xfs@xxxxxxxxxxx, Eric Sandeen <sandeen@xxxxxxxxxxx>, Adrian Bunk <bunk@xxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-aio@xxxxxxxxx
In-reply-to: <200804270617.36629.vda.linux@xxxxxxxxxxxxxx>
References: <200804270617.36629.vda.linux@xxxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.12 (Macintosh/20080213)
Denys Vlasenko wrote:
> Hi Al, Benjamin, David,
> 
> struct kiocb is placed on stack by, for example, do_sync_write().
> Eventually it contributes to xfs writeout path's stack usage, among others.
> This is *the* path which causes 4k stack overflows on i386 with xfs.
> 
> This patch trivially reorders fields of this structure,
> and makes some of them smaller.

Great, thanks for doing this.  I see one fatal bug, though.  Can you fix
it up and resubmit?

-       unsigned long           ki_flags;
+       unsigned short          ki_flags; /* range: 0..2 */

Careful, ki_flags is used by the bitops routines which require an
unsigned long.  I'd just leave ki_flags as is.

> ki_nr_segs: ulong -> uint: nobody uses 4 billion element writev's
>                            (and it would not work anyway)

Indeed.  Maybe explicitly mention that it's safe 'cause we pass
ki_nbytes to rw_copy_check_uvector() for comparison against UIO_MAXIOV
before we store it in the kiocb.

+       /*unsigned short        ki_opcode; - moved up for denser packing */

Don't bother commenting out fields that are moved, just move 'em.

Otherwise it looks great, thanks.

- z


<Prev in Thread] Current Thread [Next in Thread>