Re: [PATCH 2/2] btrfs: send: fix invalid commands for inodes with changed type but same gen

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 9020 bytes --]

Hi Roman,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on btrfs/next v5.11-rc3 next-20210111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Roman-Anasal/btrfs-send-correctly-recreate-changed-inodes/20210112-030653
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/2993e0e565f9215fc3e4cedd9b1b9bc8df6dbdad
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Roman-Anasal/btrfs-send-correctly-recreate-changed-inodes/20210112-030653
        git checkout 2993e0e565f9215fc3e4cedd9b1b9bc8df6dbdad
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/btrfs/send.c: In function 'changed_inode':
>> fs/btrfs/send.c:6289:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    6289 |   u64 left_type = S_IFMT & btrfs_inode_mode(
         |   ^~~


vim +6289 fs/btrfs/send.c

  6243	
  6244	static int changed_inode(struct send_ctx *sctx,
  6245				 enum btrfs_compare_tree_result result)
  6246	{
  6247		int ret = 0;
  6248		struct btrfs_key *key = sctx->cmp_key;
  6249		struct btrfs_inode_item *left_ii = NULL;
  6250		struct btrfs_inode_item *right_ii = NULL;
  6251		u64 left_gen = 0;
  6252		u64 right_gen = 0;
  6253	
  6254		sctx->cur_ino = key->objectid;
  6255		sctx->cur_inode_recreated = 0;
  6256		sctx->cur_inode_last_extent = (u64)-1;
  6257		sctx->cur_inode_next_write_offset = 0;
  6258		sctx->ignore_cur_inode = false;
  6259	
  6260		/*
  6261		 * Set send_progress to current inode. This will tell all get_cur_xxx
  6262		 * functions that the current inode's refs are not updated yet. Later,
  6263		 * when process_recorded_refs is finished, it is set to cur_ino + 1.
  6264		 */
  6265		sctx->send_progress = sctx->cur_ino;
  6266	
  6267		if (result == BTRFS_COMPARE_TREE_NEW ||
  6268		    result == BTRFS_COMPARE_TREE_CHANGED) {
  6269			left_ii = btrfs_item_ptr(sctx->left_path->nodes[0],
  6270					sctx->left_path->slots[0],
  6271					struct btrfs_inode_item);
  6272			left_gen = btrfs_inode_generation(sctx->left_path->nodes[0],
  6273					left_ii);
  6274		} else {
  6275			right_ii = btrfs_item_ptr(sctx->right_path->nodes[0],
  6276					sctx->right_path->slots[0],
  6277					struct btrfs_inode_item);
  6278			right_gen = btrfs_inode_generation(sctx->right_path->nodes[0],
  6279					right_ii);
  6280		}
  6281		if (result == BTRFS_COMPARE_TREE_CHANGED) {
  6282			right_ii = btrfs_item_ptr(sctx->right_path->nodes[0],
  6283					sctx->right_path->slots[0],
  6284					struct btrfs_inode_item);
  6285	
  6286			right_gen = btrfs_inode_generation(sctx->right_path->nodes[0],
  6287					right_ii);
  6288	
> 6289			u64 left_type = S_IFMT & btrfs_inode_mode(
  6290					sctx->left_path->nodes[0], left_ii);
  6291			u64 right_type = S_IFMT & btrfs_inode_mode(
  6292					sctx->right_path->nodes[0], right_ii);
  6293	
  6294	
  6295			/*
  6296			 * The cur_ino = root dir case is special here. We can't treat
  6297			 * the inode as deleted+reused because it would generate a
  6298			 * stream that tries to delete/mkdir the root dir.
  6299			 */
  6300			if ((left_gen != right_gen || left_type != right_type) &&
  6301			    sctx->cur_ino != BTRFS_FIRST_FREE_OBJECTID)
  6302				sctx->cur_inode_recreated = 1;
  6303		}
  6304	
  6305		/*
  6306		 * Normally we do not find inodes with a link count of zero (orphans)
  6307		 * because the most common case is to create a snapshot and use it
  6308		 * for a send operation. However other less common use cases involve
  6309		 * using a subvolume and send it after turning it to RO mode just
  6310		 * after deleting all hard links of a file while holding an open
  6311		 * file descriptor against it or turning a RO snapshot into RW mode,
  6312		 * keep an open file descriptor against a file, delete it and then
  6313		 * turn the snapshot back to RO mode before using it for a send
  6314		 * operation. So if we find such cases, ignore the inode and all its
  6315		 * items completely if it's a new inode, or if it's a changed inode
  6316		 * make sure all its previous paths (from the parent snapshot) are all
  6317		 * unlinked and all other the inode items are ignored.
  6318		 */
  6319		if (result == BTRFS_COMPARE_TREE_NEW ||
  6320		    result == BTRFS_COMPARE_TREE_CHANGED) {
  6321			u32 nlinks;
  6322	
  6323			nlinks = btrfs_inode_nlink(sctx->left_path->nodes[0], left_ii);
  6324			if (nlinks == 0) {
  6325				sctx->ignore_cur_inode = true;
  6326				if (result == BTRFS_COMPARE_TREE_CHANGED)
  6327					ret = btrfs_unlink_all_paths(sctx);
  6328				goto out;
  6329			}
  6330		}
  6331	
  6332		if (result == BTRFS_COMPARE_TREE_NEW) {
  6333			sctx->cur_inode_gen = left_gen;
  6334			sctx->cur_inode_new = 1;
  6335			sctx->cur_inode_deleted = 0;
  6336			sctx->cur_inode_size = btrfs_inode_size(
  6337					sctx->left_path->nodes[0], left_ii);
  6338			sctx->cur_inode_mode = btrfs_inode_mode(
  6339					sctx->left_path->nodes[0], left_ii);
  6340			sctx->cur_inode_rdev = btrfs_inode_rdev(
  6341					sctx->left_path->nodes[0], left_ii);
  6342			if (sctx->cur_ino != BTRFS_FIRST_FREE_OBJECTID)
  6343				ret = send_create_inode_if_needed(sctx);
  6344		} else if (result == BTRFS_COMPARE_TREE_DELETED) {
  6345			sctx->cur_inode_gen = right_gen;
  6346			sctx->cur_inode_new = 0;
  6347			sctx->cur_inode_deleted = 1;
  6348			sctx->cur_inode_size = btrfs_inode_size(
  6349					sctx->right_path->nodes[0], right_ii);
  6350			sctx->cur_inode_mode = btrfs_inode_mode(
  6351					sctx->right_path->nodes[0], right_ii);
  6352		} else if (result == BTRFS_COMPARE_TREE_CHANGED) {
  6353			/*
  6354			 * We need to do some special handling in case the inode was
  6355			 * reported as changed with a changed generation number or
  6356			 * changed inode type. This means that the original inode was
  6357			 * deleted and new inode reused the same inum. So we have to
  6358			 * treat the old inode as deleted and the new one as new.
  6359			 */
  6360			if (sctx->cur_inode_recreated) {
  6361				/*
  6362				 * First, process the inode as if it was deleted.
  6363				 */
  6364				sctx->cur_inode_gen = right_gen;
  6365				sctx->cur_inode_new = 0;
  6366				sctx->cur_inode_deleted = 1;
  6367				sctx->cur_inode_size = btrfs_inode_size(
  6368						sctx->right_path->nodes[0], right_ii);
  6369				sctx->cur_inode_mode = btrfs_inode_mode(
  6370						sctx->right_path->nodes[0], right_ii);
  6371				ret = process_all_refs(sctx,
  6372						BTRFS_COMPARE_TREE_DELETED);
  6373				if (ret < 0)
  6374					goto out;
  6375	
  6376				/*
  6377				 * Now process the inode as if it was new.
  6378				 */
  6379				sctx->cur_inode_gen = left_gen;
  6380				sctx->cur_inode_new = 1;
  6381				sctx->cur_inode_deleted = 0;
  6382				sctx->cur_inode_size = btrfs_inode_size(
  6383						sctx->left_path->nodes[0], left_ii);
  6384				sctx->cur_inode_mode = btrfs_inode_mode(
  6385						sctx->left_path->nodes[0], left_ii);
  6386				sctx->cur_inode_rdev = btrfs_inode_rdev(
  6387						sctx->left_path->nodes[0], left_ii);
  6388				ret = send_create_inode_if_needed(sctx);
  6389				if (ret < 0)
  6390					goto out;
  6391	
  6392				ret = process_all_refs(sctx, BTRFS_COMPARE_TREE_NEW);
  6393				if (ret < 0)
  6394					goto out;
  6395				/*
  6396				 * Advance send_progress now as we did not get into
  6397				 * process_recorded_refs_if_needed in the
  6398				 * cur_inode_recreated case.
  6399				 */
  6400				sctx->send_progress = sctx->cur_ino + 1;
  6401	
  6402				/*
  6403				 * Now process all extents and xattrs of the inode as if
  6404				 * they were all new.
  6405				 */
  6406				ret = process_all_extents(sctx);
  6407				if (ret < 0)
  6408					goto out;
  6409				ret = process_all_new_xattrs(sctx);
  6410				if (ret < 0)
  6411					goto out;
  6412			} else {
  6413				sctx->cur_inode_gen = left_gen;
  6414				sctx->cur_inode_new = 0;
  6415				sctx->cur_inode_recreated = 0;
  6416				sctx->cur_inode_deleted = 0;
  6417				sctx->cur_inode_size = btrfs_inode_size(
  6418						sctx->left_path->nodes[0], left_ii);
  6419				sctx->cur_inode_mode = btrfs_inode_mode(
  6420						sctx->left_path->nodes[0], left_ii);
  6421			}
  6422		}
  6423	
  6424	out:
  6425		return ret;
  6426	}
  6427	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 75883 bytes --]