Re: [PATCH] kernel/async.c:introduce async_schedule*_atomic

[Frederic Weisbecker <fweisbec@gmail.com>]

On Tue, May 12, 2009 at 05:44:58PM +0200, Frederic Weisbecker wrote:
> On Tue, May 12, 2009 at 11:13:42PM +0800, tom.leiming@gmail.com wrote:
> > From: Ming Lei <tom.leiming@gmail.com>
> > 
> > The async_schedule* may not be called in atomic contexts if out of
> > memory or if there's too much work pending already, because the
> > async function to be called may sleep.
> > 
> > This patch fixes the comment of async_schedule*, and introduces
> > async_schedules*_atomic to allow them called from atomic contexts
> > safely.



Note that async_schedule_atomic is a confusing name.
At a first glance, it could mean that the scheduled job
will be run atomically.

I would suggest async_schedule_inatomic() so that it follows the common
naming pattern in use in the kernel, eg:

- copy_from_user_inatomic()
- futex_atomic_cmpxchg_inatomic()

and so on.



> >   * Returns an async_cookie_t that may be used for checkpointing later.
> > - * Note: This function may be called from atomic or non-atomic contexts.
> > + * Note:This function may be called from non-atomic contexts,and not
> > + * 	called from atomic contexts with safety. Please use
> > + * 	async_schedule_atomic in atomic contexts.


I suggest to add a comment which explains the reason for which it is unsafe
to call it in atomic context: because the scheduled work might be synchronously
executed.

One could believe this is because async_schedule() internally uses
a function which might sleep whereas the actual problem may come
from the scheduled function.

BTW, now that we have an atomic safe version, may be we could
also adapt the kmalloc GFP flags subsequently?


Frederic.