On Sunday, October 16, 2016 11:56 PM Vegard Nossum wrote: > > If something we call in the fail_dump() code path tries to acquire a > resource that might fail (due to fault injection), then we should not > try to recurse back into the fault injection code. > > I've seen this happen with the console semaphore in the upcoming > semaphore trylock fault injection code. > > Cc: Hillf Danton <hillf...@alibaba-inc.com> > Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> > --- > lib/fault-inject.c | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/lib/fault-inject.c b/lib/fault-inject.c > index 6a823a5..adba7c9 100644 > --- a/lib/fault-inject.c > +++ b/lib/fault-inject.c > @@ -100,6 +100,33 @@ static inline bool fail_stacktrace(struct fault_attr > *attr) > > #endif /* CONFIG_FAULT_INJECTION_STACKTRACE_FILTER */ > > +static DEFINE_PER_CPU(int, fault_active); > + > +static bool __fail(struct fault_attr *attr) > +{ > + bool ret = false; > + > + /* > + * Prevent recursive fault injection (this could happen if for > + * example printing the fault would itself run some code that > + * could fail) > + */ > + preempt_disable(); > + if (unlikely(__this_cpu_inc_return(fault_active) != 1)) > + goto out; > + > + ret = true; > + fail_dump(attr); > + > + if (atomic_read(&attr->times) != -1) > + atomic_dec_not_zero(&attr->times); > + > +out: > + __this_cpu_dec(fault_active); > + preempt_enable();
Given no see other patches in this set, I could easily miss anything important and correct me please. Though added, there are no words in the change log about preempt, and my wonder again is: can we add it in contexts like the fast path of buddy allocator? thanks Hillf > + return ret; > +} > + > /* > * This code is stolen from failmalloc-1.0 > * http://www.nongnu.org/failmalloc/ > @@ -134,12 +161,7 @@ bool should_fail(struct fault_attr *attr, ssize_t size) > if (!fail_stacktrace(attr)) > return false; > > - fail_dump(attr); > - > - if (atomic_read(&attr->times) != -1) > - atomic_dec_not_zero(&attr->times); > - > - return true; > + return __fail(attr); > } > EXPORT_SYMBOL_GPL(should_fail); > > -- > 2.10.0.479.g221bd91