linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: wcohen@redhat.com (William Cohen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] Correct the race condition in aarch64_insn_patch_text_sync()
Date: Mon, 10 Nov 2014 11:36:02 -0500	[thread overview]
Message-ID: <1415637362-30754-1-git-send-email-wcohen@redhat.com> (raw)

When experimenting with patches to provide kprobes support for aarch64
smp machines would hang when inserting breakpoints into kernel code.
The hangs were caused by a race condition in the code called by
aarch64_insn_patch_text_sync().  The first processor in the
aarch64_insn_patch_text_cb() function would patch the code while other
processors were still entering the function and decrementing the
cpu_count field.  This resulted in some processors never observing the
exit condition and exiting the function.  Thus, processors in the
system hung.

The patching function now waits for all processors to enter the
patching function before changing code to ensure that none of the
processors are in code that is going to be patched.  Once all the
processors have entered the function, the last processor to enter the
patching function performs the pathing and signals that the patching
is complete with one last decrement of the cpu_count field to make it
-1.

Signed-off-by: William Cohen <wcohen@redhat.com>
---
 arch/arm64/kernel/insn.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index e007714..e6266db 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -153,8 +153,10 @@ static int __kprobes aarch64_insn_patch_text_cb(void *arg)
 	int i, ret = 0;
 	struct aarch64_insn_patch *pp = arg;
 
-	/* The first CPU becomes master */
-	if (atomic_inc_return(&pp->cpu_count) == 1) {
+	/* Make sure all the processors are in this function
+	   before patching the code. The last CPU to this function
+	   does the update. */
+	if (atomic_dec_return(&pp->cpu_count) == 0) {
 		for (i = 0; ret == 0 && i < pp->insn_cnt; i++)
 			ret = aarch64_insn_patch_text_nosync(pp->text_addrs[i],
 							     pp->new_insns[i]);
@@ -163,7 +165,8 @@ static int __kprobes aarch64_insn_patch_text_cb(void *arg)
 		 * which ends with "dsb; isb" pair guaranteeing global
 		 * visibility.
 		 */
-		atomic_set(&pp->cpu_count, -1);
+		/* Notifiy other processors with an additional decrement. */
+		atomic_dec(&pp->cpu_count);
 	} else {
 		while (atomic_read(&pp->cpu_count) != -1)
 			cpu_relax();
@@ -185,6 +188,7 @@ int __kprobes aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt)
 	if (cnt <= 0)
 		return -EINVAL;
 
+	atomic_set(&patch.cpu_count, num_online_cpus());
 	return stop_machine(aarch64_insn_patch_text_cb, &patch,
 			    cpu_online_mask);
 }
-- 
1.8.3.1

             reply	other threads:[~2014-11-10 16:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-10 16:36 William Cohen [this message]
2014-11-10 17:08 ` [PATCH] Correct the race condition in aarch64_insn_patch_text_sync() Will Deacon
2014-11-10 19:37   ` William Cohen
2014-11-11 11:28     ` Will Deacon
2014-11-11 14:48       ` William Cohen
2014-11-11 17:51         ` Will Deacon
2014-11-13 15:14           ` Catalin Marinas

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=1415637362-30754-1-git-send-email-wcohen@redhat.com \
    --to=wcohen@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.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).