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

[Frederic Weisbecker <fweisbec@gmail.com>]

On Tue, May 12, 2009 at 06:31:05PM +0200, Cornelia Huck wrote:
> On Tue, 12 May 2009 18:04:35 +0200,
> Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > 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:
> > > >   * 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.
> 
> I'm wondering whether this is not mixing two different things up:
> - Making async_schedule_* safe from an atomic context.


It was already safe from atomic context, async_schedule() internals
does not make use of sleeping functions.
So indeed the inatomic suffix is not that much appropriate from
an async internal point in view, it makes sense only from the
caller/callee point of view.

But if we start worrying about the GFP flags used by __async_schedule(),
then yes we are mixing up two things.


> - Disallowing calling the function synchronously if asynchronous
>   scheduling failed.
> 
> Perhaps we want async_schedule_nosync() in addition?



This division would make more sense indeed.

- async_schedule_inatomic() would be nosync() and would use
  GFP_ATOMIC. I guess the case where we want to run
  a job synchronously from atomic in case of async failure is too rare
  (non-existent?).
- async_schedule_nosync() would be only nosync() and would use
  GFP_KERNEL

I'm not sure the second case will ever be used though.

Another alternative would be to define a single async_schedule_nosync()
which also takes a gfp flag.


> > 
> > BTW, now that we have an atomic safe version, may be we could
> > also adapt the kmalloc GFP flags subsequently?
> 
> Yes, that would make sense.