Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752043AbXA2R2J (ORCPT ); Mon, 29 Jan 2007 12:28:09 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752047AbXA2R2I (ORCPT ); Mon, 29 Jan 2007 12:28:08 -0500 Received: from omx2-ext.sgi.com ([192.48.171.19]:36048 "EHLO omx2.sgi.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752043AbXA2R2F (ORCPT ); Mon, 29 Jan 2007 12:28:05 -0500 Date: Mon, 29 Jan 2007 09:27:18 -0800 (PST) From: Christoph Lameter To: Oleg Nesterov cc: Ingo Molnar , Srivatsa Vaddagiri , "Pallipadi, Venkatesh" , Gautham shenoy , Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: slab: start_cpu_timer/cache_reap CONFIG_HOTPLUG_CPU problems In-Reply-To: <20070129171923.GA138@tv-sign.ru> Message-ID: References: <20070129011301.GA844@tv-sign.ru> <20070129171923.GA138@tv-sign.ru> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1711 Lines: 46 On Mon, 29 Jan 2007, Oleg Nesterov wrote: > > > This is wrong. Suppose we have a CPU_UP,CPU_DOWN,CPU_UP sequence. The last > > > CPU_UP will not restart a per-cpu "cache_reap timer". > > > > Why? > > Because the last CPU_UP calls start_cpu_timer(), but since ->work.func != NULL > we don't do schedule_delayed_work_on(). I think (if I am right) this is a slab's > bug. The CPU_DOWN would need to set work.func == NULL for this to work. But then the slab does not shut down the work queues for the processor. Isnt this another issue with workqueues? The slab would need a notification that the workqueue for a processor was shutdown in order to set work.func = NULL. > I think cache_reap() is not alone, and this is not its fault. > > But please note another minor problem, > > void cache_reap(struct work_struct *unused) > { > ... > > schedule_delayed_work(&__get_cpu_var(reap_work), ...); > } > > Even if smp_processor_id() was stable during the execution of cache_reap(), > this work_struct can be moved to another CPU if CPU_DEAD happens. We can't > avoid this, and this is correct. Uhh.... This may not be correct in terms of how the slab operates. > This means that __get_cpu_var(reap_work) returns a "wrong" struct delayed_work. > This is absolutely harmless right now, but may be it's better to use > container_of(unused, struct delayed_work, work). Well seems that we have a set of unresolved issues with workqueues and cpu hotplug. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/