From: Jad Keskes <inasj268@gmail.com>
To: linux-edac@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Borislav Petkov <bp@alien8.de>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Tony Luck <tony.luck@intel.com>,
James Morse <james.morse@arm.com>,
Robert Richter <rric@kernel.org>, Jad Keskes <inasj268@gmail.com>
Subject: [PATCH 2/2] EDAC/device: Serialize poll_msec store against device teardown
Date: Thu, 30 Jul 2026 15:55:49 +0100 [thread overview]
Message-ID: <20260730145549.148229-2-inasj268@gmail.com> (raw)
In-Reply-To: <20260730145549.148229-1-inasj268@gmail.com>
edac_device_reset_delay_period() unconditionally calls edac_mod_work()
to re-arm the workqueue timer when the poll_msec sysfs attribute is
written. This has two issues:
1) Interrupt-driven devices (op_state = OP_RUNNING_INTERRUPT) have
no initialized workqueue, so calling edac_mod_work() would operate
on uninitialized timer state.
2) A concurrent write to poll_msec during device removal can race
with edac_device_del_device(). Even with an OP_OFFLINE state check,
the check and edac_mod_work() are not atomic, allowing the workqueue
to be re-armed after teardown.
Fix both by holding device_ctls_mutex around the state check and
edac_mod_work() call in reset_delay_period(), and moving the workqueue
teardown inside the same mutex in del_device(). With the mutex held in
both paths:
- reset_delay_period() atomically verifies op_state == OP_RUNNING_POLL
before re-arming; any other state skips the call entirely.
- del_device() sets OP_OFFLINE and tears down the workqueue while
holding the mutex, so any racing reset_delay_period() completes
before teardown or sees OP_OFFLINE and bails.
Also fix the parameter type from unsigned long to unsigned int to match
the poll_msec field, and fix a latent bug where round_jiffies_relative()
received a millisecond value instead of jiffies.
Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
drivers/edac/edac_device.c | 15 +++++++++++----
drivers/edac/edac_module.h | 2 +-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 19522c568aa5..3fb4de3ed28c 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -394,17 +394,24 @@ static void edac_device_workq_teardown(struct edac_device_ctl_info *edac_dev)
* Then restart the workq on the new delay
*/
void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev,
- unsigned long value)
+ unsigned int value)
{
unsigned long jiffs = msecs_to_jiffies(value);
if (value == 1000)
- jiffs = round_jiffies_relative(value);
+ jiffs = round_jiffies_relative(jiffs);
+
+ mutex_lock(&device_ctls_mutex);
+ if (edac_dev->op_state != OP_RUNNING_POLL) {
+ mutex_unlock(&device_ctls_mutex);
+ return;
+ }
edac_dev->poll_msec = value;
edac_dev->delay = jiffs;
edac_mod_work(&edac_dev->work, jiffs);
+ mutex_unlock(&device_ctls_mutex);
}
int edac_device_alloc_index(void)
@@ -492,11 +499,11 @@ struct edac_device_ctl_info *edac_device_del_device(struct device *dev)
/* deregister from global list */
del_edac_device_from_global_list(edac_dev);
- mutex_unlock(&device_ctls_mutex);
-
/* clear workq processing on this instance */
edac_device_workq_teardown(edac_dev);
+ mutex_unlock(&device_ctls_mutex);
+
/* Tear down the sysfs entries for this instance */
edac_device_remove_sysfs(edac_dev);
diff --git a/drivers/edac/edac_module.h b/drivers/edac/edac_module.h
index 96f6de0c8ff6..e03ec7daa64a 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -56,7 +56,7 @@ bool edac_stop_work(struct delayed_work *work);
bool edac_mod_work(struct delayed_work *work, unsigned long delay);
extern void edac_device_reset_delay_period(struct edac_device_ctl_info
- *edac_dev, unsigned long value);
+ *edac_dev, unsigned int value);
extern void edac_mc_reset_delay_period(unsigned long value);
/*
--
2.55.0
prev parent reply other threads:[~2026-07-30 14:56 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-30 14:55 [PATCH 1/2] EDAC/device_sysfs: Use kstrtouint for poll_msec to prevent truncation Jad Keskes
2026-07-30 14:55 ` Jad Keskes [this message]
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=20260730145549.148229-2-inasj268@gmail.com \
--to=inasj268@gmail.com \
--cc=bp@alien8.de \
--cc=james.morse@arm.com \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=rric@kernel.org \
--cc=tony.luck@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.