Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755883AbYH2Acx (ORCPT ); Thu, 28 Aug 2008 20:32:53 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753786AbYH2Acn (ORCPT ); Thu, 28 Aug 2008 20:32:43 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:43619 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753683AbYH2Acm (ORCPT ); Thu, 28 Aug 2008 20:32:42 -0400 Date: Thu, 28 Aug 2008 17:32:01 -0700 From: Andrew Morton To: mgross@linux.intel.com Cc: linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, peterz@infradead.org, rostedt@goodmis.org, mingo@elte.hu, tglx@linutronix.de, arjan@infradead.org, jkacur@gmail.com Subject: Re: [PATCH RFC] pm_qos_requirement might sleep Message-Id: <20080828173201.8912a1f3.akpm@linux-foundation.org> In-Reply-To: <20080828194420.GB6802@linux.intel.com> References: <20080812224926.GA20652@linux.intel.com> <520f0cf10808130124o301b6691ra37ac9007120b9df@mail.gmail.com> <20080814155241.GA31050@linux.intel.com> <1218736137.10800.234.camel@twins> <520f0cf10808141551k283aecb8y647d0f5ae321b81f@mail.gmail.com> <20080825163412.GA21910@linux.intel.com> <1219682129.8515.81.camel@twins> <520f0cf10808260148k47368b71he2737ea1a59bbe4d@mail.gmail.com> <20080826161802.GB9862@linux.intel.com> <520f0cf10808261045v9dddcdcnd1a86b224aa3feb0@mail.gmail.com> <20080828194420.GB6802@linux.intel.com> X-Mailer: Sylpheed version 2.2.4 (GTK+ 2.8.20; i486-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4463 Lines: 129 On Thu, 28 Aug 2008 12:44:20 -0700 mark gross wrote: > From: John Kacur I tend to deobfuscate email addresses when preparing the changelogs. Not sure why, really, and I feel a bit guilty about doing it (hey, you can forward the resulting spam to me if you like). One reason I guess is to avoid breaking all the emails which my scripts will automatically send out as the patch goes through its little lifecycle. Some of which can sometimes be quite important. > Patch to make PM_QOS and CPU_IDLE play nicer when ran with the > RT-Preempt kernel. CPU_IDLE polls the target_value's of some of the > pm_qos parameters from the idle loop causing sleeping locking > warnings. Changing the target_value to an atomic avoids this issue. > > Signed-off-by: mark gross > > Thanks, > > --mgross > > > Remove the spinlock in pm_qos_requirement by making target_value an atomic type. > This is necessary for real-time since pm_qos_requirement is called by idle and > cannot be allowed to sleep. > Signed-off-by: John Kacur As usual when I get two different changelogs, I create a masterpiece which is better than either of the originals! > Index: linux-2.6/kernel/pm_qos_params.c > =================================================================== > --- linux-2.6.orig/kernel/pm_qos_params.c 2008-08-08 07:52:01.000000000 -0700 > +++ linux-2.6/kernel/pm_qos_params.c 2008-08-28 12:14:55.000000000 -0700 > @@ -43,7 +43,7 @@ > #include > > /* > - * locking rule: all changes to target_value or requirements or notifiers lists > + * locking rule: all changes to requirements or notifiers lists > * or pm_qos_object list and pm_qos_objects need to happen with pm_qos_lock > * held, taken with _irqsave. One lock to rule them all > */ > @@ -66,7 +66,7 @@ > struct miscdevice pm_qos_power_miscdev; > char *name; > s32 default_value; > - s32 target_value; > + atomic_t target_value; > s32 (*comparitor)(s32, s32); > }; > > @@ -77,7 +77,7 @@ > .notifiers = &cpu_dma_lat_notifier, > .name = "cpu_dma_latency", > .default_value = 2000 * USEC_PER_SEC, > - .target_value = 2000 * USEC_PER_SEC, > + .target_value = ATOMIC_INIT(2000 * USEC_PER_SEC), > .comparitor = min_compare > }; > > @@ -87,7 +87,7 @@ > .notifiers = &network_lat_notifier, > .name = "network_latency", > .default_value = 2000 * USEC_PER_SEC, > - .target_value = 2000 * USEC_PER_SEC, > + .target_value = ATOMIC_INIT(2000 * USEC_PER_SEC), > .comparitor = min_compare > }; > > @@ -99,7 +99,7 @@ > .notifiers = &network_throughput_notifier, > .name = "network_throughput", > .default_value = 0, > - .target_value = 0, > + .target_value = ATOMIC_INIT(0), > .comparitor = max_compare > }; > > @@ -150,11 +150,11 @@ > extreme_value = pm_qos_array[target]->comparitor( > extreme_value, node->value); > } > - if (pm_qos_array[target]->target_value != extreme_value) { > + if (atomic_read(&pm_qos_array[target]->target_value) != extreme_value) { > call_notifier = 1; > - pm_qos_array[target]->target_value = extreme_value; > + atomic_set(&pm_qos_array[target]->target_value, extreme_value); > pr_debug(KERN_ERR "new target for qos %d is %d\n", target, > - pm_qos_array[target]->target_value); > + atomic_read(&pm_qos_array[target]->target_value)); > } > spin_unlock_irqrestore(&pm_qos_lock, flags); > > @@ -193,14 +193,7 @@ > */ > int pm_qos_requirement(int pm_qos_class) > { > - int ret_val; > - unsigned long flags; > - > - spin_lock_irqsave(&pm_qos_lock, flags); > - ret_val = pm_qos_array[pm_qos_class]->target_value; > - spin_unlock_irqrestore(&pm_qos_lock, flags); > - > - return ret_val; > + return atomic_read(&pm_qos_array[pm_qos_class]->target_value); > } > EXPORT_SYMBOL_GPL(pm_qos_requirement); OK, so I've stared and stared. This patch doesn't do anything. I'm suspecting that the rt patch might be doing something silly, like thinking that it needs to take pm_qos_lock to read a single word from memory, or something like that. But this patch is simply not understandable given the amount of information which you guys have provided. Help? -- 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/