Hi Nikolay,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ext4/dev]
[also build test ERROR on v4.20-rc6 next-20181207]
[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/Nikolay-Borisov/fs-Convert-open-coded-is-inode-open-for-write-check/20181211-023543
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: i386-randconfig-x000-201849 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

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

   fs/ext4/mballoc.c: In function 'ext4_mb_group_or_file':
>> fs/ext4/mballoc.c:4180:34: error: 'inode' undeclared (first use in this function)
         && !inode_is_open_for_write(inode) {
                                     ^~~~~
   fs/ext4/mballoc.c:4180:34: note: each undeclared identifier is reported only once for each function it appears in
>> fs/ext4/mballoc.c:4180:41: error: expected ')' before '{' token
         && !inode_is_open_for_write(inode) {
                                            ^
>> fs/ext4/mballoc.c:4210:1: error: expected expression before '}' token
    }
    ^
   In file included from include/linux/kernel.h:14:0,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from fs/ext4/ext4_jbd2.h:15,
                    from fs/ext4/mballoc.c:12:
   fs/ext4/mballoc.c: In function 'ext4_mb_initialize_context':
>> fs/ext4/mballoc.c:4260:28: error: passing argument 1 of 'inode_is_open_for_write' from incompatible pointer type [-Werror=incompatible-pointer-types]
       inode_is_open_for_write(&ar->inode) ? "" : "non-");
                               ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
>> fs/ext4/mballoc.c:4254:2: note: in expansion of macro 'mb_debug'
     mb_debug(1, "init ac: %u blocks @ %u, goal %u, flags %x, 2^%d, "
     ^~~~~~~~
   In file included from fs/ext4/ext4_jbd2.h:15:0,
                    from fs/ext4/mballoc.c:12:
   include/linux/fs.h:2861:20: note: expected 'const struct inode *' but argument is of type 'struct inode **'
    static inline bool inode_is_open_for_write(const struct inode *inode)
                       ^~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/inode +4180 fs/ext4/mballoc.c

  4155	
  4156	/*
  4157	 * We use locality group preallocation for small size file. The size of the
  4158	 * file is determined by the current size or the resulting size after
  4159	 * allocation which ever is larger
  4160	 *
  4161	 * One can tune this size via /sys/fs/ext4/<partition>/mb_stream_req
  4162	 */
  4163	static void ext4_mb_group_or_file(struct ext4_allocation_context *ac)
  4164	{
  4165		struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
  4166		int bsbits = ac->ac_sb->s_blocksize_bits;
  4167		loff_t size, isize;
  4168	
  4169		if (!(ac->ac_flags & EXT4_MB_HINT_DATA))
  4170			return;
  4171	
  4172		if (unlikely(ac->ac_flags & EXT4_MB_HINT_GOAL_ONLY))
  4173			return;
  4174	
  4175		size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
  4176		isize = (i_size_read(ac->ac_inode) + ac->ac_sb->s_blocksize - 1)
  4177			>> bsbits;
  4178	
  4179		if ((size == isize) && !ext4_fs_is_busy(sbi)
> 4180		    && !inode_is_open_for_write(inode) {
  4181			ac->ac_flags |= EXT4_MB_HINT_NOPREALLOC;
  4182			return;
  4183		}
  4184	
  4185		if (sbi->s_mb_group_prealloc <= 0) {
  4186			ac->ac_flags |= EXT4_MB_STREAM_ALLOC;
  4187			return;
  4188		}
  4189	
  4190		/* don't use group allocation for large files */
  4191		size = max(size, isize);
  4192		if (size > sbi->s_mb_stream_request) {
  4193			ac->ac_flags |= EXT4_MB_STREAM_ALLOC;
  4194			return;
  4195		}
  4196	
  4197		BUG_ON(ac->ac_lg != NULL);
  4198		/*
  4199		 * locality group prealloc space are per cpu. The reason for having
  4200		 * per cpu locality group is to reduce the contention between block
  4201		 * request from multiple CPUs.
  4202		 */
  4203		ac->ac_lg = raw_cpu_ptr(sbi->s_locality_groups);
  4204	
  4205		/* we're going to use group allocation */
  4206		ac->ac_flags |= EXT4_MB_HINT_GROUP_ALLOC;
  4207	
  4208		/* serialize all allocations in the group */
  4209		mutex_lock(&ac->ac_lg->lg_mutex);
> 4210	}
  4211	
  4212	static noinline_for_stack int
  4213	ext4_mb_initialize_context(struct ext4_allocation_context *ac,
  4214					struct ext4_allocation_request *ar)
  4215	{
  4216		struct super_block *sb = ar->inode->i_sb;
  4217		struct ext4_sb_info *sbi = EXT4_SB(sb);
  4218		struct ext4_super_block *es = sbi->s_es;
  4219		ext4_group_t group;
  4220		unsigned int len;
  4221		ext4_fsblk_t goal;
  4222		ext4_grpblk_t block;
  4223	
  4224		/* we can't allocate > group size */
  4225		len = ar->len;
  4226	
  4227		/* just a dirty hack to filter too big requests  */
  4228		if (len >= EXT4_CLUSTERS_PER_GROUP(sb))
  4229			len = EXT4_CLUSTERS_PER_GROUP(sb);
  4230	
  4231		/* start searching from the goal */
  4232		goal = ar->goal;
  4233		if (goal < le32_to_cpu(es->s_first_data_block) ||
  4234				goal >= ext4_blocks_count(es))
  4235			goal = le32_to_cpu(es->s_first_data_block);
  4236		ext4_get_group_no_and_offset(sb, goal, &group, &block);
  4237	
  4238		/* set up allocation goals */
  4239		ac->ac_b_ex.fe_logical = EXT4_LBLK_CMASK(sbi, ar->logical);
  4240		ac->ac_status = AC_STATUS_CONTINUE;
  4241		ac->ac_sb = sb;
  4242		ac->ac_inode = ar->inode;
  4243		ac->ac_o_ex.fe_logical = ac->ac_b_ex.fe_logical;
  4244		ac->ac_o_ex.fe_group = group;
  4245		ac->ac_o_ex.fe_start = block;
  4246		ac->ac_o_ex.fe_len = len;
  4247		ac->ac_g_ex = ac->ac_o_ex;
  4248		ac->ac_flags = ar->flags;
  4249	
  4250		/* we have to define context: we'll we work with a file or
  4251		 * locality group. this is a policy, actually */
  4252		ext4_mb_group_or_file(ac);
  4253	
> 4254		mb_debug(1, "init ac: %u blocks @ %u, goal %u, flags %x, 2^%d, "
  4255				"left: %u/%u, right %u/%u to %swritable\n",
  4256				(unsigned) ar->len, (unsigned) ar->logical,
  4257				(unsigned) ar->goal, ac->ac_flags, ac->ac_2order,
  4258				(unsigned) ar->lleft, (unsigned) ar->pleft,
  4259				(unsigned) ar->lright, (unsigned) ar->pright,
> 4260				inode_is_open_for_write(&ar->inode) ? "" : "non-");
  4261		return 0;
  4262	

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