lkml.org 
[lkml]   [2012]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCHSET] kthread_worker: reimplement flush_kthread_work() to allow freeing during execution
From
On Thu, Jul 19, 2012 at 2:15 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> kthread_worker was introduced together with concurrency managed
> workqueue to serve workqueue users which need a special dedicated
> worker - e.g. RT scheduling. This is minimal queue / flush / flush
> all iterface on top of kthread and each provided interface matches the
> workqueue counterpart so that switching isn't difficult.
>
> However, one noticeable difference was that kthread_worker doesn't
> allow a work item to be freed while being executed. The intention was
> to keep the code simpler but it didn't really and the restriction is
> subtle and does prevent some valid use cases.
>
> This two-patch series reimplements flush_kthread_work() so that it
> uses an extra work item for flushing. While this takes a bit more
> lines, this is easier to understand and removes the annoying
> difference.
>
> This patchset contains the following two patches.
>
> 0001-kthread_worker-reorganize-to-prepare-for-flush_kthre.patch
> 0002-kthread_worker-reimplement-flush_kthread_work-to-all.patch
>
> The first one is a prep patch which makes no functional changes. The
> second reimplements flush_kthread_work().
>
> All current kthread_worker users are cc'd. If no one objects, I'll
> push it through the workqueue branch. This patchset is also available
> in the following git branch.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-kthread_worker-flush
>
> diffstat follows. Thanks.
>
> include/linux/kthread.h | 8 +---
> kernel/kthread.c | 86 +++++++++++++++++++++++++++---------------------
> 2 files changed, 52 insertions(+), 42 deletions(-)
>
> --
> tejun

This patch set fixes a reproducible crash I'm seeing on a 3.4.10
kernel. flush_kthread_worker (which is different from
flush_kthread_work) is initializing a kthread_work and a completion on
the stack, then queuing it and calling wait_for_completion. Once the
completion is signaled, flush_kthread_worker exits and the stack
region used by the kthread_work may be immediately reused by another
object on the stack, but kthread_worker_fn continues accessing its
work pointer:
work->func(work); <- calls complete,
effectively frees work
smp_wmb(); /* wmb worker-b0 paired with flush-b1 */
work->done_seq = work->queue_seq; <- overwrites a
new stack object
smp_mb(); /* mb worker-b1 paired with flush-b0 */
if (atomic_read(&work->flushing))
wake_up_all(&work->done); <- or crashes here

These patches fix the problem by not accessing work after work->func
is called, and should be backported to stable. They apply cleanly to
3.4.10. Upstream commits are 9a2e03d8ed518a61154f18d83d6466628e519f94
and 46f3d976213452350f9d10b0c2780c2681f7075b.


\
 
 \ /
  Last update: 2012-09-15 01:21    [W:0.139 / U:1.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site