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: <20090514031937.GA5476@nowhere>
Date:	Thu, 14 May 2009 05:19:39 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Cornelia Huck <cornelia.huck@...ibm.com>
Cc:	tom.leiming@...il.com, arjan@...radead.org,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org
Subject: Re: [PATCH] kernel:async function call:introduce
	async_run_inatomic(v2)

On Wed, May 13, 2009 at 07:11:44PM +0200, Cornelia Huck wrote:
> 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.)
> 
> > + */
> > +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?


I have some usecases in mind concerning async_schedule_inatomic()
because it still provides a way to synchronize against pending async
jobs. Especially it could replace some lazy workqueues which receive
rare events.

But I wonder if async_run_inatomic() is really matching any common
pattern inside the kernel.
It seems to me rare to never need waiting for an async job completion.

Frederic.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ