Skip to contentSkip to conversation

pciback: check function return values

PATCH In review Jan Beulich 3 participants 5 messages Mar 28, 2007 – Apr 4, 2007 Xen hypervisor interface

lore
Share & tools
lore

Jan Beulich

author
#0 lore raw

PATCH 1/1 pciback: check function return values

.. of functions declared with __must_check post-2.6.18.

(As a note - I'm getting the impression that when used as a module, pciback appears to have a number of memory leaks).

Signed-off-by Jan Beulich <[email protected]>
drivers/xen/pciback/conf_space_header.c +12 -2
Index: head-2007-03-19/drivers/xen/pciback/conf_space_header.c
===================================================================
@@ -20,11 +20,15 @@ struct pci_bar_info {
 
 static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
 {
+	int err;
+
 	if (!dev->is_enabled && is_enable_cmd(value)) {
 		if (unlikely(verbose_request))
 			printk(KERN_DEBUG "pciback: %s: enable\n",
 			       pci_name(dev));
-		pci_enable_device(dev);
+		err = pci_enable_device(dev);
+		if (err)
+			return err;
 	} else if (dev->is_enabled && !is_enable_cmd(value)) {
 		if (unlikely(verbose_request))
 			printk(KERN_DEBUG "pciback: %s: disable\n",

@@ -44,7 +48,13 @@ static int command_write(struct pci_dev 
 			printk(KERN_DEBUG
 			       "pciback: %s: enable memory-write-invalidate\n",
 			       pci_name(dev));
-		pci_set_mwi(dev);
+		err = pci_set_mwi(dev);
+		if (err) {
+			printk(KERN_WARNING
+			       "pciback: %s: cannot enable memory-write-invalidate (%d)\n",
+			       pci_name(dev), err);
+			value &= ~PCI_COMMAND_INVALIDATE;
+		}
 	}
 
 	return pci_write_config_word(dev, offset, value);
drivers/xen/pciback/pci_stub.c +34 -17

75 lines not on this page · raw message

#1 lore raw
Quoted 6 lines · Jan Beulich

.. of functions declared with __must_check post-2.6.18.

(As a note - I'm getting the impression that when used as a module, pciback appears to have a number of memory leaks).

Signed-off-by Jan Beulich <[email protected]>

Where is the mystery pciback_exit() function defined? You use it but don't provide an implementation.

-- Keir

author
#2 lore raw
Quoted 5 lines
Quoted 1 line
Quoted 1 line

Keir Fraser <[email protected]> 28.03.07 15:41 >>>

Quoted 2 lines · Jan Beulich

.. of functions declared with __must_check post-2.6.18.

Quoted 7 lines
Quoted 4 lines

(As a note - I'm getting the impression that when used as a module, pciback appears to have a number of memory leaks).

Signed-off-by Jan Beulich <[email protected]>

Where is the mystery pciback_exit() function defined? You use it but don't provide an implementation.

Huh, sorry, unrefreshed patch. Thanks for catching this. Here's to proper one.

drivers/xen/pciback/conf_space_header.c +12 -2
Index: head-2007-03-19/drivers/xen/pciback/conf_space_header.c
===================================================================
@@ -20,11 +20,15 @@ struct pci_bar_info {
 
 static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
 {
+	int err;
+
 	if (!dev->is_enabled && is_enable_cmd(value)) {
 		if (unlikely(verbose_request))
 			printk(KERN_DEBUG "pciback: %s: enable\n",
 			       pci_name(dev));
-		pci_enable_device(dev);
+		err = pci_enable_device(dev);
+		if (err)
+			return err;
 	} else if (dev->is_enabled && !is_enable_cmd(value)) {
 		if (unlikely(verbose_request))
 			printk(KERN_DEBUG "pciback: %s: disable\n",

@@ -44,7 +48,13 @@ static int command_write(struct pci_dev 
 			printk(KERN_DEBUG
 			       "pciback: %s: enable memory-write-invalidate\n",
 			       pci_name(dev));
-		pci_set_mwi(dev);
+		err = pci_set_mwi(dev);
+		if (err) {
+			printk(KERN_WARNING
+			       "pciback: %s: cannot enable memory-write-invalidate (%d)\n",
+			       pci_name(dev), err);
+			value &= ~PCI_COMMAND_INVALIDATE;
+		}
 	}
 
 	return pci_write_config_word(dev, offset, value);
drivers/xen/pciback/pci_stub.c +34 -17

75 lines not on this page · raw message

#3 lore raw
In reply to Jan Beulich
Quoted 4 lines · Jan Beulich

.. of functions declared with __must_check post-2.6.18.

(As a note - I'm getting the impression that when used as a module, pciback appears to have a number of memory leaks).

Jan, thanks for the added robustness by way of return value checking.

What gives you the impression there are memory leaks when pciback is loaded as a module? The more specific you can be the easier problems are to find and fix.

Cheers,
Chris

author
#4 lore raw
Quoted 4 lines
Quoted 1 line
Quoted 1 line

Chris Bookholt <[email protected]> 03.04.07 17:56 >>>

Quoted 2 lines · Jan Beulich

.. of functions declared with __must_check post-2.6.18.

Quoted 8 lines
Quoted 2 lines

(As a note - I'm getting the impression that when used as a module, pciback appears to have a number of memory leaks).

Jan, thanks for the added robustness by way of return value checking.

What gives you the impression there are memory leaks when pciback is loaded as a module? The more specific you can be the easier problems are to find and fix.

First thing pcistub_init() does is parse the hide option, calling pcistub_device_id_add(). If subsequent steps fail, or if the driver is unloaded, this isn't being cleaned up. There are others that look suspicious, but this is certainly easier to determine for someone more familiar with the code than I am.

Jan