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] [day] [month] [year] [list]
Message-Id: <20250222100716.23c55ef04d48d313211bef21@kernel.org>
Date: Sat, 22 Feb 2025 10:07:16 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Waiman Long <llong@...hat.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Will Deacon <will@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
 Boqun Feng <boqun.feng@...il.com>, Joel Granados
 <joel.granados@...nel.org>, Anna Schumaker <anna.schumaker@...cle.com>,
 Lance Yang <ioworker0@...il.com>, Kent Overstreet
 <kent.overstreet@...ux.dev>, Yongliang Gao <leonylgao@...cent.com>, Steven
 Rostedt <rostedt@...dmis.org>, Tomasz Figa <tfiga@...omium.org>, Sergey
 Senozhatsky <senozhatsky@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] hung_task: Show the blocker task if the task is
 hung on mutex

On Fri, 21 Feb 2025 15:29:57 -0500
Waiman Long <llong@...hat.com> wrote:

> 
> On 2/21/25 1:59 PM, Peter Zijlstra wrote:
> > On Fri, Feb 21, 2025 at 11:30:01PM +0900, Masami Hiramatsu (Google) wrote:
> >> +static void debug_show_blocker(struct task_struct *task)
> >> +{
> >> +	struct task_struct *g, *t;
> >> +	unsigned long owner;
> >> +	struct mutex *lock;
> >> +
> >> +	lock = READ_ONCE(task->blocker_mutex);
> >> +	if (!lock)
> >> +		return;
> >> +
> >> +	owner = mutex_get_owner(lock);
> >> +	if (likely(owner)) {
> >> +		/* Ensure the owner information is correct. */
> >> +		for_each_process_thread(g, t)
> >> +			if ((unsigned long)t == owner) {
> >> +				pr_err("INFO: task %s:%d is blocked on a mutex owned by task %s:%d.\n",
> >> +					task->comm, task->pid, t->comm, t->pid);
> >> +				sched_show_task(t);
> >> +				return;
> >> +			}
> >   - that for_each_process_thread() scope needs { }

OK, let me add it.

> >
> >   - that for_each_process_thread() loop needs RCU or tasklist_lock
> 
> The call chain should be
> 
> check_hung_uninterruptible_tasks()
>    -> check_hung_task()
>      -> debug_show_blocker()
> 
> check_hung_uninterruptible_tasks() takes rcu_read_lock() before calling 
> check_hung_task(). Perhaps add a statement like
> 
>      RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "no rcu lock held");

Agreed. It needs to add  assertion.

> 
> >
> >   - there is no saying that the owner you read with mutex_get_owner() is
> >     still the owner by the time you do the compare, there can have been
> >     owner changes.
> 
> Maybe change "owned by" to "likely owned by" :-)

Indeed :-)

Thank you!

> 
> Cheers,
> Longman
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ