May the worker function free its struct_work (plus container)?

May the worker function free its struct_work (plus container)?

[Leon Woestenberg]

Hello,

may the worker function kfree() it's own work data structure?

/* worker function */
static void engine_service_work(struct work_struct *work)
{
  struct  foobar_container_struct *foobar;
  foobar = container_of(work, struct foobar, work);
  /* ...perform some work... */
  kfree(foobar);
}

Thanks,
-- 
Leon

Re: May the worker function free its struct_work (plus container)?

[Yong Zhang]

On Mon, May 10, 2010 at 12:47:23AM +0200, Leon Woestenberg wrote:
> Hello,
> 
> may the worker function kfree() it's own work data structure?

Actually, workqueue has no restriction on this kind of operation.
But if you must be careful with the caller in case it will refer
the freed struct.

Thanks,
Yong

Re: May the worker function free its struct_work (plus container)?

[Leon Woestenberg]

Hello Yong,

On Mon, May 10, 2010 at 6:52 AM, Yong Zhang <yong.zhang@windriver.com> wrote:
> On Mon, May 10, 2010 at 12:47:23AM +0200, Leon Woestenberg wrote:
>> Hello,
>>
>> may the worker function kfree() it's own work data structure?
>
> Actually, workqueue has no restriction on this kind of operation.
> But if you must be careful with the caller in case it will refer
> the freed struct.
>
Thanks. Is there API documentation or source code documentation that
confirms this is the case?

I would hate to write code that gets buggy due to the underlying
implementation assuming the work structure is available after the
worker function returned.

Regards,
-- 
Leon

Re: May the worker function free its struct_work (plus container)?

[Yong Zhang]

On Mon, May 10, 2010 at 12:18:38PM +0200, Leon Woestenberg wrote:
> Thanks. Is there API documentation or source code documentation that
> confirms this is the case?

static void run_workqueue(struct cpu_workqueue_struct *cwq)
{
	....

#ifdef CONFIG_LOCKDEP
		/*
		 * It is permissible to free the struct work_struct
		 * from inside the function that is called from it,
		 * this we need to take into account for lockdep too.
		 * To avoid bogus "held lock freed" warnings as well
		 * as problems when looking into work->lockdep_map,
		 * make a copy and use that here.
		 */
		struct lockdep_map lockdep_map = work->lockdep_map;
#endif
	....
}

I think the above comments in run_workqueue() can reflect that
from a certain point.

Thanks,
Yong