Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757461AbYGZTtm (ORCPT ); Sat, 26 Jul 2008 15:49:42 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753864AbYGZTte (ORCPT ); Sat, 26 Jul 2008 15:49:34 -0400 Received: from wa-out-1112.google.com ([209.85.146.179]:47412 "EHLO wa-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753517AbYGZTte (ORCPT ); Sat, 26 Jul 2008 15:49:34 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=EfOCTMdUVfdZy6v3Zdp9iq4dP5KV0uAodqV13sLWe3ARrOsU84fIw3ino+f/WYzV6R py6B0yHB4lw0OxINHJucnXbk3b8GjL4YRn+aHXIrYJ1zRAS96dMf549VBVWrVtLmaBvw THrjxPBMLxxHftgWr9N9+uA/J0VCOT5YNXgPk= Message-ID: Date: Sat, 26 Jul 2008 21:49:33 +0200 From: "Dmitry Adamushko" To: "Peter Zijlstra" Subject: Re: [patch, rfc: 2/2] sched, hotplug: ensure a task is on the valid cpu after set_cpus_allowed_ptr() Cc: "Ingo Molnar" , LKML In-Reply-To: <1216993184.7257.388.camel@twins> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <1216937730.5368.16.camel@earth> <1216989649.7257.381.camel@twins> <1216993184.7257.388.camel@twins> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3592 Lines: 94 2008/7/25 Peter Zijlstra : > On Fri, 2008-07-25 at 15:20 +0200, Dmitry Adamushko wrote: >> 2008/7/25 Peter Zijlstra : >> > On Fri, 2008-07-25 at 00:15 +0200, Dmitry Adamushko wrote: >> >> >> >> From: Dmitry Adamushko >> >> Subject: sched, hotplug: ensure a task is on the valid cpu after >> >> set_cpus_allowed_ptr() >> >> >> >> --- >> >> sched, hotplug: ensure a task is on the valid cpu after set_cpus_allowed_ptr() >> >> >> >> The 'new_mask' may not include task_cpu(p) so we migrate 'p' on another 'cpu'. >> >> In case it can't be placed on this 'cpu' immediately, we submit a request >> >> to the migration thread and wait for its completion. >> >> >> >> Now, by the moment this request gets handled by the migration_thread, >> >> 'cpu' may well be offline/non-active. As a result, 'p' continues >> >> running on its old cpu which is not in the 'new_mask'. >> >> >> >> Fix it: ensure 'p' ends up on a valid cpu. >> >> >> >> Theoreticaly (but unlikely), we may get an endless loop if someone cpu_down()'s >> >> a new cpu we have choosen on each iteration. >> >> >> >> Alternatively, we may introduce a special type of request to migration_thread, >> >> namely "move_to_any_allowed_cpu" (e.g. by specifying dest_cpu == -1). >> >> >> >> Note, any_active_cpu() instead of any_online_cpu() would be better here. >> > >> > Hrmm,.. this is all growing into something of a mess.. defeating the >> > whole purpose of introducing that cpu_active_map stuff. >> > >> > Would the suggested SRCU logic simplify all this? >> >> Ah, wait a second. >> >> sched_setaffinity() -> set_cpus_allowed_ptr() is ok vs. cpu_down() as >> it does use get_online_cpus(). So none of the cpus can become offline >> while we are in set_cpus_allowed_ptr(). >> >> but there are numerous calls to set_cpus_allowed_ptr() from other >> places and not all of them seem to call get_online_cpus()... >> >> yeah, I should check this issue again.. >> >> btw., indeed all these different sync. cases are a bit of mess. > > Will ponder it a bit more, but my brain can't seem to let go of SRCU > now.. I like it too. > I'll go concentrate on making the swap-over-nfs patches prettier, > maybe that will induce a brainwave ;-) what's about task-migration over NFS? ;-) >> btw., I was wondering about this change: >> >> ba42059fbd0aa1ac91b582412b5fedb1258f241f >> >> sched: hrtick_enabled() should use cpu_active() >> >> Peter pointed out that hrtick_enabled() should use cpu_active(). > > What exactly were you wondering about? > > It seemed a good idea to stop starting hrtimers before we migrate them > to another cpu (one of the things done later in cpu_down), thereby > avoiding spurious fires on remote cpus. > Yeah, I thought that it's likely cpu_down() related. I looked at it from the point of cpu_up(), e.g. a cpu is online -> tasks get queued and start running (while cpu is still _not_ active for a while). So when they get enqueued first time, hrtick_enabled() wil give 0 and hr-timer won't be used. Actually, cpu_active_map has already broken expectations/assumptions - http://lkml.org/lkml/2008/7/24/260 (in case you have missed it). But this particular "microcode"s behavior is really bad, I think. -- Best regards, Dmitry Adamushko -- 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/