From: Steven Rostedt <rostedt@goodmis.org>
To: Petr Mladek <pmladek@suse.com>
Cc: linux-kernel@vger.kernel.org,
Masami Hiramatsu <mhiramat@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
Jonathan Corbet <corbet@lwn.net>, Guo Ren <guoren@kernel.org>,
"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
Helge Deller <deller@gmx.de>,
Michael Ellerman <mpe@ellerman.id.au>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Thomas Gleixner <tglx@linutronix.de>,
Borislav Petkov <bp@alien8.de>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Kees Cook <keescook@chromium.org>,
Anton Vorontsov <anton@enomsg.org>,
Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>,
Joe Lawrence <joe.lawrence@redhat.com>,
Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>,
Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
linux-doc@vger.kernel.org, linux-csky@vger.kernel.org,
linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-s390@vger.kernel.org, live-patching@vger.kernel.org
Subject: Re: [PATCH 11/11 v2] ftrace: Add recording of functions that caused recursion
Date: Mon, 2 Nov 2020 12:46:06 -0500 [thread overview]
Message-ID: <20201102124606.72bd89c5@gandalf.local.home> (raw)
In-Reply-To: <20201102123721.4fcce2cb@gandalf.local.home>
On Mon, 2 Nov 2020 12:37:21 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> The only race that I see that can happen, is the one in the comment I
> showed. And that is after enabling the recursed functions again after
> clearing, one CPU could add a function while another CPU that just added
> that same function could be just exiting this routine, notice that a
> clearing of the array happened, and remove its function (which was the same
> as the one just happened). So we get a "zero" in the array. If this
> happens, it is likely that that function will recurse again and will be
> added later.
>
Updated version of this function:
-- Steve
void ftrace_record_recursion(unsigned long ip, unsigned long parent_ip)
{
int index = 0;
int i;
unsigned long old;
again:
/* First check the last one recorded */
if (ip == cached_function)
return;
i = atomic_read(&nr_records);
/* nr_records is -1 when clearing records */
smp_mb__after_atomic();
if (i < 0)
return;
/*
* If there's two writers and this writer comes in second,
* the cmpxchg() below to update the ip will fail. Then this
* writer will try again. It is possible that index will now
* be greater than nr_records. This is because the writer
* that succeeded has not updated the nr_records yet.
* This writer could keep trying again until the other writer
* updates nr_records. But if the other writer takes an
* interrupt, and that interrupt locks up that CPU, we do
* not want this CPU to lock up due to the recursion protection,
* and have a bug report showing this CPU as the cause of
* locking up the computer. To not lose this record, this
* writer will simply use the next position to update the
* recursed_functions, and it will update the nr_records
* accordingly.
*/
if (index < i)
index = i;
if (index >= CONFIG_FTRACE_RECORD_RECURSION_SIZE)
return;
for (i = index - 1; i >= 0; i--) {
if (recursed_functions[i].ip == ip) {
cached_function = ip;
return;
}
}
cached_function = ip;
/*
* We only want to add a function if it hasn't been added before.
* Add to the current location before incrementing the count.
* If it fails to add, then increment the index (save in i)
* and try again.
*/
old = cmpxchg(&recursed_functions[index].ip, 0, ip);
if (old != 0) {
/* Did something else already added this for us? */
if (old == ip)
return;
/* Try the next location (use i for the next index) */
index++;
goto again;
}
recursed_functions[index].parent_ip = parent_ip;
/*
* It's still possible that we could race with the clearing
* CPU0 CPU1
* ---- ----
* ip = func
* nr_records = -1;
* recursed_functions[0] = 0;
* i = -1
* if (i < 0)
* nr_records = 0;
* (new recursion detected)
* recursed_functions[0] = func
* cmpxchg(recursed_functions[0],
* func, 0)
*
* But the worse that could happen is that we get a zero in
* the recursed_functions array, and it's likely that "func" will
* be recorded again.
*/
i = atomic_read(&nr_records);
smp_mb__after_atomic();
if (i < 0)
cmpxchg(&recursed_functions[index].ip, ip, 0);
else if (i <= index)
atomic_cmpxchg(&nr_records, i, index + 1);
}
next prev parent reply other threads:[~2020-11-02 17:46 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20201030213142.096102821@goodmis.org>
2020-10-30 21:31 ` [PATCH 06/11 v2] livepatch/ftrace: Add recursion protection to the ftrace callback Steven Rostedt
2020-10-30 21:31 ` [PATCH 07/11 v2] livepatch: Trigger WARNING if livepatch function fails due to recursion Steven Rostedt
2020-11-02 14:41 ` Petr Mladek
2020-11-02 16:56 ` Steven Rostedt
2020-10-30 21:31 ` [PATCH 11/11 v2] ftrace: Add recording of functions that caused recursion Steven Rostedt
2020-11-02 16:41 ` Petr Mladek
2020-11-02 17:09 ` Steven Rostedt
2020-11-02 17:19 ` Steven Rostedt
2020-11-03 10:40 ` Petr Mladek
2020-11-02 17:37 ` Steven Rostedt
2020-11-02 17:46 ` Steven Rostedt [this message]
2020-11-02 19:23 ` [PATCH 11/11 v2.2] " Steven Rostedt
2020-11-03 14:10 ` Petr Mladek
2020-11-03 16:14 ` Steven Rostedt
2020-11-04 19:13 ` Steven Rostedt
2020-11-02 19:14 ` [PATCH 11/11 v2.1] " Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201102124606.72bd89c5@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=James.Bottomley@hansenpartnership.com \
--cc=akpm@linux-foundation.org \
--cc=anton@enomsg.org \
--cc=benh@kernel.crashing.org \
--cc=bigeasy@linutronix.de \
--cc=borntraeger@de.ibm.com \
--cc=bp@alien8.de \
--cc=ccross@android.com \
--cc=corbet@lwn.net \
--cc=deller@gmx.de \
--cc=gor@linux.ibm.com \
--cc=guoren@kernel.org \
--cc=hca@linux.ibm.com \
--cc=hpa@zytor.com \
--cc=jikos@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=jpoimboe@redhat.com \
--cc=kamalesh@linux.vnet.ibm.com \
--cc=keescook@chromium.org \
--cc=linux-csky@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=live-patching@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=mchehab+huawei@kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).