All of lore.kernel.org
 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 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.