WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xen/pci: make bus notifier handler return sane v

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xen/pci: make bus notifier handler return sane values
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Mon, 22 Aug 2011 12:20:35 -0400
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 22 Aug 2011 09:22:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E4B98C00200007800051A56@xxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4E4B98C00200007800051A56@xxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Aug 17, 2011 at 09:32:32AM +0100, Jan Beulich wrote:
> Notifier functions are expected to return NOTIFY_* codes, not -E...
> ones. In particular, since the respective hypercalls failing is not
> fatal to the operation of the Dom0 kernel, it must be avoided to
> return negative values here as those would make it appear as if
> NOTIFY_STOP_MASK wa set, suppressing further notification calls to
> other interested parties (which is also why we don't want to use
> notifier_from_errno() here).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> ---
>  drivers/xen/pci.c |   11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> --- 3.1-rc2/drivers/xen/pci.c
> +++ 3.1-rc2-xen-pci-bus-notifier/drivers/xen/pci.c
> @@ -86,23 +86,22 @@ static int xen_pci_notifier(struct notif
>                           unsigned long action, void *data)
>  {
>       struct device *dev = data;
> -     int r = 0;
>  
>       switch (action) {
>       case BUS_NOTIFY_ADD_DEVICE:
> -             r = xen_add_device(dev);
> +             xen_add_device(dev);
>               break;
>       case BUS_NOTIFY_DEL_DEVICE:
> -             r = xen_remove_device(dev);
> +             xen_remove_device(dev);
>               break;
>       default:
> -             break;
> +             return NOTIFY_DONE;
>       }
>  
> -     return r;
> +     return NOTIFY_OK;
>  }
>  
> -struct notifier_block device_nb = {
> +static struct notifier_block device_nb = {
>       .notifier_call = xen_pci_notifier,
>  };
>  
> 

Somehow I lost the email thread.. but what if we did this:

diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c
index cef4baf..11e231c 100644
--- a/drivers/xen/pci.c
+++ b/drivers/xen/pci.c
@@ -96,13 +96,19 @@ static int xen_pci_notifier(struct notifier_block *nb,
                r = xen_remove_device(dev);
                break;
        default:
-               break;
+               return NOTIFY_DONE;
        }
-
-       return r;
+       if (r) {
+               dev_err(dev, "Failed to %s - passthrough or MSI/MSI-X might 
fail!\n",
+                       action == BUS_NOTIFY_ADD_DEVICE ? "add" :
+                       (action == BUS_NOTIFY_DEL_DEVICE ? "delete" : 
"unknown"));
+               if (action == BUS_NOTIFY_ADD_DEVICE)
+                       pci_no_msi();
+       }
+       return NOTIFY_OK;
 }
 
-struct notifier_block device_nb = {
+static struct notifier_block device_nb = {
        .notifier_call = xen_pci_notifier,
 };
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel