linux-kernel - Re: [PATCH] kernel:async function call:introduce async

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 13 May 2009 20:56:40 +0800
From:	Ming Lei <tom.leiming@...il.com>
To:	Cornelia Huck <cornelia.huck@...ibm.com>
Cc:	arjan@...radead.org, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org
Subject: Re: [PATCH] kernel:async function call:introduce async_run

2009/5/13 Cornelia Huck <cornelia.huck@...ibm.com>:
> On Wed, 13 May 2009 08:33:49 +0800,
> tom.leiming@...il.com wrote:
>
>> From: Ming Lei <tom.leiming@...il.com>
>>
>> In fact, some async function call does not need to check, async_run
>> is right for this kind of function call. Also, async_run is very
>> suitable to used to start a thread in atomic context to do somthing,
>> for now there is no such kind of kernel api, eg. kthread_run
>> can not be started in atomic context.
>
> Could you rework your explanation a bit? If I understand correctly, you
> want to introduce a way to queue an async thread for those callers that
> don't want to synchronize on cookies.
>
>>
>> This patch is againt my another patch:
>>    kernel/async.c:introduce async_schedule*_atomic
>> please review.
>>
>> Signed-off-by: Ming Lei <tom.leiming@...il.com>
>> ---
>>  include/linux/async.h |    1 +
>>  kernel/async.c        |   24 +++++++++++++++++++++++-
>>  2 files changed, 24 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/async.h b/include/linux/async.h
>> index ede9849..5390572 100644
>> --- a/include/linux/async.h
>> +++ b/include/linux/async.h
>> @@ -16,6 +16,7 @@
>>  typedef u64 async_cookie_t;
>>  typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
>>
>> +extern void async_run(async_func_ptr *ptr, void *data);
>>  extern async_cookie_t async_schedule(async_func_ptr *ptr, void *data);
>>  extern async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
>>                                           struct list_head *list);
>> diff --git a/kernel/async.c b/kernel/async.c
>> index 6bf565b..9cc4670 100644
>> --- a/kernel/async.c
>> +++ b/kernel/async.c
>> @@ -60,6 +60,7 @@ asynchronous and synchronous parts of the kernel.
>>
>>  static async_cookie_t next_cookie = 1;
>>
>> +#define MAX_COOKIE   (~0)
>>  #define MAX_THREADS  256
>>  #define MAX_WORK     32768
>>
>> @@ -207,7 +208,10 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
>>       entry->running = running;
>>
>>       spin_lock_irqsave(&async_lock, flags);
>> -     newcookie = entry->cookie = next_cookie++;
>> +     if (atomic == 1)
>> +             newcookie = entry->cookie = next_cookie++;
>> +     else
>> +             newcookie = entry->cookie = MAX_COOKIE;
>
> This confuses me. Why do you change the behaviour for atomic == 0?

Yes, it needs to be fixed.

>
>>       list_add_tail(&entry->list, &async_pending);
>>       atomic_inc(&entry_count);
>>       spin_unlock_irqrestore(&async_lock, flags);
>> @@ -216,6 +220,24 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, \
>>  }
>>
>>  /**
>> + * async_run - schedule a function for asynchronous execution
>> + * @ptr: function to execute asynchronously
>> + * @data: data pointer to pass to the function
>> + *
>> + * Note:we do not allocate a cookie for this kind of aysnchronous
>> + * function to decrease the wait time of async_synchronize_full().
>
> But async_synchronize_full() still waits for list_empty(&async_running)
> - so what does this buy us?

I mean it can decrease the wait time for other async function.
async_schedule() still can be used to do such thing, but may lead to a
slower boot.  It is the main
purpose of the patch.

>
>> + * In fact, some function does not need to check, async_run is right
>> + * for this kind of function call. Also, async_run is very suitable to
>> + * start a thread to do somthing in atomic context,for now there is no such
>> + * kind of kernel api, eg. kthread_run can not be run in atomic context.
>
> Hm...
> "The purpose of this function is to offer a simple way to schedule an
> asynchronous thread, especially from an atomic context."
> Would that describe async_run() better?

Yes, very good, please forgive my poor english.

Thanks!

-- 
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/