Re: [PATCH v8 02/12] kthread: Kthread worker API cleanup

On Mon, 13 Jun 2016 17:13:53 +0200
Petr Mladek [off-list ref] wrote:

OK, all wants to keep DEFINE stuff as is:

  DEFINE_KTHREAD_WORKER()		stay
  DEFINE_KTHREAD_WORK()			stay
  DEFINE_KTHREAD_WORKER_ONSTACK()	stay
  DEFINE_KTHREAD_WORKER_ONSTACK()	stay


Nobody was against renaming the non-init functions:

  insert_kthread_work()		-> kthread_insert_work()
  queue_kthread_work()		-> kthread_queue_work()
  flush_kthread_work()		-> kthread_flush_work()
  flush_kthread_worker()	-> kthread_flush_worker()

Yep.

Now, the question seem to be the init() functions.
Andrew would prefer:

  __init_kthread_worker()	-> __kthread_worker_init()
  init_kthread_worker()		-> kthread_worker_init()
  init_kthread_work()		-> kthread_work_init()

AFAIK, Steven would prefer to keep it

  __init_kthread_worker()	stay as is
  init_kthread_worker()		stay as is
  init_kthread_work()		stay as is

I would personally prefer the way from this patch:

  __init_kthread_worker()	-> __kthread_init_worker()
  init_kthread_worker()		-> kthread_init_worker()
  init_kthread_work()		-> kthread_init_work()


I have several reasons:

1. The init functions will be used close to the other functions in
   the code. It will be easier if all functions use the same
   naming scheme. Here are some snippets:

	kthread_init_work(&w_data->balancing_work, clamp_balancing_func);
	kthread_init_delayed_work(&w_data->idle_injection_work,
				  clamp_idle_injection_func);
	kthread_queue_work(w_data->worker, &w_data->balancing_work);

   or

	kthread_init_delayed_work(&kmemleak_scan_work, kmemleak_scan_func);
	kmemleak_scan_worker = kthread_create_worker(0, "kmemleak");


2. We are going to add kthread_destroy_worker() which would need
   to be another exception. Also this function will be used together
   with the others, for example:

	kthread_cancel_delayed_work_sync(&rb_producer_hammer_work);
	kthread_destroy_worker(rb_producer_worker);

   Also here the same naming scheme will help.


3. It is closer to the workqueues API, so it reduces confusion.

Using workqueues as an example of "reduces confusion" is not the most
convincing argument ;-)

4. Note that there are already several precedents, for example:

	amd_iommu_init_device()
	free_area_init_node()
	jump_label_init_type()
	regmap_init_mmio_clk()


Andrew, Steven, are you really so strongly against my version
of the init functions, please?

I don't really have that strong opinion on the "init" part. I was much
more concerned about the DEFINE/DECLARE macros.

-- Steve