From: Waiman Long > Sent: 07 December 2020 15:34 > > On 12/7/20 4:02 AM, Peter Zijlstra wrote: > > On Thu, Dec 03, 2020 at 08:59:13PM -0500, Waiman Long wrote: > >> On 12/3/20 3:11 PM, Eric W. Biederman wrote: > >>> +static inline int __down_read_interruptible(struct rw_semaphore *sem) > >>> +{ > >>> + if (!rwsem_read_trylock(sem)) { > >>> + if (IS_ERR(rwsem_down_read_slowpath(sem, TASK_INTERRUPTIBLE))) > >>> + return -EINTR; > >>> + DEBUG_RWSEMS_WARN_ON(!is_rwsem_reader_owned(sem), sem); > >>> + } else { > >>> + rwsem_set_reader_owned(sem); > >>> + } > >>> + return 0; > >>> +} > >>> + > >>> static inline int __down_read_killable(struct rw_semaphore *sem) > >>> { > >>> if (!rwsem_read_trylock(sem)) { > >>> @@ -1495,6 +1507,20 @@ void __sched down_read(struct rw_semaphore *sem) > >>> } > >>> EXPORT_SYMBOL(down_read); > >>> +int __sched down_read_interruptible(struct rw_semaphore *sem) > >>> +{ > >>> + might_sleep(); > >>> + rwsem_acquire_read(&sem->dep_map, 0, 0, _RET_IP_); > >>> + > >>> + if (LOCK_CONTENDED_RETURN(sem, __down_read_trylock, > >>> __down_read_interruptible)) { > >>> + rwsem_release(&sem->dep_map, _RET_IP_); > >>> + return -EINTR; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> +EXPORT_SYMBOL(down_read_interruptible); > >>> + > >>> int __sched down_read_killable(struct rw_semaphore *sem) > >>> { > >>> might_sleep(); > >> Acked-by: Waiman Long <long...@redhat.com> > > Yeah, that seems correct.. There's an unfortunate amount of copy-paste > > there though. > > > > Do we want to follow that up with something like this? > > I am actually thinking about similar streamlining once the patch lands. > > Your suggested changes look fine to me.
How much more difficult would it be to also add a timeout option? I looked at adding one to the mutex code - and fell into a big pile of replicated code. ISTM that one the initial locked exchange (and spin) fails a few extra instructions when heading for the sleep don't really matter David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)