lkml.org 
[lkml]   [2016]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v8 02/12] kthread: Kthread worker API cleanup
On Mon, 13 Jun 2016 17:13:53 +0200
Petr Mladek <pmladek@suse.com> 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

\
 
 \ /
  Last update: 2016-06-13 18:41    [W:1.016 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site