From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] x86/IRQ: fix create_irq() after c/s 24068:6928172f7ded Date: Fri, 04 Nov 2011 16:34:10 +0000 Message-ID: References: <4EB40D3C020000780005F192@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4EB40D3C020000780005F192@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: andrew.cooper3@citrix.com, "xen-devel@lists.xensource.com" , Haitao Shan , xiantao.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 04/11/2011 15:05, "Jan Beulich" wrote: >>>> On 04.11.11 at 15:41, Keir Fraser wrote: >> On 04/11/2011 11:52, "Jan Beulich" wrote: >> >>> init_one_irq_desc() must be called with interrupts enabled (as it may >>> call functions from the xmalloc() group). Rather than mis-using >>> vector_lock to also protect the finding of an unused IRQ, make this >>> lockless through using cmpxchg(), and obtain the lock only around the >>> actual assignment of the vector. >> >> Looks fine to me. >> >> Acked-by: Keir Fraser >> >>> Also fold find_unassigned_irq() into its only caller. >>> >>> It is, btw, questionable whether create_irq() calling >>> __assign_irq_vector() (rather than assign_irq_vector()) is actually >>> correct - desc->affinity appears to not get initialized properly in >>> this case. > > Any thought on this one? Adjusting this would have the nice side > effect of the function no longer explicitly acquiring vector_lock. I would agree it should call assign_irq_vector(). It was probably only taking the lock itself, and thus using __assign_irq_vector(), to avoid the irq it found in find_unassigned_irq() being stolen. That can't happen now you reserve it via cmpxchg. -- Keir > Jan >