From: Zach Brown <zach.brown@oracle.com>
To: Denys Vlasenko <vda.linux@googlemail.com>
Cc: David Chinner <dgc@sgi.com>, Benjamin LaHaise <bcrl@kvack.org>,
xfs@oss.sgi.com, Eric Sandeen <sandeen@sandeen.net>,
Adrian Bunk <bunk@kernel.org>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-aio@kvack.org
Subject: Re: [PATCH] vfs: reduce stack usage by shrinking struct kiocb
Date: Mon, 28 Apr 2008 09:13:22 -0700 [thread overview]
Message-ID: <4815F7A2.7060605@oracle.com> (raw)
In-Reply-To: <200804270617.36629.vda.linux@googlemail.com>
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 parent reply other threads:[~2008-04-28 16:13 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-04-27 4:17 [PATCH] vfs: reduce stack usage by shrinking struct kiocb Denys Vlasenko
2008-04-28 16:13 ` Zach Brown [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4815F7A2.7060605@oracle.com \
--to=zach.brown@oracle.com \
--cc=bcrl@kvack.org \
--cc=bunk@kernel.org \
--cc=dgc@sgi.com \
--cc=linux-aio@kvack.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sandeen@sandeen.net \
--cc=vda.linux@googlemail.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.