From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] x86: check if desc->action is NULL when unbinding guest pirq Date: Tue, 26 Jan 2010 15:57:29 +0000 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "Cui, Dexuan" , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 22/01/2010 08:01, "Cui, Dexuan" wrote: > Before igb PF driver is unloaded, dom0 doesn't unload igbvf driver > automatically. When igb drver is unloaded, it invokes the > PHYSDEVOP_manage_pci_remove hypercall to remove the VFs and xen frees the msi > irqs by pci_cleanup_msi() -> ... -> dynamic_irq_cleanup() and sets the > desc->action to NULL. > igbvf driver knows the VF is disappearing via a hook ndo_stop() in dev_close() > and tries to unbind the pirq and xen would crash as the desc->action > is NULL now. > The patch adds the checking for this. Although I checked this in as c/s 20844, I now wonder what the 'desc->status|IRQ_DISABLED' is included for? (e.g., see below extract:) + if (unlikely((desc->status | IRQ_DISABLED) && (desc->action == NULL))) Looks pointless: should this just be 'if(desc->action==NULL)'? -- Keir