| Message ID | 20190926231824.149014-6-bgardon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm: mmu: Rework the x86 TDP direct mapped case | expand |
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@google.com> > --- > 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 >
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@intel.com> 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@google.com> > > --- > > 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 > >
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) { + 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. *
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. Signed-off-by: Ben Gardon <bgardon@google.com> --- include/linux/sched.h | 11 +++++++++++ kernel/sched/core.c | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+)