From: William Lee Irwin III <wli@holomorphy.com>
To: linux-kernel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org, netdev@oss.sgi.com, akpm@osdl.org,
davem@redhat.com
Subject: kiocb->private is too large for kiocb's on-stack
Date: Mon, 28 Jun 2004 01:08:01 -0700 [thread overview]
Message-ID: <20040628080801.GO21066@holomorphy.com> (raw)
sizeof(struct kiocb) is dangerously large for a structure commonly
allocated on-stack. This patch converts the 24*sizeof(long) field,
->private, to a void pointer for use by file_operations entrypoints.
A ->dtor() method is added to the kiocb in order to support the release
of dynamically allocated structures referred to by ->private.
The sole in-tree users of ->private are async network read/write,
which are not, in fact, async, and so need not handle preallocated
->private as they would need to if ->ki_retry were ever used. The sole
truly async operations are direct IO pread()/pwrite() which do not
now use ->ki_retry(). All they would need to do in that case is to
check for ->private already being allocated for async kiocbs.
This rips 88B off the stack on 32-bit in the common case.
vs. 2.6.7-mm3
Index: mm3-2.6.7/include/net/sock.h
===================================================================
--- mm3-2.6.7.orig/include/net/sock.h 2004-06-27 15:44:42.365168400 -0700
+++ mm3-2.6.7/include/net/sock.h 2004-06-27 15:50:40.651700568 -0700
@@ -617,17 +617,17 @@
struct scm_cookie *scm;
struct msghdr *msg, async_msg;
struct iovec async_iov;
+ struct kiocb *kiocb;
};
static inline struct sock_iocb *kiocb_to_siocb(struct kiocb *iocb)
{
- BUG_ON(sizeof(struct sock_iocb) > KIOCB_PRIVATE_SIZE);
return (struct sock_iocb *)iocb->private;
}
static inline struct kiocb *siocb_to_kiocb(struct sock_iocb *si)
{
- return container_of((void *)si, struct kiocb, private);
+ return si->kiocb;
}
struct socket_alloc {
Index: mm3-2.6.7/include/linux/aio.h
===================================================================
--- mm3-2.6.7.orig/include/linux/aio.h 2004-06-15 22:18:54.000000000 -0700
+++ mm3-2.6.7/include/linux/aio.h 2004-06-27 15:50:40.654700112 -0700
@@ -23,8 +23,6 @@
#define KIOCB_SYNC_KEY (~0U)
-#define KIOCB_PRIVATE_SIZE (24 * sizeof(long))
-
/* ki_flags bits */
#define KIF_LOCKED 0
#define KIF_KICKED 1
@@ -55,6 +53,7 @@
struct kioctx *ki_ctx; /* may be NULL for sync ops */
int (*ki_cancel)(struct kiocb *, struct io_event *);
long (*ki_retry)(struct kiocb *);
+ void (*ki_dtor)(struct kiocb *);
struct list_head ki_list; /* the aio core uses this
* for cancellation */
@@ -65,8 +64,7 @@
} ki_obj;
__u64 ki_user_data; /* user's data for completion */
loff_t ki_pos;
-
- char private[KIOCB_PRIVATE_SIZE];
+ void *private;
};
#define is_sync_kiocb(iocb) ((iocb)->ki_key == KIOCB_SYNC_KEY)
@@ -79,6 +77,7 @@
(x)->ki_filp = (filp); \
(x)->ki_ctx = &tsk->active_mm->default_kioctx; \
(x)->ki_cancel = NULL; \
+ (x)->ki_dtor = NULL; \
(x)->ki_obj.tsk = tsk; \
} while (0)
Index: mm3-2.6.7/net/socket.c
===================================================================
--- mm3-2.6.7.orig/net/socket.c 2004-06-15 22:19:13.000000000 -0700
+++ mm3-2.6.7/net/socket.c 2004-06-27 15:50:40.662698896 -0700
@@ -548,9 +548,11 @@
int sock_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct kiocb iocb;
+ struct sock_iocb siocb;
int ret;
init_sync_kiocb(&iocb, NULL);
+ iocb.private = &siocb;
ret = __sock_sendmsg(&iocb, sock, msg, size);
if (-EIOCBQUEUED == ret)
ret = wait_on_sync_kiocb(&iocb);
@@ -581,15 +583,22 @@
size_t size, int flags)
{
struct kiocb iocb;
+ struct sock_iocb siocb;
int ret;
init_sync_kiocb(&iocb, NULL);
+ iocb.private = &siocb;
ret = __sock_recvmsg(&iocb, sock, msg, size, flags);
if (-EIOCBQUEUED == ret)
ret = wait_on_sync_kiocb(&iocb);
return ret;
}
+static void sock_aio_dtor(struct kiocb *iocb)
+{
+ kfree(iocb->private);
+}
+
/*
* Read data from a socket. ubuf is a user mode pointer. We make sure the user
* area ubuf...ubuf+size-1 is writable before asking the protocol.
@@ -598,7 +607,7 @@
static ssize_t sock_aio_read(struct kiocb *iocb, char __user *ubuf,
size_t size, loff_t pos)
{
- struct sock_iocb *x = kiocb_to_siocb(iocb);
+ struct sock_iocb *x, siocb;
struct socket *sock;
int flags;
@@ -607,6 +616,16 @@
if (size==0) /* Match SYS5 behaviour */
return 0;
+ if (is_sync_kiocb(iocb))
+ x = &siocb;
+ else {
+ x = kmalloc(sizeof(struct sock_iocb), GFP_KERNEL);
+ if (!x)
+ return -ENOMEM;
+ iocb->ki_dtor = sock_aio_dtor;
+ }
+ iocb->private = x;
+ x->kiocb = iocb;
sock = SOCKET_I(iocb->ki_filp->f_dentry->d_inode);
x->async_msg.msg_name = NULL;
@@ -631,7 +650,7 @@
static ssize_t sock_aio_write(struct kiocb *iocb, const char __user *ubuf,
size_t size, loff_t pos)
{
- struct sock_iocb *x = kiocb_to_siocb(iocb);
+ struct sock_iocb *x, siocb;
struct socket *sock;
if (pos != 0)
@@ -639,6 +658,16 @@
if(size==0) /* Match SYS5 behaviour */
return 0;
+ if (is_sync_kiocb(iocb))
+ x = &siocb;
+ else {
+ x = kmalloc(sizeof(struct sock_iocb), GFP_KERNEL);
+ if (!x)
+ return -ENOMEM;
+ iocb->ki_dtor = sock_aio_dtor;
+ }
+ iocb->private = x;
+ x->kiocb = iocb;
sock = SOCKET_I(iocb->ki_filp->f_dentry->d_inode);
x->async_msg.msg_name = NULL;
Index: mm3-2.6.7/fs/aio.c
===================================================================
--- mm3-2.6.7.orig/fs/aio.c 2004-06-27 15:44:54.278357320 -0700
+++ mm3-2.6.7/fs/aio.c 2004-06-27 15:50:40.667698136 -0700
@@ -396,6 +396,8 @@
req->ki_cancel = NULL;
req->ki_retry = NULL;
req->ki_obj.user = NULL;
+ req->ki_dtor = NULL;
+ req->private = NULL;
/* Check if the completion queue has enough free space to
* accept an event from this io.
@@ -436,9 +438,13 @@
static inline void really_put_req(struct kioctx *ctx, struct kiocb *req)
{
+ if (req->ki_dtor)
+ req->ki_dtor(req);
req->ki_ctx = NULL;
req->ki_filp = NULL;
req->ki_obj.user = NULL;
+ req->ki_dtor = NULL;
+ req->private = NULL;
kmem_cache_free(kiocb_cachep, req);
ctx->reqs_active--;
next reply other threads:[~2004-06-28 8:08 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-06-28 8:08 William Lee Irwin III [this message]
2004-06-28 8:12 ` kiocb->private is too large for kiocb's on-stack Andrew Morton
2004-06-28 8:20 ` William Lee Irwin III
2004-06-28 18:25 ` David S. Miller
2004-06-29 2:02 ` William Lee Irwin III
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=20040628080801.GO21066@holomorphy.com \
--to=wli@holomorphy.com \
--cc=akpm@osdl.org \
--cc=davem@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@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.