From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755915AbcBDKEh (ORCPT ); Thu, 4 Feb 2016 05:04:37 -0500 Received: from mail-wm0-f49.google.com ([74.125.82.49]:34774 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755647AbcBDKEc (ORCPT ); Thu, 4 Feb 2016 05:04:32 -0500 Message-ID: <1454580263.3407.114.camel@gmail.com> Subject: Re: Crashes with 874bbfe600a6 in 3.18.25 From: Mike Galbraith To: Tejun Heo Cc: Michal Hocko , Jiri Slaby , Thomas Gleixner , Petr Mladek , Jan Kara , Ben Hutchings , Sasha Levin , Shaohua Li , LKML , stable@vger.kernel.org, Daniel Bilik Date: Thu, 04 Feb 2016 11:04:23 +0100 In-Reply-To: <20160203170652.GI14091@mtj.duckdns.org> References: <20160122160903.GH32380@htj.duckdns.org> <1453515623.3734.156.camel@decadent.org.uk> <20160126093400.GV24938@quack.suse.cz> <20160126111438.GA731@pathway.suse.cz> <56B1C9E4.4020400@suse.cz> <20160203122855.GB6762@dhcp22.suse.cz> <20160203162441.GE14091@mtj.duckdns.org> <1454518913.6148.15.camel@gmail.com> <20160203170652.GI14091@mtj.duckdns.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.5 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2016-02-03 at 12:06 -0500, Tejun Heo wrote: > On Wed, Feb 03, 2016 at 06:01:53PM +0100, Mike Galbraith wrote: > > Hm, so it's ok to queue work to an offline CPU? What happens if it > > doesn't come back for an eternity or two? > > Right now, it just loses affinity.... WRT affinity... Somebody somewhere queues a delayed work, a timer is started on CPUX, work is targeted at CPUX. Now wash/rinse/repeat mod_delayed_work() along with migrations. Should __queue_delayed_work() not refrain from altering dwork->cpu once set? I'm also wondering why 22b886dd only applies to kernels >= 4.2. Regardless of the previous CPU a timer was on, add_timer_on() currently simply sets timer->flags to the new CPU. As the caller must be seeing the timer as idle, this is locally fine, but the timer leaving the old base while unlocked can lead to race conditions as follows. Let's say timer was on cpu 0. cpu 0 cpu 1 ----------------------------------------------------------------------------- del_timer(timer) succeeds del_timer(timer) lock_timer_base(timer) locks cpu_0_base add_timer_on(timer, 1) spin_lock(&cpu_1_base->lock) timer->flags set to cpu_1_base operates on @timer operates on @timer What's the difference between... timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu; and... timer_set_base(timer, base); ...that makes that fix unneeded prior to 4.2? We take the same locks in < 4.2 kernels, so seemingly both will diddle concurrently above. -Mike