Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
From: kernel test robot <lkp@intel.com>
To: Harshad Shirwadkar <harshadshirwadkar@gmail.com>,
	linux-ext4@vger.kernel.org
Cc: kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
	tytso@mit.edu, Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Subject: Re: [PATCH v9 5/9] ext4: main fast-commit commit path
Date: Sat, 19 Sep 2020 16:19:03 +0800
Message-ID: <202009191602.OLtP5Go5%lkp@intel.com> (raw)
In-Reply-To: <20200919005451.3899779-6-harshadshirwadkar@gmail.com>


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

Hi Harshad,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on ext4/dev]
[also build test WARNING on linus/master v5.9-rc5 next-20200918]
[cannot apply to tip/perf/core]
[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/Harshad-Shirwadkar/ext4-add-fast-commits-feature/20200919-085652
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: x86_64-randconfig-a006-20200917 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project ed79827aea444e6995fb3d36abc2bfd36331773c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

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/ext4/fast_commit.c:1079:6: warning: variable 'start_time' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!test_opt2(sb, JOURNAL_FAST_COMMIT) ||
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ext4/fast_commit.c:1135:51: note: uninitialized use occurs here
           commit_time = ktime_to_ns(ktime_sub(ktime_get(), start_time));
                                                            ^~~~~~~~~~
   include/linux/ktime.h:46:39: note: expanded from macro 'ktime_sub'
   #define ktime_sub(lhs, rhs)     ((lhs) - (rhs))
                                             ^~~
   fs/ext4/fast_commit.c:1079:2: note: remove the 'if' if its condition is always false
           if (!test_opt2(sb, JOURNAL_FAST_COMMIT) ||
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> fs/ext4/fast_commit.c:1079:6: warning: variable 'start_time' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
           if (!test_opt2(sb, JOURNAL_FAST_COMMIT) ||
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ext4/fast_commit.c:1135:51: note: uninitialized use occurs here
           commit_time = ktime_to_ns(ktime_sub(ktime_get(), start_time));
                                                            ^~~~~~~~~~
   include/linux/ktime.h:46:39: note: expanded from macro 'ktime_sub'
   #define ktime_sub(lhs, rhs)     ((lhs) - (rhs))
                                             ^~~
   fs/ext4/fast_commit.c:1079:6: note: remove the '||' if its condition is always false
           if (!test_opt2(sb, JOURNAL_FAST_COMMIT) ||
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ext4/fast_commit.c:1075:20: note: initialize the variable 'start_time' to silence this warning
           ktime_t start_time, commit_time;
                             ^
                              = 0
   2 warnings generated.

# https://github.com/0day-ci/linux/commit/2384cbfbcf98b789d426c39b458c52adbb36d4f9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Harshad-Shirwadkar/ext4-add-fast-commits-feature/20200919-085652
git checkout 2384cbfbcf98b789d426c39b458c52adbb36d4f9
vim +1079 fs/ext4/fast_commit.c

  1061	
  1062	/*
  1063	 * The main commit entry point. Performs a fast commit for transaction
  1064	 * commit_tid if needed. If it's not possible to perform a fast commit
  1065	 * due to various reasons, we fall back to full commit. Returns 0
  1066	 * on success, error otherwise.
  1067	 */
  1068	int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
  1069	{
  1070		struct super_block *sb = (struct super_block *)(journal->j_private);
  1071		struct ext4_sb_info *sbi = EXT4_SB(sb);
  1072		int nblks = 0, ret, bsize = journal->j_blocksize;
  1073		int subtid = atomic_read(&sbi->s_fc_subtid);
  1074		int reason = EXT4_FC_REASON_OK, fc_bufs_before = 0;
  1075		ktime_t start_time, commit_time;
  1076	
  1077		trace_ext4_fc_commit_start(sb);
  1078	
> 1079		if (!test_opt2(sb, JOURNAL_FAST_COMMIT) ||
  1080			(ext4_fc_is_ineligible(sb))) {
  1081			reason = EXT4_FC_REASON_INELIGIBLE;
  1082			goto out;
  1083		}
  1084	
  1085		start_time = ktime_get();
  1086	restart_fc:
  1087		ret = jbd2_fc_start(journal, commit_tid);
  1088		if (ret == -EALREADY) {
  1089			/* There was an ongoing commit, check if we need to restart */
  1090			if (atomic_read(&sbi->s_fc_subtid) <= subtid &&
  1091				commit_tid > journal->j_commit_sequence)
  1092				goto restart_fc;
  1093			reason = EXT4_FC_REASON_ALREADY_COMMITTED;
  1094			goto out;
  1095		} else if (ret) {
  1096			sbi->s_fc_stats.fc_ineligible_reason_count[EXT4_FC_COMMIT_FAILED]++;
  1097			reason = EXT4_FC_REASON_FC_START_FAILED;
  1098			goto out;
  1099		}
  1100	
  1101		fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize;
  1102		ret = ext4_fc_perform_commit(journal);
  1103		if (ret < 0) {
  1104			sbi->s_fc_stats.fc_ineligible_reason_count[EXT4_FC_COMMIT_FAILED]++;
  1105			reason = EXT4_FC_REASON_FC_FAILED;
  1106			goto out;
  1107		}
  1108		nblks = (sbi->s_fc_bytes + bsize - 1) / bsize - fc_bufs_before;
  1109		ret = jbd2_fc_wait_bufs(journal, nblks);
  1110		if (ret < 0) {
  1111			sbi->s_fc_stats.fc_ineligible_reason_count[EXT4_FC_COMMIT_FAILED]++;
  1112			reason = EXT4_FC_REASON_FC_FAILED;
  1113			goto out;
  1114		}
  1115		atomic_inc(&sbi->s_fc_subtid);
  1116		jbd2_fc_stop(journal);
  1117	out:
  1118		/* Has any ineligible update happened since we started? */
  1119		if (reason == EXT4_FC_REASON_OK && ext4_fc_is_ineligible(sb)) {
  1120			sbi->s_fc_stats.fc_ineligible_reason_count[EXT4_FC_COMMIT_FAILED]++;
  1121			reason = EXT4_FC_REASON_INELIGIBLE;
  1122		}
  1123	
  1124		spin_lock(&sbi->s_fc_lock);
  1125		if (reason != EXT4_FC_REASON_OK &&
  1126			reason != EXT4_FC_REASON_ALREADY_COMMITTED) {
  1127			sbi->s_fc_stats.fc_ineligible_commits++;
  1128		} else {
  1129			sbi->s_fc_stats.fc_num_commits++;
  1130			sbi->s_fc_stats.fc_numblks += nblks;
  1131		}
  1132		spin_unlock(&sbi->s_fc_lock);
  1133		nblks = (reason == EXT4_FC_REASON_OK) ? nblks : 0;
  1134		trace_ext4_fc_commit_stop(sb, nblks, reason);
  1135		commit_time = ktime_to_ns(ktime_sub(ktime_get(), start_time));
  1136		/*
  1137		 * weight the commit time higher than the average time so we don't
  1138		 * react too strongly to vast changes in the commit time
  1139		 */
  1140		if (likely(sbi->s_fc_avg_commit_time))
  1141			sbi->s_fc_avg_commit_time = (commit_time +
  1142					sbi->s_fc_avg_commit_time * 3) / 4;
  1143		else
  1144			sbi->s_fc_avg_commit_time = commit_time;
  1145		jbd_debug(1,
  1146			"Fast commit ended with blks = %d, reason = %d, subtid - %d",
  1147			nblks, reason, subtid);
  1148		if (reason == EXT4_FC_REASON_FC_FAILED)
  1149			return jbd2_fc_stop_do_commit(journal, commit_tid);
  1150		if (reason == EXT4_FC_REASON_FC_START_FAILED ||
  1151			reason == EXT4_FC_REASON_INELIGIBLE)
  1152			return jbd2_complete_transaction(journal, commit_tid);
  1153		return 0;
  1154	}
  1155	

---
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: 43605 bytes --]

  reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-19  0:54 [PATCH v9 0/9] ext4: add fast commits feature Harshad Shirwadkar
2020-09-19  0:54 ` [PATCH v9 1/9] doc: update ext4 and journalling docs to include fast commit feature Harshad Shirwadkar
2020-09-22 17:50   ` Darrick J. Wong
2020-09-24  6:56     ` harshad shirwadkar
2020-10-09 18:28   ` Theodore Y. Ts'o
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 2/9] ext4: add fast_commit feature and handling for extended mount options Harshad Shirwadkar
2020-10-09 17:58   ` Theodore Y. Ts'o
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 3/9] ext4 / jbd2: add fast commit initialization Harshad Shirwadkar
2020-09-19 15:22   ` kernel test robot
2020-10-09 16:10   ` Ritesh Harjani
2020-10-13  0:28     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 4/9] jbd2: add fast commit machinery Harshad Shirwadkar
2020-10-09 16:16   ` Ritesh Harjani
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 5/9] ext4: main fast-commit commit path Harshad Shirwadkar
2020-09-19  8:19   ` kernel test robot [this message]
2020-10-09 17:04   ` Ritesh Harjani
2020-10-13  0:25     ` harshad shirwadkar
2020-10-09 19:14   ` Theodore Y. Ts'o
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 6/9] jbd2: fast commit recovery path Harshad Shirwadkar
2020-09-19  0:54 ` [PATCH v9 7/9] ext4: " Harshad Shirwadkar
2020-09-19 14:15   ` kernel test robot
2020-10-09 17:14   ` Ritesh Harjani
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 8/9] ext4: add a mount opt to forcefully turn fast commits on Harshad Shirwadkar
2020-09-19  0:54 ` [PATCH v9 9/9] ext4: add fast commit stats in procfs Harshad Shirwadkar

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=202009191602.OLtP5Go5%lkp@intel.com \
    --to=lkp@intel.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=harshadshirwadkar@gmail.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git