kernel: work: Fix race in workqueue thread
After a call to k_work_flush returns the sync variable may still be modified by the workq. This is because the work queue thread continues to modify the flag in sync even after k_work_flush returns. This commit adds K_WORK_FLUSHING_BIT, and with this bit, we moved the logic of waking up the caller from handle_flush to the finalize_flush_locked in workq, so that after waking up the caller, the workqueue will no longer operate on sync. Fixes: #64530 Signed-off-by: Junfan Song <sjf221100@gmail.com>
This commit is contained in:
parent
b8184ca3b7
commit
4ae558c505
2 changed files with 41 additions and 15 deletions
|
@ -3289,7 +3289,7 @@ void k_work_init(struct k_work *work,
|
||||||
* @param work pointer to the work item.
|
* @param work pointer to the work item.
|
||||||
*
|
*
|
||||||
* @return a mask of flags K_WORK_DELAYED, K_WORK_QUEUED,
|
* @return a mask of flags K_WORK_DELAYED, K_WORK_QUEUED,
|
||||||
* K_WORK_RUNNING, and K_WORK_CANCELING.
|
* K_WORK_RUNNING, K_WORK_CANCELING, and K_WORK_FLUSHING.
|
||||||
*/
|
*/
|
||||||
int k_work_busy_get(const struct k_work *work);
|
int k_work_busy_get(const struct k_work *work);
|
||||||
|
|
||||||
|
@ -3545,9 +3545,9 @@ k_work_delayable_from_work(struct k_work *work);
|
||||||
*
|
*
|
||||||
* @param dwork pointer to the delayable work item.
|
* @param dwork pointer to the delayable work item.
|
||||||
*
|
*
|
||||||
* @return a mask of flags K_WORK_DELAYED, K_WORK_QUEUED, K_WORK_RUNNING, and
|
* @return a mask of flags K_WORK_DELAYED, K_WORK_QUEUED, K_WORK_RUNNING,
|
||||||
* K_WORK_CANCELING. A zero return value indicates the work item appears to
|
* K_WORK_CANCELING, and K_WORK_FLUSHING. A zero return value indicates the
|
||||||
* be idle.
|
* work item appears to be idle.
|
||||||
*/
|
*/
|
||||||
int k_work_delayable_busy_get(const struct k_work_delayable *dwork);
|
int k_work_delayable_busy_get(const struct k_work_delayable *dwork);
|
||||||
|
|
||||||
|
@ -3795,9 +3795,10 @@ enum {
|
||||||
K_WORK_CANCELING_BIT = 1,
|
K_WORK_CANCELING_BIT = 1,
|
||||||
K_WORK_QUEUED_BIT = 2,
|
K_WORK_QUEUED_BIT = 2,
|
||||||
K_WORK_DELAYED_BIT = 3,
|
K_WORK_DELAYED_BIT = 3,
|
||||||
|
K_WORK_FLUSHING_BIT = 4,
|
||||||
|
|
||||||
K_WORK_MASK = BIT(K_WORK_DELAYED_BIT) | BIT(K_WORK_QUEUED_BIT)
|
K_WORK_MASK = BIT(K_WORK_DELAYED_BIT) | BIT(K_WORK_QUEUED_BIT)
|
||||||
| BIT(K_WORK_RUNNING_BIT) | BIT(K_WORK_CANCELING_BIT),
|
| BIT(K_WORK_RUNNING_BIT) | BIT(K_WORK_CANCELING_BIT) | BIT(K_WORK_FLUSHING_BIT),
|
||||||
|
|
||||||
/* Static work flags */
|
/* Static work flags */
|
||||||
K_WORK_DELAYABLE_BIT = 8,
|
K_WORK_DELAYABLE_BIT = 8,
|
||||||
|
@ -3848,6 +3849,12 @@ enum {
|
||||||
* Accessed via k_work_busy_get(). May co-occur with other flags.
|
* Accessed via k_work_busy_get(). May co-occur with other flags.
|
||||||
*/
|
*/
|
||||||
K_WORK_DELAYED = BIT(K_WORK_DELAYED_BIT),
|
K_WORK_DELAYED = BIT(K_WORK_DELAYED_BIT),
|
||||||
|
|
||||||
|
/** @brief Flag indicating a synced work item that is being flushed.
|
||||||
|
*
|
||||||
|
* Accessed via k_work_busy_get(). May co-occur with other flags.
|
||||||
|
*/
|
||||||
|
K_WORK_FLUSHING = BIT(K_WORK_FLUSHING_BIT),
|
||||||
};
|
};
|
||||||
|
|
||||||
/** @brief A structure used to submit work. */
|
/** @brief A structure used to submit work. */
|
||||||
|
|
|
@ -63,18 +63,14 @@ static inline uint32_t flags_get(const uint32_t *flagp)
|
||||||
static struct k_spinlock lock;
|
static struct k_spinlock lock;
|
||||||
|
|
||||||
/* Invoked by work thread */
|
/* Invoked by work thread */
|
||||||
static void handle_flush(struct k_work *work)
|
static void handle_flush(struct k_work *work) { }
|
||||||
{
|
|
||||||
struct z_work_flusher *flusher
|
|
||||||
= CONTAINER_OF(work, struct z_work_flusher, work);
|
|
||||||
|
|
||||||
k_sem_give(&flusher->sem);
|
|
||||||
}
|
|
||||||
|
|
||||||
static inline void init_flusher(struct z_work_flusher *flusher)
|
static inline void init_flusher(struct z_work_flusher *flusher)
|
||||||
{
|
{
|
||||||
|
struct k_work *work = &flusher->work;
|
||||||
k_sem_init(&flusher->sem, 0, 1);
|
k_sem_init(&flusher->sem, 0, 1);
|
||||||
k_work_init(&flusher->work, handle_flush);
|
k_work_init(&flusher->work, handle_flush);
|
||||||
|
flag_set(&work->flags, K_WORK_FLUSHING_BIT);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* List of pending cancellations. */
|
/* List of pending cancellations. */
|
||||||
|
@ -96,6 +92,26 @@ static inline void init_work_cancel(struct z_work_canceller *canceler,
|
||||||
sys_slist_append(&pending_cancels, &canceler->node);
|
sys_slist_append(&pending_cancels, &canceler->node);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Comeplete flushing of a work item.
|
||||||
|
*
|
||||||
|
* Invoked with work lock held.
|
||||||
|
*
|
||||||
|
* Invoked from a work queue thread.
|
||||||
|
*
|
||||||
|
* Reschedules.
|
||||||
|
*
|
||||||
|
* @param work the work structure that has completed flushing.
|
||||||
|
*/
|
||||||
|
static void finalize_flush_locked(struct k_work *work)
|
||||||
|
{
|
||||||
|
struct z_work_flusher *flusher
|
||||||
|
= CONTAINER_OF(work, struct z_work_flusher, work);
|
||||||
|
|
||||||
|
flag_clear(&work->flags, K_WORK_FLUSHING_BIT);
|
||||||
|
|
||||||
|
k_sem_give(&flusher->sem);
|
||||||
|
};
|
||||||
|
|
||||||
/* Complete cancellation of a work item and unlock held lock.
|
/* Complete cancellation of a work item and unlock held lock.
|
||||||
*
|
*
|
||||||
* Invoked with work lock held.
|
* Invoked with work lock held.
|
||||||
|
@ -672,13 +688,16 @@ static void work_queue_main(void *workq_ptr, void *p2, void *p3)
|
||||||
handler(work);
|
handler(work);
|
||||||
|
|
||||||
/* Mark the work item as no longer running and deal
|
/* Mark the work item as no longer running and deal
|
||||||
* with any cancellation issued while it was running.
|
* with any cancellation and flushing issued while it
|
||||||
* Clear the BUSY flag and optionally yield to prevent
|
* was running. Clear the BUSY flag and optionally
|
||||||
* starving other threads.
|
* yield to prevent starving other threads.
|
||||||
*/
|
*/
|
||||||
key = k_spin_lock(&lock);
|
key = k_spin_lock(&lock);
|
||||||
|
|
||||||
flag_clear(&work->flags, K_WORK_RUNNING_BIT);
|
flag_clear(&work->flags, K_WORK_RUNNING_BIT);
|
||||||
|
if (flag_test(&work->flags, K_WORK_FLUSHING_BIT)) {
|
||||||
|
finalize_flush_locked(work);
|
||||||
|
}
|
||||||
if (flag_test(&work->flags, K_WORK_CANCELING_BIT)) {
|
if (flag_test(&work->flags, K_WORK_CANCELING_BIT)) {
|
||||||
finalize_cancel_locked(work);
|
finalize_cancel_locked(work);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue