Messages in this thread Patch in this message | | | Date | Mon, 18 Oct 2021 13:06:43 +0200 | From | Peter Zijlstra <> | Subject | Re: [peterz-queue:locking/core 41/43] kernel/locking/rwsem.c:1023:19: error: variable has incomplete type 'enum owner_state' |
| |
On Sat, Oct 16, 2021 at 02:41:17AM +0800, Xu, Yanfei wrote: > > > On 10/15/21 9:51 PM, kernel test robot wrote: > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git locking/core > > head: 44e63f63c47dfb202eb25cdd97d04ec7e47f51d8 > > commit: b08614038dba3f6982e1e7701f23784bb0aedba6 [41/43] locking/rwsem: disable preemption for spinning region > > config: hexagon-randconfig-r041-20211014 (attached as .config) > > compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project acb3b187c4c88650a6a717a1bcb234d27d0d7f54) > > 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 > > # https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?id=b08614038dba3f6982e1e7701f23784bb0aedba6 > > git remote add peterz-queue https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git > > git fetch --no-tags peterz-queue locking/core > > git checkout b08614038dba3f6982e1e7701f23784bb0aedba6 > > # save the attached .config to linux build tree > > mkdir build_dir > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash kernel/locking/ > > > > 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/locking/rwsem.c:1023:19: error: variable has incomplete type 'enum owner_state' > > enum owner_state owner_state; > > ^ > > kernel/locking/rwsem.c:1023:7: note: forward declaration of 'enum owner_state' > > enum owner_state owner_state; > > ^ > > 1 error generated. > > > > > Hi Peter, > > I send a patch named ("locking/rwsem: Introduce __rwsem_spin_on_owner()") > for fixing this.
How about we do something like this instead? That makes the whole CONFIG_RWSEM_SPIN_ON_OWNER=n side more consistent.
---
--- a/kernel/locking/rwsem.c +++ b/kernel/locking/rwsem.c @@ -577,6 +577,24 @@ static inline bool rwsem_try_write_lock( return true; } +/* + * The rwsem_spin_on_owner() function returns the following 4 values + * depending on the lock owner state. + * OWNER_NULL : owner is currently NULL + * OWNER_WRITER: when owner changes and is a writer + * OWNER_READER: when owner changes and the new owner may be a reader. + * OWNER_NONSPINNABLE: + * when optimistic spinning has to stop because either the + * owner stops running, is unknown, or its timeslice has + * been used up. + */ +enum owner_state { + OWNER_NULL = 1 << 0, + OWNER_WRITER = 1 << 1, + OWNER_READER = 1 << 2, + OWNER_NONSPINNABLE = 1 << 3, +}; + #ifdef CONFIG_RWSEM_SPIN_ON_OWNER /* * Try to acquire write lock before the writer has been put on wait queue. @@ -632,23 +650,6 @@ static inline bool rwsem_can_spin_on_own return ret; } -/* - * The rwsem_spin_on_owner() function returns the following 4 values - * depending on the lock owner state. - * OWNER_NULL : owner is currently NULL - * OWNER_WRITER: when owner changes and is a writer - * OWNER_READER: when owner changes and the new owner may be a reader. - * OWNER_NONSPINNABLE: - * when optimistic spinning has to stop because either the - * owner stops running, is unknown, or its timeslice has - * been used up. - */ -enum owner_state { - OWNER_NULL = 1 << 0, - OWNER_WRITER = 1 << 1, - OWNER_READER = 1 << 2, - OWNER_NONSPINNABLE = 1 << 3, -}; #define OWNER_SPINNABLE (OWNER_NULL | OWNER_WRITER | OWNER_READER) static inline enum owner_state @@ -878,12 +879,11 @@ static inline bool rwsem_optimistic_spin static inline void clear_nonspinnable(struct rw_semaphore *sem) { } -static inline int +static inline enum owner_state rwsem_spin_on_owner(struct rw_semaphore *sem) { - return 0; + return OWNER_NONSPINNABLE; } -#define OWNER_NULL 1 #endif /* @@ -1095,9 +1095,16 @@ rwsem_down_write_slowpath(struct rw_sema * In this case, we attempt to acquire the lock again * without sleeping. */ - if (wstate == WRITER_HANDOFF && - rwsem_spin_on_owner(sem) == OWNER_NULL) - goto trylock_again; + if (wstate == WRITER_HANDOFF) { + enum owner_state owner_state; + + preempt_disable(); + owner_state = rwsem_spin_on_owner(sem); + preempt_enable(); + + if (owner_state == OWNER_NULL) + goto trylock_again; + } /* Block until there are no active lockers. */ for (;;) {
| |