| 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
| ||
|
Message-ID: <CAJhGHyA3zte-0MWGC+DrOptprHBRroCn8fHM678iVnb-1VStLA@mail.gmail.com>
Date: Fri, 16 Mar 2018 14:01:35 +0800
From: Lai Jiangshan <jiangshanlai+lkml@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Jann Horn <jannh@...gle.com>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Benjamin LaHaise <bcrl@...ck.org>,
Al Viro <viro@...iv.linux.org.uk>,
Kent Overstreet <kent.overstreet@...il.com>,
security@...nel.org, LKML <linux-kernel@...r.kernel.org>,
kernel-team@...com
Subject: Re: [PATCH 6/8] RCU, workqueue: Implement rcu_work
On Thu, Mar 15, 2018 at 3:45 AM, Tejun Heo <tj@...nel.org> wrote:
> There are cases where RCU callback needs to be bounced to a sleepable
> context. This is currently done by the RCU callback queueing a work
> item, which can be cumbersome to write and confusing to read.
>
> This patch introduces rcu_work, a workqueue work variant which gets
> executed after a RCU grace period, and converts the open coded
> bouncing in fs/aio and kernel/cgroup.
>
> v3: Dropped queue_rcu_work_on(). Documented rcu grace period behavior
> after queue_rcu_work().
>
> v2: Use rcu_barrier() instead of synchronize_rcu() to wait for
> completion of previously queued rcu callback as per Paul.
>
> Signed-off-by: Tejun Heo <tj@...nel.org>
> Cc: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> ---
> include/linux/workqueue.h | 23 ++++++++++++++++++++
> kernel/workqueue.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index bc0cda1..d026f8f 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -13,6 +13,7 @@
> #include <linux/threads.h>
> #include <linux/atomic.h>
> #include <linux/cpumask.h>
> +#include <linux/rcupdate.h>
>
> struct workqueue_struct;
>
> @@ -120,6 +121,14 @@ struct delayed_work {
> int cpu;
> };
>
> +struct rcu_work {
> + struct work_struct work;
> + struct rcu_head rcu;
> +
> + /* target workqueue ->rcu uses to queue ->work */
> + struct workqueue_struct *wq;
> +};
> +
> /**
> * struct workqueue_attrs - A struct for workqueue attributes.
> *
> @@ -151,6 +160,11 @@ static inline struct delayed_work *to_delayed_work(struct work_struct *work)
> return container_of(work, struct delayed_work, work);
> }
>
> +static inline struct rcu_work *to_rcu_work(struct work_struct *work)
> +{
> + return container_of(work, struct rcu_work, work);
> +}
> +
> struct execute_work {
> struct work_struct work;
> };
> @@ -266,6 +280,12 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
> #define INIT_DEFERRABLE_WORK_ONSTACK(_work, _func) \
> __INIT_DELAYED_WORK_ONSTACK(_work, _func, TIMER_DEFERRABLE)
>
> +#define INIT_RCU_WORK(_work, _func) \
> + INIT_WORK(&(_work)->work, (_func))
> +
> +#define INIT_RCU_WORK_ONSTACK(_work, _func) \
> + INIT_WORK_ONSTACK(&(_work)->work, (_func))
> +
> /**
> * work_pending - Find out whether a work item is currently pending
> * @work: The work item in question
> @@ -447,6 +467,7 @@ extern bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
> struct delayed_work *work, unsigned long delay);
> extern bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq,
> struct delayed_work *dwork, unsigned long delay);
> +extern bool queue_rcu_work(struct workqueue_struct *wq, struct rcu_work *rwork);
>
> extern void flush_workqueue(struct workqueue_struct *wq);
> extern void drain_workqueue(struct workqueue_struct *wq);
> @@ -463,6 +484,8 @@ extern bool flush_delayed_work(struct delayed_work *dwork);
> extern bool cancel_delayed_work(struct delayed_work *dwork);
> extern bool cancel_delayed_work_sync(struct delayed_work *dwork);
>
> +extern bool flush_rcu_work(struct rcu_work *rwork);
> +
> extern void workqueue_set_max_active(struct workqueue_struct *wq,
> int max_active);
> extern struct work_struct *current_work(void);
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index bb9a519..7df85fa 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1604,6 +1604,40 @@ bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq,
> }
> EXPORT_SYMBOL_GPL(mod_delayed_work_on);
>
> +static void rcu_work_rcufn(struct rcu_head *rcu)
> +{
> + struct rcu_work *rwork = container_of(rcu, struct rcu_work, rcu);
> +
> + /* read the comment in __queue_work() */
> + local_irq_disable();
> + __queue_work(WORK_CPU_UNBOUND, rwork->wq, &rwork->work);
> + local_irq_enable();
> +}
> +
> +/**
> + * queue_rcu_work - queue work after a RCU grace period
> + * @wq: workqueue to use
> + * @rwork: work to queue
> + *
> + * Return: %false if @rwork was already pending, %true otherwise. Note
> + * that a full RCU grace period is guaranteed only after a %true return.
> + * While @rwork is guarnateed to be executed after a %false return, the
> + * execution may happen before a full RCU grace period has passed.
> + */
LGTM
Reviewed-by: Lai Jiangshan <jiangshanlai@...il.com>
> +bool queue_rcu_work(struct workqueue_struct *wq, struct rcu_work *rwork)
> +{
> + struct work_struct *work = &rwork->work;
> +
> + if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) {
> + rwork->wq = wq;
> + call_rcu(&rwork->rcu, rcu_work_rcufn);
> + return true;
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL(queue_rcu_work);
> +
> /**
> * worker_enter_idle - enter idle state
> * @worker: worker which is entering idle state
> @@ -3001,6 +3035,26 @@ bool flush_delayed_work(struct delayed_work *dwork)
> }
> EXPORT_SYMBOL(flush_delayed_work);
>
> +/**
> + * flush_rcu_work - wait for a rwork to finish executing the last queueing
> + * @rwork: the rcu work to flush
> + *
> + * Return:
> + * %true if flush_rcu_work() waited for the work to finish execution,
> + * %false if it was already idle.
> + */
> +bool flush_rcu_work(struct rcu_work *rwork)
> +{
> + if (test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&rwork->work))) {
> + rcu_barrier();
> + flush_work(&rwork->work);
> + return true;
A possible tiny improvement: check if it was already queued on wq.
For example:
if (test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&rwork->work))) {
if (!flush_work(&rwork->work)) {
rcu_barrier();
flush_work(&rwork->work);
}
return true;
> + } else {
> + return flush_work(&rwork->work);
> + }
> +}
> +EXPORT_SYMBOL(flush_rcu_work);
> +
> static bool __cancel_work(struct work_struct *work, bool is_dwork)
> {
> unsigned long flags;
> --
> 2.9.5
>
Powered by blists - more mailing lists