[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LFD.0.999.0707311555410.4161@woody.linux-foundation.org>
Date: Tue, 31 Jul 2007 15:59:45 -0700 (PDT)
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Andrew Morton <akpm@...ux-foundation.org>
cc: Badari Pulavarty <pbadari@...ibm.com>,
Joe Jin <joe.jin@...cle.com>,
Zach Brown <zach.brown@...cle.com>,
gurudas pai <gurudas.pai@...cle.com>, jens.axboe@...cle.com,
lkml <linux-kernel@...r.kernel.org>, wen.gang.wang@...cle.com
Subject: Re: [PATCH] add check do_direct_IO() return val
On Tue, 31 Jul 2007, Andrew Morton wrote:
>
> We zero out so many fields in there now that a kzalloc() might yield
> a net gain. 0.000001% in an unnamed benchmark!
Indeed. That's *especially* true since right now it passes a mostly
totally uninitialized structure pointer down to direct_io_worker(), but it
has actually initialized a _few_ fields. Ie the "die" pointer is almost
totally filled with random crap, except for two members: dio->lock_type
and dio->is_async have been initialized the rest is random crud.
That kind of mixing of totally-but-not-entirely-uninitialized pointer
passing is just a recipe for disaster.
Does a patch like this work? I don't have any test-cases, but it would be
good to have something like this tested and passed back with proper
explanations and sign-offs.
Linus
---
fs/direct-io.c | 17 +----------------
1 files changed, 1 insertions(+), 16 deletions(-)
diff --git a/fs/direct-io.c b/fs/direct-io.c
index 52bb263..901dc55 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -958,35 +958,22 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
ssize_t ret2;
size_t bytes;
- dio->bio = NULL;
dio->inode = inode;
dio->rw = rw;
dio->blkbits = blkbits;
dio->blkfactor = inode->i_blkbits - blkbits;
- dio->start_zero_done = 0;
- dio->size = 0;
dio->block_in_file = offset >> blkbits;
- dio->blocks_available = 0;
- dio->cur_page = NULL;
- dio->boundary = 0;
- dio->reap_counter = 0;
dio->get_block = get_block;
dio->end_io = end_io;
- dio->map_bh.b_private = NULL;
dio->final_block_in_bio = -1;
dio->next_block_for_io = -1;
- dio->page_errors = 0;
- dio->io_error = 0;
- dio->result = 0;
dio->iocb = iocb;
dio->i_size = i_size_read(inode);
spin_lock_init(&dio->bio_lock);
dio->refcount = 1;
- dio->bio_list = NULL;
- dio->waiter = NULL;
/*
* In case of non-aligned buffers, we may need 2 more
@@ -994,8 +981,6 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
*/
if (unlikely(dio->blkfactor))
dio->pages_in_io = 2;
- else
- dio->pages_in_io = 0;
for (seg = 0; seg < nr_segs; seg++) {
user_addr = (unsigned long)iov[seg].iov_base;
@@ -1183,7 +1168,7 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
}
}
- dio = kmalloc(sizeof(*dio), GFP_KERNEL);
+ dio = kzalloc(sizeof(*dio), GFP_KERNEL);
retval = -ENOMEM;
if (!dio)
goto out;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists