Re: [PATCH v1 1/1] Firstly, as Andy mentioned, this should be smp_rmb() instead of rmb(). considering that TSYNC is a cross-thread situation, and rmb() is a mandatory barrier which should not be used to control SMP effects, since mandatory barriers impose unnecessary overhead on both SMP and UP systems, as kernel Documentation said.

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

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

Hi wanghongzhe,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.11-rc6]
[also build test WARNING on next-20210125]
[cannot apply to kees/for-next/seccomp]
[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/wanghongzhe/Firstly-as-Andy-mentioned-this-should-be-smp_rmb-instead-of-rmb-considering-that-TSYNC-is-a-cross-thread-situation-and-r/20210202-173311
base:    1048ba83fb1c00cd24172e23e8263972f6b5d9ac
config: i386-randconfig-s001-20210202 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-215-g0fb77bb6-dirty
        # https://github.com/0day-ci/linux/commit/f79414957fc8acb6b680bbcd26fa987328a5724a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review wanghongzhe/Firstly-as-Andy-mentioned-this-should-be-smp_rmb-instead-of-rmb-considering-that-TSYNC-is-a-cross-thread-situation-and-r/20210202-173311
        git checkout f79414957fc8acb6b680bbcd26fa987328a5724a
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' 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 >>):

   kernel/seccomp.c: In function '__secure_computing':
>> kernel/seccomp.c:1313:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    1313 |   if (unlikely(current->seccomp.mode != SECCOMP_MODE_FILTER))
         |      ^
   kernel/seccomp.c:1315:2: note: here
    1315 |  case SECCOMP_MODE_FILTER:
         |  ^~~~


vim +1313 kernel/seccomp.c

  1283	
  1284	int __secure_computing(const struct seccomp_data *sd)
  1285	{
  1286		int this_syscall;
  1287	
  1288		if (IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) &&
  1289		    unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
  1290			return 0;
  1291	
  1292		this_syscall = sd ? sd->nr :
  1293			syscall_get_nr(current, current_pt_regs());
  1294	
  1295		/*
  1296		 * Make sure that any changes to mode from another thread have
  1297		 * been seen after SYSCALL_WORK_SECCOMP was seen.
  1298		 */
  1299		smp_rmb();
  1300	
  1301		switch (current->seccomp.mode) {
  1302		case SECCOMP_MODE_STRICT:
  1303			__secure_computing_strict(this_syscall);  /* may call do_exit */
  1304			return 0;
  1305		/*
  1306		 * Make sure that change to mode (from SECCOMP_MODE_DISABLED to
  1307		 * SECCOMP_MODE_FILTER) from another thread using TSYNC ability
  1308		 * have been seen after SYSCALL_WORK_SECCOMP was seen. Read mode again behind
  1309		 * smp_rmb(), if it equals SECCOMP_MODE_FILTER, go to the right path.
  1310		 */
  1311		case SECCOMP_MODE_DISABLED:
  1312			smp_rmb();
> 1313			if (unlikely(current->seccomp.mode != SECCOMP_MODE_FILTER))
  1314				BUG();
  1315		case SECCOMP_MODE_FILTER:
  1316			return __seccomp_filter(this_syscall, sd, false);
  1317		default:
  1318			BUG();
  1319		}
  1320	}
  1321	#endif /* CONFIG_HAVE_ARCH_SECCOMP_FILTER */
  1322	

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