Lock contention should definitely be considered. It was an oversight
on my part to not have a check for that.
On Wed, Nov 27, 2019 at 10:42 AM Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
>
> On Thu, Sep 26, 2019 at 04:18:01PM -0700, Ben Gardon wrote:
> > Rescheduling while holding a spin lock is essential for keeping long
> > running kernel operations running smoothly. Add the facility to
> > cond_resched read/write spin locks.
> >
> > RFC_NOTE: The current implementation of this patch set uses a read/write
> > lock to replace the existing MMU spin lock. See the next patch in this
> > series for more on why a read/write lock was chosen, and possible
> > alternatives.
>
> This definitely needs to be run by the sched/locking folks sooner rather
> than later.
>
> > Signed-off-by: Ben Gardon <bgardon@xxxxxxxxxx>
> > ---
> > include/linux/sched.h | 11 +++++++++++
> > kernel/sched/core.c | 23 +++++++++++++++++++++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 70db597d6fd4f..4d1fd96693d9b 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -1767,12 +1767,23 @@ static inline int _cond_resched(void) { return 0; }
> > })
> >
> > extern int __cond_resched_lock(spinlock_t *lock);
> > +extern int __cond_resched_rwlock(rwlock_t *lock, bool write_lock);
> >
> > #define cond_resched_lock(lock) ({ \
> > ___might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET);\
> > __cond_resched_lock(lock); \
> > })
> >
> > +#define cond_resched_rwlock_read(lock) ({ \
> > + __might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET); \
> > + __cond_resched_rwlock(lock, false); \
> > +})
> > +
> > +#define cond_resched_rwlock_write(lock) ({ \
> > + __might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET); \
> > + __cond_resched_rwlock(lock, true); \
> > +})
> > +
> > static inline void cond_resched_rcu(void)
> > {
> > #if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index f9a1346a5fa95..ba7ed4bed5036 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -5663,6 +5663,29 @@ int __cond_resched_lock(spinlock_t *lock)
> > }
> > EXPORT_SYMBOL(__cond_resched_lock);
> >
> > +int __cond_resched_rwlock(rwlock_t *lock, bool write_lock)
> > +{
> > + int ret = 0;
> > +
> > + lockdep_assert_held(lock);
> > + if (should_resched(PREEMPT_LOCK_OFFSET)) {
> > + if (write_lock) {
>
> The existing __cond_resched_lock() checks for resched *or* lock
> contention. Is lock contention not something that needs (or can't?) be
> considered?
>
> > + write_unlock(lock);
> > + preempt_schedule_common();
> > + write_lock(lock);
> > + } else {
> > + read_unlock(lock);
> > + preempt_schedule_common();
> > + read_lock(lock);
> > + }
> > +
> > + ret = 1;
> > + }
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(__cond_resched_rwlock);
> > +
> > /**
> > * yield - yield the current processor to other threads.
> > *
> > --
> > 2.23.0.444.g18eeb5a265-goog
> >
