2014-01-07 08:49:20

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 1/3] workqueue: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()


In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
call destroy_work_on_stack() which frees the debug object to pair
with INIT_WORK_ONSTACK().

Signed-off-by: Liu, Chuansheng <[email protected]>
---
kernel/workqueue.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index b010eac..82ef9f3 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4789,6 +4789,7 @@ static int workqueue_cpu_down_callback(struct notifier_block *nfb,

/* wait for per-cpu unbinding to finish */
flush_work(&unbind_work);
+ destroy_work_on_stack(&unbind_work);
break;
}
return NOTIFY_OK;
@@ -4828,6 +4829,7 @@ long work_on_cpu(int cpu, long (*fn)(void *), void *arg)
INIT_WORK_ONSTACK(&wfc.work, work_for_cpu_fn);
schedule_work_on(cpu, &wfc.work);
flush_work(&wfc.work);
+ destroy_work_on_stack(&wfc.work);
return wfc.ret;
}
EXPORT_SYMBOL_GPL(work_on_cpu);
--
1.7.9.5



2014-01-07 08:48:28

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 2/3] xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()


In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
call destroy_work_on_stack() which frees the debug object to pair
with INIT_WORK_ONSTACK().

Signed-off-by: Liu, Chuansheng <[email protected]>
---
fs/xfs/xfs_bmap_util.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 1394106..82e0dab 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -287,6 +287,7 @@ xfs_bmapi_allocate(
INIT_WORK_ONSTACK(&args->work, xfs_bmapi_allocate_worker);
queue_work(xfs_alloc_wq, &args->work);
wait_for_completion(&done);
+ destroy_work_on_stack(&args->work);
return args->result;
}

--
1.7.9.5


2014-01-07 08:51:12

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 3/3] dm snapshot: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()


In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
call destroy_work_on_stack() which frees the debug object to pair
with INIT_WORK_ONSTACK().

Signed-off-by: Liu, Chuansheng <[email protected]>
---
drivers/md/dm-snap-persistent.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c
index 2d2b1b7..2f5a9f8 100644
--- a/drivers/md/dm-snap-persistent.c
+++ b/drivers/md/dm-snap-persistent.c
@@ -257,6 +257,7 @@ static int chunk_io(struct pstore *ps, void *area, chunk_t chunk, int rw,
INIT_WORK_ONSTACK(&req.work, do_metadata);
queue_work(ps->metadata_wq, &req.work);
flush_workqueue(ps->metadata_wq);
+ destroy_work_on_stack(&req.work);

return req.result;
}
--
1.7.9.5


2014-01-07 11:21:18

by Jeff Liu

[permalink] [raw]
Subject: Re: [PATCH 2/3] xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()

Hi Chuansheng,

On 01/07 2014 16:53 PM, Chuansheng Liu wrote:
>
> In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
> call destroy_work_on_stack() which frees the debug object to pair
> with INIT_WORK_ONSTACK().
>
> Signed-off-by: Liu, Chuansheng <[email protected]>
> ---
> fs/xfs/xfs_bmap_util.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 1394106..82e0dab 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -287,6 +287,7 @@ xfs_bmapi_allocate(
> INIT_WORK_ONSTACK(&args->work, xfs_bmapi_allocate_worker);
> queue_work(xfs_alloc_wq, &args->work);
> wait_for_completion(&done);
> + destroy_work_on_stack(&args->work);
> return args->result;
> }

Thanks for your patch and it work fine for my testing. I missed this in an
old commit: [ 3b876c8f2a xfs: fix debug_object WARN at xfs_alloc_vextent() ]

Just out of curious, do you notice memory leaks or other hints which help you
finding out this problem?

Thanks,
-Jeff

2014-01-07 15:44:15

by Mike Snitzer

[permalink] [raw]
Subject: Re: [PATCH 3/3] dm snapshot: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()

On Tue, Jan 07 2014 at 3:56am -0500,
Chuansheng Liu <[email protected]> wrote:

>
> In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
> call destroy_work_on_stack() which frees the debug object to pair
> with INIT_WORK_ONSTACK().
>
> Signed-off-by: Liu, Chuansheng <[email protected]>

Applied to linux-dm.git's 'for-next' branch.

Thanks,
Mike

2014-01-07 19:34:18

by Ben Myers

[permalink] [raw]
Subject: Re: [PATCH 2/3] xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()

On Tue, Jan 07, 2014 at 07:21:05PM +0800, Jeff Liu wrote:
> Hi Chuansheng,
>
> On 01/07 2014 16:53 PM, Chuansheng Liu wrote:
> >
> > In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
> > call destroy_work_on_stack() which frees the debug object to pair
> > with INIT_WORK_ONSTACK().
> >
> > Signed-off-by: Liu, Chuansheng <[email protected]>
> > ---
> > fs/xfs/xfs_bmap_util.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> > index 1394106..82e0dab 100644
> > --- a/fs/xfs/xfs_bmap_util.c
> > +++ b/fs/xfs/xfs_bmap_util.c
> > @@ -287,6 +287,7 @@ xfs_bmapi_allocate(
> > INIT_WORK_ONSTACK(&args->work, xfs_bmapi_allocate_worker);
> > queue_work(xfs_alloc_wq, &args->work);
> > wait_for_completion(&done);
> > + destroy_work_on_stack(&args->work);
> > return args->result;
> > }
>
> Thanks for your patch and it work fine for my testing. I missed this in an
> old commit: [ 3b876c8f2a xfs: fix debug_object WARN at xfs_alloc_vextent() ]

Looks good to me too.
Reviewed-by: Ben Myers <[email protected]>

2014-01-08 00:57:36

by Liu, Chuansheng

[permalink] [raw]
Subject: RE: [PATCH 2/3] xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()

Hello Jeff,

> -----Original Message-----
> From: Jeff Liu [mailto:[email protected]]
> Sent: Tuesday, January 07, 2014 7:21 PM
> To: Liu, Chuansheng; [email protected]; [email protected]
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH 2/3] xfs: Calling destroy_work_on_stack() to pair with
> INIT_WORK_ONSTACK()
>
> Hi Chuansheng,
>
> On 01/07 2014 16:53 PM, Chuansheng Liu wrote:
> >
> > In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
> > call destroy_work_on_stack() which frees the debug object to pair
> > with INIT_WORK_ONSTACK().
> >
> > Signed-off-by: Liu, Chuansheng <[email protected]>
> > ---
> Thanks for your patch and it work fine for my testing. I missed this in an
> old commit: [ 3b876c8f2a xfs: fix debug_object WARN at xfs_alloc_vextent() ]
Thanks your testing.

>
> Just out of curious, do you notice memory leaks or other hints which help you
> finding out this problem?
I am trying to use the INIT_WORK_ONSTACK() in my code, then I found in some places
Calling destroy_work_on_stack() is missed.

2014-01-12 03:27:26

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 1/3] workqueue: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()

On Tue, Jan 07, 2014 at 04:50:05PM +0800, Chuansheng Liu wrote:
>
> In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to
> call destroy_work_on_stack() which frees the debug object to pair
> with INIT_WORK_ONSTACK().
>
> Signed-off-by: Liu, Chuansheng <[email protected]>

Applied to wq/for-3.14. Thanks.

--
tejun