Re: [PATCH] add check do_direct_IO() return val

From: Linus Torvalds
Date: Tue Jul 31 2007 - 19:00:58 EST




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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/