Messages in this thread | | | From | "Davis, George" <> | Subject | Re: [PATCH v2 1/1] sched/debug: fix potential deadlock when write to sched_features | Date | Tue, 31 Jul 2018 18:51:01 +0000 |
| |
> On Jul 31, 2018, at 8:36 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Tue, Jul 31, 2018 at 09:12:22PM +0900, jiada_wang@mentor.com wrote: >> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c >> index 60caf1fb94e0..7b2997d4a349 100644 >> --- a/kernel/sched/debug.c >> +++ b/kernel/sched/debug.c >> @@ -87,21 +87,21 @@ struct static_key sched_feat_keys[__SCHED_FEAT_NR] = { >> >> #undef SCHED_FEAT >> >> -static void sched_feat_disable(int i) >> +static void sched_feat_disable_cpuslocked(int i) >> { >> - static_key_disable(&sched_feat_keys[i]); >> + static_key_disable_cpuslocked(&sched_feat_keys[i]); >> } >> >> -static void sched_feat_enable(int i) >> +static void sched_feat_enable_cpuslocked(int i) >> { >> - static_key_enable(&sched_feat_keys[i]); >> + static_key_enable_cpuslocked(&sched_feat_keys[i]); >> } >> #else >> -static void sched_feat_disable(int i) { }; >> -static void sched_feat_enable(int i) { }; >> +static void sched_feat_disable_cpuslocked(int i) { }; >> +static void sched_feat_enable_cpuslocked(int i) { }; >> #endif /* HAVE_JUMP_LABEL */ >> >> -static int sched_feat_set(char *cmp) >> +static int sched_feat_set_cpuslocked(char *cmp) >> { >> int i; >> int neg = 0; >> @@ -117,10 +117,10 @@ static int sched_feat_set(char *cmp) >> >> if (neg) { >> sysctl_sched_features &= ~(1UL << i); >> - sched_feat_disable(i); >> + sched_feat_disable_cpuslocked(i); >> } else { >> sysctl_sched_features |= (1UL << i); >> - sched_feat_enable(i); >> + sched_feat_enable_cpuslocked(i); >> } >> >> return 0; >> @@ -146,9 +146,11 @@ sched_feat_write(struct file *filp, const char __user *ubuf, >> >> /* Ensure the static_key remains in a consistent state */ >> inode = file_inode(filp); >> + cpus_read_lock(); >> inode_lock(inode); >> - ret = sched_feat_set(cmp); >> + ret = sched_feat_set_cpuslocked(cmp); >> inode_unlock(inode); >> + cpus_read_unlock(); >> if (ret < 0) >> return ret; >> > > I've made that.. > > --- > --- a/kernel/sched/debug.c > +++ b/kernel/sched/debug.c > @@ -89,12 +89,12 @@ struct static_key sched_feat_keys[__SCHE > > static void sched_feat_disable(int i) > { > - static_key_disable(&sched_feat_keys[i]); > + static_key_disable_cpuslocked(&sched_feat_keys[i]); > } > > static void sched_feat_enable(int i) > { > - static_key_enable(&sched_feat_keys[i]); > + static_key_enable_cpuslocked(&sched_feat_keys[i]); > } > #else > static void sched_feat_disable(int i) { }; > @@ -146,9 +146,11 @@ sched_feat_write(struct file *filp, cons > > /* Ensure the static_key remains in a consistent state */ > inode = file_inode(filp); > + cpus_read_lock(); > inode_lock(inode); > ret = sched_feat_set(cmp); > inode_unlock(inode); > + cpus_read_unlock(); > if (ret < 0) > return ret; >
For the record, for both Jiada's v2 and Peter's simplified versions:
Reviewed-and-Tested-by: George G. Davis <george_davis@mentor.com>
| |