linux-ext4 - Re: [PATCH 1/2] code clean up for dio fallocate handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <1255050229.4931.146.camel@mingming-laptop>
Date:	Thu, 08 Oct 2009 18:03:49 -0700
From:	Mingming <cmm@...ibm.com>
To:	tytso@....edu
Cc:	ext4 development <linux-ext4@...r.kernel.org>,
	Curt Wohlgemuth <curtw@...gle.com>
Subject: Re: [PATCH 1/2] code clean up for dio fallocate handling

On Wed, 2009-10-07 at 10:19 -0700, Mingming wrote:
> ext4: code clean up for dio fallocate handling
> 
> In the non async IO direct IO case, the io_end structure could be NULL.
> The ext4_debug() call in ext4_end_io_dio() (inode.c) should be moved
> after checking the io_end structure to be a non NULL pointer.
> 
> The comment above ext4_get_block_dio_write() ("Maximum
> number of blocks...") is a duplicate; the original and correct comment
> is above the #define DIO_MAX_BLOCKS up above.
> 
> The check for allocated > max_blocks in ext4_split_unwritten_extents()
> can be removed, since the code returns immediately once allocated blocks
> is less or equals to the requested blocks to convert.
> 
> Based on review comments from Curt Wohlgemuth.
> 
> Signed-off-by:	Mingming Cao <cmm@...ibm.com>

Updated cleanup patch after [patch 2/2 Fix return value of splitting
extents for dio write over fallocate ]fixed another bug. We still need
the check for allocated > max_blocks in ext4_split_unwritten_extents().

I'll resubmit the updated patch series (including two other bug
fixes/clean ups sent earlier this week) to the list again to avoid
confusion.

Thanks,
Mingming


ext4: code clean up for dio fallocate handling

The ext4_debug() call in ext4_end_io_dio() (inode.c) has a
should be moved after checking the io_end structure to be not a NULL pointer.

The comment above ext4_get_block_dio_write() ("Maximum
number of blocks...") is a duplicate; the original and correct comment
is above the #define DIO_MAX_BLOCKS up above.

The check for allocated > max_blocks in ext4_split_unwritten_extents()
can be removed, since the code returns immediately once allocated blocks is
less or equals to the requested blocks to convert.

Based on review comments from Curt Wohlgemuth.

Signed-off-by:	Mingming Cao <cmm@...ibm.com>
---
 fs/ext4/extents.c |   96 +++++++++++++++++++++++++-----------------------------
 fs/ext4/inode.c   |    9 ++---
 2 files changed, 50 insertions(+), 55 deletions(-)

Index: linux-2.6.31-rc4/fs/ext4/inode.c
===================================================================
--- linux-2.6.31-rc4.orig/fs/ext4/inode.c
+++ linux-2.6.31-rc4/fs/ext4/inode.c
@@ -3367,8 +3367,6 @@ out:
 	return ret;
 }
 
-/* Maximum number of blocks we map for direct IO at once. */
-
 static int ext4_get_block_dio_write(struct inode *inode, sector_t iblock,
 		   struct buffer_head *bh_result, int create)
 {
@@ -3576,13 +3574,14 @@ static void ext4_end_io_dio(struct kiocb
         ext4_io_end_t *io_end = iocb->private;
 	struct workqueue_struct *wq;
 
+	/* if not async direct IO or dio with 0 bytes write, just return */
+	if (!io_end || !size)
+		return;
+
 	ext_debug("ext4_end_io_dio(): io_end 0x%p"
 		  "for inode %lu, iocb 0x%p, offset %llu, size %llu\n",
  		  iocb->private, io_end->inode->i_ino, iocb, offset,
 		  size);
-	/* if not async direct IO or dio with 0 bytes write, just return */
-	if (!io_end || !size)
-		return;
 
 	/* if not aio dio with unwritten extents, just free io and return */
 	if (io_end->flag != DIO_AIO_UNWRITTEN){


--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html