diff mbox

btrfs: remove old tree_root dirent processing in btrfs_real_readdir()

Message ID

1478313693-3356-1-git-send-email-jeffm@suse.com (mailing list archive)

State

Superseded

Headers

show

From: jeffm@suse.com
To: linux-btrfs@vger.kernel.org
Cc: Jeff Mahoney <jeffm@suse.com>
Subject: [PATCH] btrfs: remove old tree_root dirent processing in
	btrfs_real_readdir()
Date: Fri,  4 Nov 2016 22:41:33 -0400
Message-Id: <1478313693-3356-1-git-send-email-jeffm@suse.com>
Sender: linux-btrfs-owner@vger.kernel.org
Precedence: bulk

Commit Message

Jeff Mahoney Nov. 5, 2016, 2:41 a.m. UTC

From: Jeff Mahoney <jeffm@suse.com>

Commit 3de4586c527 (Btrfs: Allow subvolumes and snapshots anywhere
in the directory tree) introduced the current system of placing
snapshots in the directory tree.  It also introduced the behavior of
creating the snapshot and then creating the directory entries for it.

We've kept this code around for compatibility reasons, but it turns
out that no file systems with the old tree_root based snapshots can
be mounted on newer (>= 2009) kernels anyway.  About a month after the
above commit, commit 2a7108ad89e (Btrfs: rev the disk format for the
inode compat and csum selection changes) landed, changing the superblock
magic number.

As a result, we know that we'll never encounter tree_root-based dirents
or have to deal with skipping our own snapshot dirents.  Since that
also means that we're now only iterating over DIR_INDEX items, which only
contain one directory entry per leaf item, we don't need to loop over
the leaf item contents anymore either.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 fs/btrfs/inode.c | 115 ++++++++++++++++++-------------------------------------
 1 file changed, 37 insertions(+), 78 deletions(-)

Comments

kernel test robot Nov. 5, 2016, 3:24 a.m. UTC | #1

Hi Jeff,

[auto build test WARNING on btrfs/next]
[also build test WARNING on v4.9-rc3 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/jeffm-suse-com/btrfs-remove-old-tree_root-dirent-processing-in-btrfs_real_readdir/20161105-104432
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git next
config: i386-randconfig-b0-11051048 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   fs/btrfs/inode.c: In function 'btrfs_real_readdir':
>> fs/btrfs/inode.c:5754:6: warning: unused variable 'di_len' [-Wunused-variable]
     u32 di_len;
         ^
>> fs/btrfs/inode.c:5753:6: warning: unused variable 'di_total' [-Wunused-variable]
     u32 di_total;
         ^
>> fs/btrfs/inode.c:5752:6: warning: unused variable 'di_cur' [-Wunused-variable]
     u32 di_cur;
         ^

vim +/di_len +5754 fs/btrfs/inode.c

16cdcec7 Miao Xie    2011-04-22  5746  	struct list_head del_list;
39279cc3 Chris Mason 2007-06-12  5747  	int ret;
5f39d397 Chris Mason 2007-10-15  5748  	struct extent_buffer *leaf;
39279cc3 Chris Mason 2007-06-12  5749  	int slot;
39279cc3 Chris Mason 2007-06-12  5750  	unsigned char d_type;
39279cc3 Chris Mason 2007-06-12  5751  	int over = 0;
39279cc3 Chris Mason 2007-06-12 @5752  	u32 di_cur;
39279cc3 Chris Mason 2007-06-12 @5753  	u32 di_total;
39279cc3 Chris Mason 2007-06-12 @5754  	u32 di_len;
5f39d397 Chris Mason 2007-10-15  5755  	char tmp_name[32];
5f39d397 Chris Mason 2007-10-15  5756  	char *name_ptr;
5f39d397 Chris Mason 2007-10-15  5757  	int name_len;

:::::: The code at line 5754 was first introduced by commit
:::::: 39279cc3d2704cfbf9c35dcb5bdd392159ae4625 Btrfs: split up super.c

:::::: TO: Chris Mason <chris.mason@oracle.com>
:::::: CC: David Woodhouse <dwmw2@hera.kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Jeff Mahoney Nov. 5, 2016, 4:58 p.m. UTC | #2

On 11/4/16 11:24 PM, kbuild test robot wrote:
> Hi Jeff,
> 
> [auto build test WARNING on btrfs/next]
> [also build test WARNING on v4.9-rc3 next-20161028]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]


Whoops.  I cleaned this up in the next patch that I'm still testing.
I'll re-send.

-Jeff

> url:    https://github.com/0day-ci/linux/commits/jeffm-suse-com/btrfs-remove-old-tree_root-dirent-processing-in-btrfs_real_readdir/20161105-104432
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git next
> config: i386-randconfig-b0-11051048 (attached as .config)
> compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
>    fs/btrfs/inode.c: In function 'btrfs_real_readdir':
>>> fs/btrfs/inode.c:5754:6: warning: unused variable 'di_len' [-Wunused-variable]
>      u32 di_len;
>          ^
>>> fs/btrfs/inode.c:5753:6: warning: unused variable 'di_total' [-Wunused-variable]
>      u32 di_total;
>          ^
>>> fs/btrfs/inode.c:5752:6: warning: unused variable 'di_cur' [-Wunused-variable]
>      u32 di_cur;
>          ^
> 
> vim +/di_len +5754 fs/btrfs/inode.c
> 
> 16cdcec7 Miao Xie    2011-04-22  5746  	struct list_head del_list;
> 39279cc3 Chris Mason 2007-06-12  5747  	int ret;
> 5f39d397 Chris Mason 2007-10-15  5748  	struct extent_buffer *leaf;
> 39279cc3 Chris Mason 2007-06-12  5749  	int slot;
> 39279cc3 Chris Mason 2007-06-12  5750  	unsigned char d_type;
> 39279cc3 Chris Mason 2007-06-12  5751  	int over = 0;
> 39279cc3 Chris Mason 2007-06-12 @5752  	u32 di_cur;
> 39279cc3 Chris Mason 2007-06-12 @5753  	u32 di_total;
> 39279cc3 Chris Mason 2007-06-12 @5754  	u32 di_len;
> 5f39d397 Chris Mason 2007-10-15  5755  	char tmp_name[32];
> 5f39d397 Chris Mason 2007-10-15  5756  	char *name_ptr;
> 5f39d397 Chris Mason 2007-10-15  5757  	int name_len;
> 
> :::::: The code at line 5754 was first introduced by commit
> :::::: 39279cc3d2704cfbf9c35dcb5bdd392159ae4625 Btrfs: split up super.c
> 
> :::::: TO: Chris Mason <chris.mason@oracle.com>
> :::::: CC: David Woodhouse <dwmw2@hera.kernel.org>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>

diff mbox

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2b790bd..74f5a92 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5808,17 +5808,13 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 	u32 di_cur;
 	u32 di_total;
 	u32 di_len;
-	int key_type = BTRFS_DIR_INDEX_KEY;
 	char tmp_name[32];
 	char *name_ptr;
 	int name_len;
 	int is_curr = 0;	/* ctx->pos points to the current index? */
 	bool emitted;
 	bool put = false;
-
-	/* FIXME, use a real flag for deciding about the key type */
-	if (root->fs_info->tree_root == root)
-		key_type = BTRFS_DIR_ITEM_KEY;
+	struct btrfs_key location;
 
 	if (!dir_emit_dots(file, ctx))
 		return 0;
@@ -5829,14 +5825,11 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 
 	path->reada = READA_FORWARD;
 
-	if (key_type == BTRFS_DIR_INDEX_KEY) {
-		INIT_LIST_HEAD(&ins_list);
-		INIT_LIST_HEAD(&del_list);
-		put = btrfs_readdir_get_delayed_items(inode, &ins_list,
-						      &del_list);
-	}
+	INIT_LIST_HEAD(&ins_list);
+	INIT_LIST_HEAD(&del_list);
+	put = btrfs_readdir_get_delayed_items(inode, &ins_list, &del_list);
 
-	key.type = key_type;
+	key.type = BTRFS_DIR_INDEX_KEY;
 	key.offset = ctx->pos;
 	key.objectid = btrfs_ino(inode);
 
@@ -5862,85 +5855,53 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 
 		if (found_key.objectid != key.objectid)
 			break;
-		if (found_key.type != key_type)
+		if (found_key.type != BTRFS_DIR_INDEX_KEY)
 			break;
 		if (found_key.offset < ctx->pos)
 			goto next;
-		if (key_type == BTRFS_DIR_INDEX_KEY &&
-		    btrfs_should_delete_dir_index(&del_list,
-						  found_key.offset))
+		if (btrfs_should_delete_dir_index(&del_list, found_key.offset))
 			goto next;
 
 		ctx->pos = found_key.offset;
 		is_curr = 1;
 
 		di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
-		di_cur = 0;
-		di_total = btrfs_item_size(leaf, item);
-
-		while (di_cur < di_total) {
-			struct btrfs_key location;
-
-			if (verify_dir_item(root, leaf, di))
-				break;
+		if (verify_dir_item(root, leaf, di))
+			goto next;
 
-			name_len = btrfs_dir_name_len(leaf, di);
-			if (name_len <= sizeof(tmp_name)) {
-				name_ptr = tmp_name;
-			} else {
-				name_ptr = kmalloc(name_len, GFP_KERNEL);
-				if (!name_ptr) {
-					ret = -ENOMEM;
-					goto err;
-				}
+		name_len = btrfs_dir_name_len(leaf, di);
+		if (name_len <= sizeof(tmp_name)) {
+			name_ptr = tmp_name;
+		} else {
+			name_ptr = kmalloc(name_len, GFP_KERNEL);
+			if (!name_ptr) {
+				ret = -ENOMEM;
+				goto err;
 			}
-			read_extent_buffer(leaf, name_ptr,
-					   (unsigned long)(di + 1), name_len);
-
-			d_type = btrfs_filetype_table[btrfs_dir_type(leaf, di)];
-			btrfs_dir_item_key_to_cpu(leaf, di, &location);
+		}
+		read_extent_buffer(leaf, name_ptr, (unsigned long)(di + 1),
+				   name_len);
 
+		d_type = btrfs_filetype_table[btrfs_dir_type(leaf, di)];
+		btrfs_dir_item_key_to_cpu(leaf, di, &location);
 
-			/* is this a reference to our own snapshot? If so
-			 * skip it.
-			 *
-			 * In contrast to old kernels, we insert the snapshot's
-			 * dir item and dir index after it has been created, so
-			 * we won't find a reference to our own snapshot. We
-			 * still keep the following code for backward
-			 * compatibility.
-			 */
-			if (location.type == BTRFS_ROOT_ITEM_KEY &&
-			    location.objectid == root->root_key.objectid) {
-				over = 0;
-				goto skip;
-			}
-			over = !dir_emit(ctx, name_ptr, name_len,
-				       location.objectid, d_type);
+		over = !dir_emit(ctx, name_ptr, name_len, location.objectid,
+				 d_type);
 
-skip:
-			if (name_ptr != tmp_name)
-				kfree(name_ptr);
+		if (name_ptr != tmp_name)
+			kfree(name_ptr);
 
-			if (over)
-				goto nopos;
-			emitted = true;
-			di_len = btrfs_dir_name_len(leaf, di) +
-				 btrfs_dir_data_len(leaf, di) + sizeof(*di);
-			di_cur += di_len;
-			di = (struct btrfs_dir_item *)((char *)di + di_len);
-		}
+		if (over)
+			goto nopos;
 next:
 		path->slots[0]++;
 	}
 
-	if (key_type == BTRFS_DIR_INDEX_KEY) {
-		if (is_curr)
-			ctx->pos++;
-		ret = btrfs_readdir_delayed_dir_index(ctx, &ins_list, &emitted);
-		if (ret)
-			goto nopos;
-	}
+	if (is_curr)
+		ctx->pos++;
+	ret = btrfs_readdir_delayed_dir_index(ctx, &ins_list, &emitted);
+	if (ret)
+		goto nopos;
 
 	/*
 	 * If we haven't emitted any dir entry, we must not touch ctx->pos as
@@ -5971,12 +5932,10 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 	 * last entry requires it because doing so has broken 32bit apps
 	 * in the past.
 	 */
-	if (key_type == BTRFS_DIR_INDEX_KEY) {
-		if (ctx->pos >= INT_MAX)
-			ctx->pos = LLONG_MAX;
-		else
-			ctx->pos = INT_MAX;
-	}
+	if (ctx->pos >= INT_MAX)
+		ctx->pos = LLONG_MAX;
+	else
+		ctx->pos = INT_MAX;
 nopos:
 	ret = 0;
 err: