Messages in this thread Patch in this message | | | Date | Wed, 24 Jun 2020 13:30:25 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [rcu:rcu/next 35/35] kernel/rcu/tree.c:251:8: error: implicit declaration of function 'arch_atomic_add_return'; did you mean |
| |
On Thu, Jun 25, 2020 at 03:38:03AM +0800, kernel test robot wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git rcu/next > head: 347acb93a34a6e4f312f8b9ec1afdb86d27858d2 > commit: 347acb93a34a6e4f312f8b9ec1afdb86d27858d2 [35/35] rcu: Fixup noinstr warnings > config: mips-allyesconfig (attached as .config) > compiler: mips-linux-gcc (GCC) 9.3.0 > 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 > git checkout 347acb93a34a6e4f312f8b9ec1afdb86d27858d2 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All errors (new ones prefixed by >>): > > kernel/rcu/tree.c: In function 'rcu_dynticks_eqs_enter': > >> kernel/rcu/tree.c:251:8: error: implicit declaration of function 'arch_atomic_add_return'; did you mean 'atomic_add_return'? [-Werror=implicit-function-declaration] > 251 | seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks); > | ^~~~~~~~~~~~~~~~~~~~~~ > | atomic_add_return > kernel/rcu/tree.c: In function 'rcu_dynticks_eqs_exit': > >> kernel/rcu/tree.c:281:3: error: implicit declaration of function 'arch_atomic_andnot'; did you mean 'atomic_andnot'? [-Werror=implicit-function-declaration] > 281 | arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, &rdp->dynticks); > | ^~~~~~~~~~~~~~~~~~ > | atomic_andnot > kernel/rcu/tree.c: In function 'rcu_dynticks_curr_cpu_in_eqs': > >> kernel/rcu/tree.c:314:11: error: implicit declaration of function 'arch_atomic_read'; did you mean 'atomic_read'? [-Werror=implicit-function-declaration] > 314 | return !(arch_atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR); > | ^~~~~~~~~~~~~~~~ > | atomic_read > cc1: some warnings being treated as errors
And architectures using the definitions in include/linux/atomic-fallback.h don't like this patch much. MIPS defines everything in terms of atomic_add_return_relaxed(), for which it provides inline assembly for SMP-capable builds and a C-language code sequence otherwise.
One way of handling this is as follows:
------------------------------------------------------------------------
diff --git a/include/linux/atomic-fallback.h b/include/linux/atomic-fallback.h index 2c4927b..b7935857 100644 --- a/include/linux/atomic-fallback.h +++ b/include/linux/atomic-fallback.h @@ -133,6 +133,7 @@ atomic_add_return(int i, atomic_t *v) return ret; } #define atomic_add_return atomic_add_return +#define arch_atomic_add_return atomic_add_return #endif #endif /* atomic_add_return_relaxed */ ------------------------------------------------------------------------ And of course similar for arch_atomic_andnot() and arch_atomic_read().
Another way would be to define a noinstr_atomic_add_return() that was defined something like this:
------------------------------------------------------------------------ #ifdef CONFIG_HAVE_ARCH_KCSAN # define noinstr_atomic_add_return arch_atomic_add_return #else # define noinstr_atomic_add_return atomic_add_return #endif
------------------------------------------------------------------------ And again similarly for the others.
Left to myself, I would take the second option just because it provably leaves unaltered anything that isn't using the new API. That said, there has to be a better Kconfig option to key this off of.
Thoughts?
Thanx, Paul
> vim +251 kernel/rcu/tree.c > > 233 > 234 /* > 235 * Record entry into an extended quiescent state. This is only to be > 236 * called when not already in an extended quiescent state, that is, > 237 * RCU is watching prior to the call to this function and is no longer > 238 * watching upon return. > 239 */ > 240 static noinstr void rcu_dynticks_eqs_enter(void) > 241 { > 242 struct rcu_data *rdp = this_cpu_ptr(&rcu_data); > 243 int seq; > 244 > 245 /* > 246 * CPUs seeing atomic_add_return() must see prior RCU read-side > 247 * critical sections, and we also must force ordering with the > 248 * next idle sojourn. > 249 */ > 250 rcu_dynticks_task_trace_enter(); // Before ->dynticks update! > > 251 seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks); > 252 // RCU is no longer watching. Better be in extended quiescent state! > 253 WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && > 254 (seq & RCU_DYNTICK_CTRL_CTR)); > 255 /* Better not have special action (TLB flush) pending! */ > 256 WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && > 257 (seq & RCU_DYNTICK_CTRL_MASK)); > 258 } > 259 > 260 /* > 261 * Record exit from an extended quiescent state. This is only to be > 262 * called from an extended quiescent state, that is, RCU is not watching > 263 * prior to the call to this function and is watching upon return. > 264 */ > 265 static noinstr void rcu_dynticks_eqs_exit(void) > 266 { > 267 struct rcu_data *rdp = this_cpu_ptr(&rcu_data); > 268 int seq; > 269 > 270 /* > 271 * CPUs seeing atomic_add_return() must see prior idle sojourns, > 272 * and we also must force ordering with the next RCU read-side > 273 * critical section. > 274 */ > 275 seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks); > 276 // RCU is now watching. Better not be in an extended quiescent state! > 277 rcu_dynticks_task_trace_exit(); // After ->dynticks update! > 278 WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && > 279 !(seq & RCU_DYNTICK_CTRL_CTR)); > 280 if (seq & RCU_DYNTICK_CTRL_MASK) { > > 281 arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, &rdp->dynticks); > 282 smp_mb__after_atomic(); /* _exit after clearing mask. */ > 283 } > 284 } > 285 > 286 /* > 287 * Reset the current CPU's ->dynticks counter to indicate that the > 288 * newly onlined CPU is no longer in an extended quiescent state. > 289 * This will either leave the counter unchanged, or increment it > 290 * to the next non-quiescent value. > 291 * > 292 * The non-atomic test/increment sequence works because the upper bits > 293 * of the ->dynticks counter are manipulated only by the corresponding CPU, > 294 * or when the corresponding CPU is offline. > 295 */ > 296 static void rcu_dynticks_eqs_online(void) > 297 { > 298 struct rcu_data *rdp = this_cpu_ptr(&rcu_data); > 299 > 300 if (atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR) > 301 return; > 302 atomic_add(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks); > 303 } > 304 > 305 /* > 306 * Is the current CPU in an extended quiescent state? > 307 * > 308 * No ordering, as we are sampling CPU-local information. > 309 */ > 310 static __always_inline bool rcu_dynticks_curr_cpu_in_eqs(void) > 311 { > 312 struct rcu_data *rdp = this_cpu_ptr(&rcu_data); > 313 > > 314 return !(arch_atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR); > 315 } > 316 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
| |