From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH] Avoid panic when adjusting sedf parameters Date: Thu, 17 Nov 2011 15:41:11 +0100 Message-ID: <4EC51D07.2030302@ts.fujitsu.com> References: <4EC51489.4090609@ts.fujitsu.com> <4EC528FD0200007800061983@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4EC528FD0200007800061983@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: Keir Fraser , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 11/17/2011 03:32 PM, Jan Beulich wrote: >>>> On 17.11.11 at 15:04, Juergen Gross wrote: >> On 11/17/2011 02:52 PM, Keir Fraser wrote: >>> On 17/11/2011 13:30, "Jan Beulich" wrote: >>> >>>> which would now associate the else with the wrong (inner) if. One >>>> possible solution that comes to mind would be >>>> >>>> #define for_each_domain_in_cpupool(_d,_c) \ >>>> for_each_domain_in_cpupool (_d) \ >>>> if ((_d)->cpupool != (_c)) \ >>>> continue; \ >>>> else >>>> >>>> but I think I had seen a more clever solution to this problem, but cannot >>>> remember/locate it right now. >>> Given the gcc ({}) construction, you could do a double-loop: >>> for ( (_d) = rcu_dereference(domain_list); \ >>> (_d) != NULL; \ >>> ({ while ((_d) = rcu_dereference((_d)->next_in_list != NULL) >>> if ((_d)->cpupool == (_c)) break; >>> (_d); }) ) >>> >>> A bit ugly. ;-) And I still worry about cpupool locking... >> What about: >> >> static inline struct domain *next_domain_in_cpupool( >> struct domain *d, struct cpupool *c) >> { >> for (d = rcu_dereference(d->next_in_list); d&& d->cpupool != c; >> d = rcu_dereference(d->next_in_list)); >> return d; >> } >> >> #define for_each_domain_in_cpupool(_d,_c) \ >> for ( (_d) = rcu_dereference(domain_list); \ >> (_d) != NULL; \ >> (_d) = next_domain_in_cpupool((_d), (_c))) > Same problem as with Keir's variant - you'd enter the loop body for > the first domain on the list regardless of its cpupool. But with a > first_domain_in_cpupool() counterpart this might be usable. Or, as > said in the other reply, putting a more complex construct in the > middle clause. I think adding first_domain_in_cpupool() is a good way to go. Sending out adjusted patch with appropriate locking added in cpupool_unassign_cpu() soon. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html