linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Fan Li <fanofcode.li@samsung.com>
To: linux-fsdevel@vger.kernel.org
Subject: [PATCH] fs: fix bugs for __generic_block_fiemap()
Date: Sat, 09 Jan 2016 16:50:34 +0800	[thread overview]
Message-ID: <001401d14aba$dfe6c370$9fb44a50$@samsung.com> (raw)

Fix 3 bugs:
1. If there are more than two blocks of holes after the last
   extent of file, it would fail to add FIEMAP_EXTENT_LAST
   to the last extent.
2. len hasn't been updated correctly, if len > isize and
   start > 0.
3. If len is less than one block, it will be extended to
   one block. If start + len exceeds the boundary of the 
   original block because of the extension, one extra block 
   will be returned.

And simplify the codes of __generic_block_fiemap() as well.

Signed-off-by: Fan Li <fanofcode.li@samsung.com>
---
 fs/ioctl.c |  116 +++++++++++++++---------------------------------------------
 1 file changed, 29 insertions(+), 87 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 41c352e..8e2b426 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -256,7 +256,6 @@ int __generic_block_fiemap(struct inode *inode,
 	loff_t isize = i_size_read(inode);
 	u64 logical = 0, phys = 0, size = 0;
 	u32 flags = FIEMAP_EXTENT_MERGED;
-	bool past_eof = false, whole_file = false;
 	int ret = 0;
 
 	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
@@ -268,28 +267,27 @@ int __generic_block_fiemap(struct inode *inode,
 	 * since we expect isize to not change at all through the duration of
 	 * this call.
 	 */
-	if (len >= isize) {
-		whole_file = true;
-		len = isize;
-	}
+	if (start >= isize)
+		return 0;
 
-	/*
-	 * Some filesystems can't deal with being asked to map less than
-	 * blocksize, so make sure our len is at least block length.
-	 */
-	if (logical_to_blk(inode, len) == 0)
-		len = blk_to_logical(inode, 1);
+	if (start + len > isize)
+		len = isize - start;
 
 	start_blk = logical_to_blk(inode, start);
 	last_blk = logical_to_blk(inode, start + len - 1);
 
 	do {
+		memset(&map_bh, 0, sizeof(struct buffer_head));
 		/*
-		 * we set b_size to the total size we want so it will map as
-		 * many contiguous blocks as possible at once
+		 * Some filesystems would round down b_size to align
+		 * with block size, if len isn't aligned already, the last
+		 * block may not be returned. Let's round it up first.
 		 */
-		memset(&map_bh, 0, sizeof(struct buffer_head));
-		map_bh.b_size = len;
+		if (last_blk > start_blk)
+			map_bh.b_size = blk_to_logical(inode,
+						last_blk - start_blk + 1);
+		else
+			map_bh.b_size = blk_to_logical(inode, 1);
 
 		ret = get_block(inode, start_blk, &map_bh, 0);
 		if (ret)
@@ -299,91 +297,35 @@ int __generic_block_fiemap(struct inode *inode,
 		if (!buffer_mapped(&map_bh)) {
 			start_blk++;
 
-			/*
-			 * We want to handle the case where there is an
-			 * allocated block at the front of the file, and then
-			 * nothing but holes up to the end of the file properly,
-			 * to make sure that extent at the front gets properly
-			 * marked with FIEMAP_EXTENT_LAST
-			 */
-			if (!past_eof &&
-			    blk_to_logical(inode, start_blk) >= isize)
-				past_eof = 1;
-
+			/* Skip holes unless it indicates the EOF */
+			if (blk_to_logical(inode, start_blk) < isize)
+				goto next;
 			/*
 			 * First hole after going past the EOF, this is our
 			 * last extent
 			 */
-			if (past_eof && size) {
-				flags = FIEMAP_EXTENT_MERGED|FIEMAP_EXTENT_LAST;
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size,
-							      flags);
-			} else if (size) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size, flags);
-				size = 0;
-			}
-
-			/* if we have holes up to/past EOF then we're done */
-			if (start_blk > last_blk || past_eof || ret)
-				break;
-		} else {
-			/*
-			 * We have gone over the length of what we wanted to
-			 * map, and it wasn't the entire file, so add the extent
-			 * we got last time and exit.
-			 *
-			 * This is for the case where say we want to map all the
-			 * way up to the second to the last block in a file, but
-			 * the last block is a hole, making the second to last
-			 * block FIEMAP_EXTENT_LAST.  In this case we want to
-			 * see if there is a hole after the second to last block
-			 * so we can mark it properly.  If we found data after
-			 * we exceeded the length we were requesting, then we
-			 * are good to go, just add the extent to the fieinfo
-			 * and break
-			 */
-			if (start_blk > last_blk && !whole_file) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size,
-							      flags);
-				break;
-			}
+			flags |= FIEMAP_EXTENT_LAST;
+		}
 
-			/*
-			 * if size != 0 then we know we already have an extent
-			 * to add, so add it.
-			 */
-			if (size) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size,
-							      flags);
-				if (ret)
-					break;
-			}
+		if (size)
+			ret = fiemap_fill_next_extent(fieinfo, logical,
+					phys, size, flags);
 
-			logical = blk_to_logical(inode, start_blk);
-			phys = blk_to_logical(inode, map_bh.b_blocknr);
-			size = map_bh.b_size;
-			flags = FIEMAP_EXTENT_MERGED;
+		if (start_blk > last_blk || ret)
+			break;
 
-			start_blk += logical_to_blk(inode, size);
+		logical = blk_to_logical(inode, start_blk);
+		phys = blk_to_logical(inode, map_bh.b_blocknr);
+		size = map_bh.b_size;
+		flags = FIEMAP_EXTENT_MERGED;
 
-			/*
-			 * If we are past the EOF, then we need to make sure as
-			 * soon as we find a hole that the last extent we found
-			 * is marked with FIEMAP_EXTENT_LAST
-			 */
-			if (!past_eof && logical + size >= isize)
-				past_eof = true;
-		}
+		start_blk += logical_to_blk(inode, size);
+next:
 		cond_resched();
 		if (fatal_signal_pending(current)) {
 			ret = -EINTR;
 			break;
 		}
-
 	} while (1);
 
 	/* If ret is 1 then we just hit the end of the extent array */
-- 
1.7.9.5



             reply	other threads:[~2016-01-09  8:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-09  8:50 Fan Li [this message]
2016-01-10 22:21 ` [PATCH] fs: fix bugs for __generic_block_fiemap() Dave Chinner
2016-01-11  6:41   ` Fan Li

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='001401d14aba$dfe6c370$9fb44a50$@samsung.com' \
    --to=fanofcode.li@samsung.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).