Re: [PATCH v2 4/6] fs: Establish locking order for unrelated directories

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

Hi Jan,

kernel test robot noticed the following build errors:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on jaegeuk-f2fs/dev-test jaegeuk-f2fs/dev linus/master v6.4-rc4 next-20230601]
[cannot apply to vfs-idmapping/for-next]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/ext4-Remove-ext4-locking-of-moved-directory/20230601-225100
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230601105830.13168-4-jack%40suse.cz
patch subject: [PATCH v2 4/6] fs: Establish locking order for unrelated directories
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20230602/202306020948.TBmCxtVw-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/234d970a1de0d79e372cc04d6a8112d2aec56c44
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jan-Kara/ext4-Remove-ext4-locking-of-moved-directory/20230601-225100
        git checkout 234d970a1de0d79e372cc04d6a8112d2aec56c44
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=um SUBARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306020948.TBmCxtVw-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   fs/inode.c: In function 'lock_two_inodes':
>> fs/inode.c:1121:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
    1121 |         if (!inode1 || !inode2)
         |         ^~
   fs/inode.c:1129:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
    1129 |                 goto lock;
         |                 ^~~~
>> fs/inode.c:1129:17: error: label 'lock' used but not defined
   fs/inode.c: At top level:
>> fs/inode.c:1136:9: error: expected identifier or '(' before 'if'
    1136 |         if (S_ISDIR(inode2->i_mode) == S_ISDIR(inode1->i_mode)) {
         |         ^~
>> fs/inode.c:1139:11: error: expected identifier or '(' before 'else'
    1139 |         } else if (!S_ISDIR(inode1->i_mode))
         |           ^~~~
   In file included from include/linux/kernel.h:27,
                    from include/linux/cpumask.h:10,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:63,
                    from include/linux/wait.h:9,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from fs/inode.c:7:
>> include/linux/minmax.h:167:63: error: expected identifier or '(' before 'while'
     167 |         do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
         |                                                               ^~~~~
   fs/inode.c:1140:17: note: in expansion of macro 'swap'
    1140 |                 swap(inode1, inode2);
         |                 ^~~~
>> fs/inode.c:1141:5: error: expected '=', ',', ';', 'asm' or '__attribute__' before ':' token
    1141 | lock:
         |     ^
   fs/inode.c:1144:9: error: expected identifier or '(' before 'if'
    1144 |         if (inode2 && inode2 != inode1)
         |         ^~
>> fs/inode.c:1146:1: error: expected identifier or '(' before '}' token
    1146 | }
         | ^


vim +/lock +1129 fs/inode.c

  1105	
  1106	/**
  1107	 * lock_two_inodes - lock two inodes (may be regular files but also dirs)
  1108	 *
  1109	 * Lock any non-NULL argument. The caller must make sure that if he is passing
  1110	 * in two directories, one is not ancestor of the other.  Zero, one or two
  1111	 * objects may be locked by this function.
  1112	 *
  1113	 * @inode1: first inode to lock
  1114	 * @inode2: second inode to lock
  1115	 * @subclass1: inode lock subclass for the first lock obtained
  1116	 * @subclass2: inode lock subclass for the second lock obtained
  1117	 */
  1118	void lock_two_inodes(struct inode *inode1, struct inode *inode2,
  1119			     unsigned subclass1, unsigned subclass2)
  1120	{
> 1121		if (!inode1 || !inode2)
  1122			/*
  1123			 * Make sure @subclass1 will be used for the acquired lock.
  1124			 * This is not strictly necessary (no current caller cares) but
  1125			 * let's keep things consistent.
  1126			 */
  1127			if (!inode1)
  1128				swap(inode1, inode2);
> 1129			goto lock;
  1130		}
  1131	
  1132		/*
  1133		 * If one object is directory and the other is not, we must make sure
  1134		 * to lock directory first as the other object may be its child.
  1135		 */
> 1136		if (S_ISDIR(inode2->i_mode) == S_ISDIR(inode1->i_mode)) {
  1137			if (inode1 > inode2)
  1138				swap(inode1, inode2);
> 1139		} else if (!S_ISDIR(inode1->i_mode))
  1140			swap(inode1, inode2);
> 1141	lock:
  1142		if (inode1)
  1143			inode_lock_nested(inode1, subclass1);
  1144		if (inode2 && inode2 != inode1)
  1145			inode_lock_nested(inode2, subclass2);
> 1146	}
  1147	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki