Re: [PATCH 2/2] EDAC/device: Serialize poll_msec store against device teardown

On Thu, Jul 30, 2026 at 03:55:49PM +0100, Jad Keskes wrote:
> 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.

Yeah, that's a good catch.

>  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:

Why?

Why don't you simply check for OP_RUNNING_POLL in
edac_device_ctl_poll_msec_store() and _show() and return an error if op_state
is not OP_RUNNING_POLL?

>   - 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.

Ah, I already did that in my patch and just saw you're doing it too.

I can remove mine again if you insist but the change to "unsigned int" needs
to be a separate patch and not part of this one.

Thx.


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette