linux-kernel - Re: [PATCH v4 07/22] kthread: Detect when a kthread work is used by more workers

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160125185747.GC3628@mtj.duckdns.org>
Date:	Mon, 25 Jan 2016 13:57:47 -0500
From:	Tejun Heo <tj@...nel.org>
To:	Petr Mladek <pmladek@...e.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Josh Triplett <josh@...htriplett.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jiri Kosina <jkosina@...e.cz>, Borislav Petkov <bp@...e.de>,
	Michal Hocko <mhocko@...e.cz>, linux-mm@...ck.org,
	Vlastimil Babka <vbabka@...e.cz>, linux-api@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 07/22] kthread: Detect when a kthread work is used by
 more workers

On Mon, Jan 25, 2016 at 04:44:56PM +0100, Petr Mladek wrote:
> +static void insert_kthread_work_sanity_check(struct kthread_worker *worker,
> +					       struct kthread_work *work)
> +{
> +	lockdep_assert_held(&worker->lock);
> +	WARN_ON_ONCE(!irqs_disabled());

Isn't worker->lock gonna be a irq-safe lock?  If so, why would this
need to be tested separately?

> +	WARN_ON_ONCE(!list_empty(&work->node));
> +	/* Do not use a work with more workers, see queue_kthread_work() */
> +	WARN_ON_ONCE(work->worker && work->worker != worker);
> +}

Is this sanity check function gonna be used from multiple places?

>  /* insert @work before @pos in @worker */
>  static void insert_kthread_work(struct kthread_worker *worker,
> -			       struct kthread_work *work,
> -			       struct list_head *pos)
> +				struct kthread_work *work,
> +				struct list_head *pos)
>  {
> -	lockdep_assert_held(&worker->lock);
> +	insert_kthread_work_sanity_check(worker, work);
>  
>  	list_add_tail(&work->node, pos);
>  	work->worker = worker;
> @@ -717,6 +730,15 @@ static void insert_kthread_work(struct kthread_worker *worker,
>   * Queue @work to work processor @task for async execution.  @task
>   * must have been created with kthread_worker_create().  Returns %true
>   * if @work was successfully queued, %false if it was already pending.
> + *
> + * Never queue a work into a worker when it is being processed by another
> + * one. Otherwise, some operations, e.g. cancel or flush, will not work
> + * correctly or the work might run in parallel. This is not enforced
> + * because it would make the code too complex. There are only warnings
> + * printed when such a situation is detected.

I'm not sure the above paragraph adds much.  It isn't that accurate to
begin with as what's being disallowed is larger scope than the above.
Isn't the paragraph below enough?

> + * Reinitialize the work if it needs to be used by another worker.
> + * For example, when the worker was stopped and started again.
>   */
>  bool queue_kthread_work(struct kthread_worker *worker,
>  			struct kthread_work *work)
> -- 
> 1.8.5.6
> 

-- 
tejun