linux-kernel - Re: Crashes with 874bbfe600a6 in 3.18.25

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]

Date:	Thu, 4 Feb 2016 08:40:49 +0100
From:	Michal Hocko <mhocko@...nel.org>
To:	Tejun Heo <tj@...nel.org>
Cc:	Jiri Slaby <jslaby@...e.cz>, Thomas Gleixner <tglx@...utronix.de>,
	Petr Mladek <pmladek@...e.com>, Jan Kara <jack@...e.cz>,
	Ben Hutchings <ben@...adent.org.uk>,
	Sasha Levin <sasha.levin@...cle.com>, Shaohua Li <shli@...com>,
	LKML <linux-kernel@...r.kernel.org>, stable@...r.kernel.org,
	Daniel Bilik <daniel.bilik@...system.cz>
Subject: Re: Crashes with 874bbfe600a6 in 3.18.25

On Thu 04-02-16 07:37:23, Michal Hocko wrote:
> On Wed 03-02-16 11:59:01, Tejun Heo wrote:
> > On Wed, Feb 03, 2016 at 05:48:52PM +0100, Michal Hocko wrote:
> [...]
> > > anything and add_timer_on also for WORK_CPU_UNBOUND is really required
> > > then we should at least preserve WORK_CPU_UNBOUND in dwork->cpu so that
> > > __queue_work can actually move on to the local CPU properly and handle
> > > the offline cpu properly.
> > 
> > delayed_work->cpu is determined on queueing time.  Dealing with
> > offlined cpus at execution is completley fine.  There's no need to
> > "preserve" anything.
> 
> I've seen you have posted a fix in the mean time but just for my
> understading. Why the following is not an appropriate fix?
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index c579dbab2e36..52bb11cf20d1 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1459,9 +1459,9 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq,
>  
>  	dwork->wq = wq;
>  	/* timer isn't guaranteed to run in this cpu, record earlier */
> +	dwork->cpu = cpu;
>  	if (cpu == WORK_CPU_UNBOUND)
>  		cpu = raw_smp_processor_id();
> -	dwork->cpu = cpu;
>  	timer->expires = jiffies + delay;
>  
>  	add_timer_on(timer, cpu);

Ok, so after some more thinking about that, this won't really help for
memory less CPU which would still have NUMA_NO_NODE associated with it
AFAIU. So this is definitely better to be handled at unbound_pwq_by_node
level.
-- 
Michal Hocko
SUSE Labs