Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754616AbcJQEDx (ORCPT ); Mon, 17 Oct 2016 00:03:53 -0400 Received: from out0-147.mail.aliyun.com ([140.205.0.147]:54409 "EHLO out0-147.mail.aliyun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750752AbcJQEDp (ORCPT ); Mon, 17 Oct 2016 00:03:45 -0400 X-Greylist: delayed 316 seconds by postgrey-1.27 at vger.kernel.org; Mon, 17 Oct 2016 00:03:44 EDT X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R171e4;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e02c03310;MF=hillf.zj@alibaba-inc.com;NM=1;PH=DS;RN=3;SR=0;TI=SMTPD_---.73a-LMR_1476676690; Reply-To: "Hillf Danton" From: "Hillf Danton" To: "'Vegard Nossum'" , "'Akinobu Mita'" Cc: References: <20161016155612.4784-1-vegard.nossum@oracle.com> <20161016155612.4784-4-vegard.nossum@oracle.com> In-Reply-To: <20161016155612.4784-4-vegard.nossum@oracle.com> Subject: Re: [PATCH 04/10] fault injection: prevent recursive fault injection Date: Mon, 17 Oct 2016 11:58:10 +0800 Message-ID: <01ca01d2282a$ad367350$07a359f0$@alibaba-inc.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQIhTQlT54Bh7xc3q8iaCTOLk4bRAgJpRLx/n/prv8A= Content-Language: zh-cn Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2220 Lines: 79 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 > Signed-off-by: Vegard Nossum > --- > 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