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

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]
Message-ID: <d82e647a0905132136g355d6ad4y697e58af0a360c45@mail.gmail.com>
Date:	Thu, 14 May 2009 12:36:27 +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_inatomic(v2)

2009/5/14 Cornelia Huck <cornelia.huck@...ibm.com>:
> On Wed, 13 May 2009 23:51:39 +0800,
> tom.leiming@...il.com wrote:
>
>> From: Ming Lei <tom.leiming@...il.com>
>>
>> The purpose of this function is to offer a simple way to schedule an
>> asynchronous thread from an atomic context, because there are not any
>> functions which can create and start a kernel thread in atomic contexts
>> now. We use async_running_no_sync as running list to make callers that
>> don't want to synchronize on cookies, so can avoid some side effects for
>> async_synchronize_full().
>>
>> Part of note for async_run_inatomic and some guideline is from Cornelia Huck.
>>
>> Signed-off-by: Ming Lei <tom.leiming@...il.com>
>> Signed-off-by: Cornelia Huck <cornelia.huck@...ibm.com>
>> ---
>>  include/linux/async.h |    2 ++
>>  kernel/async.c        |   23 +++++++++++++++++++++++
>>  2 files changed, 25 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/async.h b/include/linux/async.h
>> index 18ce92b..9ff62af 100644
>> --- a/include/linux/async.h
>> +++ b/include/linux/async.h
>> @@ -16,6 +16,8 @@
>>  typedef u64 async_cookie_t;
>>  typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
>>
>> +extern int async_run_inatomic(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 8663f5b..cb527b3 100644
>> --- a/kernel/async.c
>> +++ b/kernel/async.c
>> @@ -65,6 +65,7 @@ static async_cookie_t next_cookie = 1;
>>
>>  static LIST_HEAD(async_pending);
>>  static LIST_HEAD(async_running);
>> +static LIST_HEAD(async_running_no_sync);
>>  static DEFINE_SPINLOCK(async_lock);
>>
>>  static int async_enabled = 0;
>> @@ -222,6 +223,28 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data,
>>       return newcookie;
>>  }
>>
>> + /**
>> + * async_run_inatomic - in atomic contexts schedule a function for
>> + *                   asynchronous execution
>
> I'm not 100% sure whether kerneldoc likes split lines for the
> description.
>
> "async_run_inatomic - schedule a function for asynchronous execution from atomic context without checkpointing"
> - but that's really a bit long...
>
>> + *
>> + * @ptr: function to execute asynchronously
>> + * @data: data pointer to pass to the function
>> + *
>> + * Return zero if success, or else return others if failured
>
> Returns zero on success, !zero on failure
>
>> + * Note:
>
> I'd move the description of the function's purpose before Note:.
>
>> + * The purpose of this function is to offer a simple way to schedule an
>> + * asynchronous thread from an atomic context, because there are not any
>> + * functions which can create and start a kernel thread in atomic contexts
>> + * now.
>
> I don't think the second part of the sentence is needed, this should
> only be mentioned in the patch description. I'd rather add
>
> "Since it does not return a cookie for checkpointing, it is for callers
> that don't need later synchronization."
>
>> We use async_running_no_sync as running list to make callers that
>> + * don't want to synchronize on cookies, so can avoid some side effects for
>> + * async_synchronize_full().
>
> "async_run_inatomic() uses a distinct running list in order to avoid
> slowing down synchronization within the general domain."
>
> (The cookie will still be checked when __lowest_in_progress() returns
> the cookie for the async_pending list, but I think that is negligible -
> and it would be a general issue for special domains.)

Yes, it still needs some synchronization before the non-sync function
entry is moved
into async_running_no_sync, but it is negligible really.

>
>> + */
>> +int async_run_inatomic(async_func_ptr *ptr, void *data)
>> +{
>> +     return !__async_schedule(ptr, data, &async_running_no_sync, 1);
>> +}
>> +EXPORT_SYMBOL_GPL(async_run_inatomic);
>> +
>>  /**
>>   * async_schedule - schedule a function for asynchronous execution
>>   * @ptr: function to execute asynchronously
>
> Btw: Do you have any specific use cases for this function in mind?
>

For example request_firmware_nowait(), this function declares it can be
called in contexts where it is not possible to sleep, but really can
not be called
in irq contexts, because the function calls kthread_run to do firmware loading.

-- 
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/