Skip to content

Commit f9aa1fb

Browse files
committed
Merge tag 'wq-for-6.13-rc5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq
Pull workqueue fixes from Tejun Heo: - Suppress a corner case spurious flush dependency warning - Two trivial changes * tag 'wq-for-6.13-rc5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq: workqueue: add printf attribute to __alloc_workqueue() workqueue: Do not warn when cancelling WQ_MEM_RECLAIM work from !WQ_MEM_RECLAIM worker rust: add safety comment in workqueue traits
2 parents 2ae3aab + d57212f commit f9aa1fb

File tree

2 files changed

+30
-11
lines changed

2 files changed

+30
-11
lines changed

kernel/workqueue.c

+14-9
Original file line numberDiff line numberDiff line change
@@ -3680,23 +3680,27 @@ void workqueue_softirq_dead(unsigned int cpu)
36803680
* check_flush_dependency - check for flush dependency sanity
36813681
* @target_wq: workqueue being flushed
36823682
* @target_work: work item being flushed (NULL for workqueue flushes)
3683+
* @from_cancel: are we called from the work cancel path
36833684
*
36843685
* %current is trying to flush the whole @target_wq or @target_work on it.
3685-
* If @target_wq doesn't have %WQ_MEM_RECLAIM, verify that %current is not
3686-
* reclaiming memory or running on a workqueue which doesn't have
3687-
* %WQ_MEM_RECLAIM as that can break forward-progress guarantee leading to
3688-
* a deadlock.
3686+
* If this is not the cancel path (which implies work being flushed is either
3687+
* already running, or will not be at all), check if @target_wq doesn't have
3688+
* %WQ_MEM_RECLAIM and verify that %current is not reclaiming memory or running
3689+
* on a workqueue which doesn't have %WQ_MEM_RECLAIM as that can break forward-
3690+
* progress guarantee leading to a deadlock.
36893691
*/
36903692
static void check_flush_dependency(struct workqueue_struct *target_wq,
3691-
struct work_struct *target_work)
3693+
struct work_struct *target_work,
3694+
bool from_cancel)
36923695
{
3693-
work_func_t target_func = target_work ? target_work->func : NULL;
3696+
work_func_t target_func;
36943697
struct worker *worker;
36953698

3696-
if (target_wq->flags & WQ_MEM_RECLAIM)
3699+
if (from_cancel || target_wq->flags & WQ_MEM_RECLAIM)
36973700
return;
36983701

36993702
worker = current_wq_worker();
3703+
target_func = target_work ? target_work->func : NULL;
37003704

37013705
WARN_ONCE(current->flags & PF_MEMALLOC,
37023706
"workqueue: PF_MEMALLOC task %d(%s) is flushing !WQ_MEM_RECLAIM %s:%ps",
@@ -3980,7 +3984,7 @@ void __flush_workqueue(struct workqueue_struct *wq)
39803984
list_add_tail(&this_flusher.list, &wq->flusher_overflow);
39813985
}
39823986

3983-
check_flush_dependency(wq, NULL);
3987+
check_flush_dependency(wq, NULL, false);
39843988

39853989
mutex_unlock(&wq->mutex);
39863990

@@ -4155,7 +4159,7 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr,
41554159
}
41564160

41574161
wq = pwq->wq;
4158-
check_flush_dependency(wq, work);
4162+
check_flush_dependency(wq, work, from_cancel);
41594163

41604164
insert_wq_barrier(pwq, barr, work, worker);
41614165
raw_spin_unlock_irq(&pool->lock);
@@ -5641,6 +5645,7 @@ static void wq_adjust_max_active(struct workqueue_struct *wq)
56415645
} while (activated);
56425646
}
56435647

5648+
__printf(1, 0)
56445649
static struct workqueue_struct *__alloc_workqueue(const char *fmt,
56455650
unsigned int flags,
56465651
int max_active, va_list args)

rust/kernel/workqueue.rs

+16-2
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,15 @@ impl_has_work! {
519519
impl{T} HasWork<Self> for ClosureWork<T> { self.work }
520520
}
521521

522-
// SAFETY: TODO.
522+
// SAFETY: The `__enqueue` implementation in RawWorkItem uses a `work_struct` initialized with the
523+
// `run` method of this trait as the function pointer because:
524+
// - `__enqueue` gets the `work_struct` from the `Work` field, using `T::raw_get_work`.
525+
// - The only safe way to create a `Work` object is through `Work::new`.
526+
// - `Work::new` makes sure that `T::Pointer::run` is passed to `init_work_with_key`.
527+
// - Finally `Work` and `RawWorkItem` guarantee that the correct `Work` field
528+
// will be used because of the ID const generic bound. This makes sure that `T::raw_get_work`
529+
// uses the correct offset for the `Work` field, and `Work::new` picks the correct
530+
// implementation of `WorkItemPointer` for `Arc<T>`.
523531
unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
524532
where
525533
T: WorkItem<ID, Pointer = Self>,
@@ -537,7 +545,13 @@ where
537545
}
538546
}
539547

540-
// SAFETY: TODO.
548+
// SAFETY: The `work_struct` raw pointer is guaranteed to be valid for the duration of the call to
549+
// the closure because we get it from an `Arc`, which means that the ref count will be at least 1,
550+
// and we don't drop the `Arc` ourselves. If `queue_work_on` returns true, it is further guaranteed
551+
// to be valid until a call to the function pointer in `work_struct` because we leak the memory it
552+
// points to, and only reclaim it if the closure returns false, or in `WorkItemPointer::run`, which
553+
// is what the function pointer in the `work_struct` must be pointing to, according to the safety
554+
// requirements of `WorkItemPointer`.
541555
unsafe impl<T, const ID: u64> RawWorkItem<ID> for Arc<T>
542556
where
543557
T: WorkItem<ID, Pointer = Self>,

0 commit comments

Comments
 (0)