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:37:21 -0500 [thread overview]
Message-ID: <20201102123721.4fcce2cb@gandalf.local.home> (raw)
In-Reply-To: <20201102164147.GJ20201@alley>
On Mon, 2 Nov 2020 17:41:47 +0100
Petr Mladek <pmladek@suse.com> wrote:
> > + 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);
>
> This looks weird. It would shift nr_records past the record added
> in this call. It might skip many slots that were zeroed when clearing.
> Also we do not know if our entry was not zeroed as well.
nr_records always holds the next position to write to.
index = nr_records;
recursed_functions[index].ip = ip;
nr_records++;
Before clearing, we have:
nr_records = -1;
smp_mb();
memset(recursed_functions, 0);
smp_wmb();
nr_records = 0;
When we enter this function:
i = nr_records;
smp_mb();
if (i < 0)
return;
Thus, we just stopped all new updates while clearing the records.
But what about if something is currently updating?
i = nr_records;
smp_mb();
if (i < 0)
cmpxchg(recursed_functions, ip, 0);
The above shows that if the current updating process notices that the
clearing happens, it will clear the function it added.
else if (i <= index)
cmpxchg(nr_records, i, index + 1);
This makes sure that nr_records only grows if it is greater or equal to
zero.
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.
-- Steve
next prev parent reply other threads:[~2020-11-02 17:37 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 [this message]
2020-11-02 17:46 ` Steven Rostedt
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=20201102123721.4fcce2cb@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).