From: "tip-bot2 for John Stultz" <tip-bot2@linutronix.de>
To: linux-tip-commits@vger.kernel.org
Cc: John Stultz <jstultz@google.com>, Ingo Molnar <mingo@kernel.org>,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: [tip: locking/core] locking/ww_mutex/test: Fix potential workqueue corruption
Date: Fri, 22 Sep 2023 08:11:37 -0000 [thread overview]
Message-ID: <169537029753.27769.15529645927257680234.tip-bot2@tip-bot2> (raw)
In-Reply-To: <20230922043616.19282-3-jstultz@google.com>
The following commit has been merged into the locking/core branch of tip:
Commit-ID: bccdd808902f8c677317cec47c306e42b93b849e
Gitweb: https://git.kernel.org/tip/bccdd808902f8c677317cec47c306e42b93b849e
Author: John Stultz <jstultz@google.com>
AuthorDate: Fri, 22 Sep 2023 04:36:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 22 Sep 2023 09:43:40 +02:00
locking/ww_mutex/test: Fix potential workqueue corruption
In some cases running with the test-ww_mutex code, I was seeing
odd behavior where sometimes it seemed flush_workqueue was
returning before all the work threads were finished.
Often this would cause strange crashes as the mutexes would be
freed while they were being used.
Looking at the code, there is a lifetime problem as the
controlling thread that spawns the work allocates the
"struct stress" structures that are passed to the workqueue
threads. Then when the workqueue threads are finished,
they free the stress struct that was passed to them.
Unfortunately the workqueue work_struct node is in the stress
struct. Which means the work_struct is freed before the work
thread returns and while flush_workqueue is waiting.
It seems like a better idea to have the controlling thread
both allocate and free the stress structures, so that we can
be sure we don't corrupt the workqueue by freeing the structure
prematurely.
So this patch reworks the test to do so, and with this change
I no longer see the early flush_workqueue returns.
Signed-off-by: John Stultz <jstultz@google.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20230922043616.19282-3-jstultz@google.com
---
kernel/locking/test-ww_mutex.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
index 9bceba6..358d661 100644
--- a/kernel/locking/test-ww_mutex.c
+++ b/kernel/locking/test-ww_mutex.c
@@ -479,7 +479,6 @@ retry:
} while (!time_after(jiffies, stress->timeout));
kfree(order);
- kfree(stress);
}
struct reorder_lock {
@@ -544,7 +543,6 @@ out:
list_for_each_entry_safe(ll, ln, &locks, link)
kfree(ll);
kfree(order);
- kfree(stress);
}
static void stress_one_work(struct work_struct *work)
@@ -565,8 +563,6 @@ static void stress_one_work(struct work_struct *work)
break;
}
} while (!time_after(jiffies, stress->timeout));
-
- kfree(stress);
}
#define STRESS_INORDER BIT(0)
@@ -577,15 +573,24 @@ static void stress_one_work(struct work_struct *work)
static int stress(int nlocks, int nthreads, unsigned int flags)
{
struct ww_mutex *locks;
- int n;
+ struct stress *stress_array;
+ int n, count;
locks = kmalloc_array(nlocks, sizeof(*locks), GFP_KERNEL);
if (!locks)
return -ENOMEM;
+ stress_array = kmalloc_array(nthreads, sizeof(*stress_array),
+ GFP_KERNEL);
+ if (!stress_array) {
+ kfree(locks);
+ return -ENOMEM;
+ }
+
for (n = 0; n < nlocks; n++)
ww_mutex_init(&locks[n], &ww_class);
+ count = 0;
for (n = 0; nthreads; n++) {
struct stress *stress;
void (*fn)(struct work_struct *work);
@@ -609,9 +614,7 @@ static int stress(int nlocks, int nthreads, unsigned int flags)
if (!fn)
continue;
- stress = kmalloc(sizeof(*stress), GFP_KERNEL);
- if (!stress)
- break;
+ stress = &stress_array[count++];
INIT_WORK(&stress->work, fn);
stress->locks = locks;
@@ -626,6 +629,7 @@ static int stress(int nlocks, int nthreads, unsigned int flags)
for (n = 0; n < nlocks; n++)
ww_mutex_destroy(&locks[n]);
+ kfree(stress_array);
kfree(locks);
return 0;
next prev parent reply other threads:[~2023-09-22 8:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 4:35 [PATCH 0/3] Fixes for test-ww_mutex stress test John Stultz
2023-09-22 4:35 ` [PATCH 1/3] test-ww_mutex: Use prng instead of rng to avoid hangs at bootup John Stultz
2023-09-22 8:11 ` [tip: locking/core] locking/ww_mutex/test: " tip-bot2 for John Stultz
2023-09-22 4:36 ` [PATCH 2/3] test-ww_mutex: Fix potential workqueue corruption John Stultz
2023-09-22 8:11 ` tip-bot2 for John Stultz [this message]
2023-09-22 4:36 ` [PATCH 3/3] test-ww_mutex: Make sure we bail out instead of livelock John Stultz
2023-09-22 8:11 ` [tip: locking/core] locking/ww_mutex/test: " tip-bot2 for John Stultz
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=169537029753.27769.15529645927257680234.tip-bot2@tip-bot2 \
--to=tip-bot2@linutronix.de \
--cc=jstultz@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=mingo@kernel.org \
--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
all inboxes | Powered by JetHome®