Xen devel - Aug 2012 - [PATCH] Various cleanups in the code base found with sparse (v1) for v3.7.

I''ve been starting to run sparse to make sure we are not introducing
any potential issues. It does not report any in the Xen code-base but it
does reports a bit of the warnings so these take care of removing them.

Most of the warnings were of the type : <function or value> not declared.

And in some cases they just needed to be converted to ''static''
or
the header file included in the C code.

Sparse warns us off:
drivers/xen/swiotlb-xen.c:506:1: warning: symbol
''xen_swiotlb_map_sg'' was not declared. Should it be static?
drivers/xen/swiotlb-xen.c:534:1: warning: symbol
''xen_swiotlb_unmap_sg'' was not declared. Should it be static?

and it looks like we do not need this function at all.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/swiotlb-xen.c |   16 ----------------
 include/xen/swiotlb-xen.h |    9 ---------
 2 files changed, 0 insertions(+), 25 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 1942a3e..09e25a6 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -502,14 +502,6 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct
scatterlist *sgl,
 }
 EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg_attrs);
 
-int
-xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sgl, int nelems,
-		   enum dma_data_direction dir)
-{
-	return xen_swiotlb_map_sg_attrs(hwdev, sgl, nelems, dir, NULL);
-}
-EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg);
-
 /*
  * Unmap a set of streaming mode DMA translations.  Again, cpu read rules
  * concerning calls here are the same as for swiotlb_unmap_page() above.
@@ -530,14 +522,6 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct
scatterlist *sgl,
 }
 EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
 
-void
-xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sgl, int nelems,
-		     enum dma_data_direction dir)
-{
-	return xen_swiotlb_unmap_sg_attrs(hwdev, sgl, nelems, dir, NULL);
-}
-EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg);
-
 /*
  * Make physical memory consistent for a set of streaming mode DMA translations
  * after a transfer.
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index d38d984..d8cba6f 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -24,15 +24,6 @@ extern dma_addr_t xen_swiotlb_map_page(struct device *dev,
struct page *page,
 extern void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
 				   size_t size, enum dma_data_direction dir,
 				   struct dma_attrs *attrs);
-/*
-extern int
-xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sg, int nents,
-		   enum dma_data_direction dir);
-
-extern void
-xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents,
-		     enum dma_data_direction dir);
-*/
 extern int
 xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
 			 int nelems, enum dma_data_direction dir,
-- 
1.7.7.6

arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer
arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 031d8bc..1667602 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -92,6 +92,6 @@ int pci_xen_swiotlb_init_late(void)
 EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
 
 IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
-		  0,
+		  NULL,
 		  pci_xen_swiotlb_init,
-		  0);
+		  NULL);
-- 
1.7.7.6

There is no need for those functions/variables to be visible. Make them
static and also fix the compile warnings of this sort:

drivers/xen/<some file>.c: warning: symbol
''<blah>'' was not declared. Should it be static?

Some of them just require including the header file that
declares the functions.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/apic.c                     |    3 ++-
 arch/x86/xen/enlighten.c                |    2 ++
 arch/x86/xen/pci-swiotlb-xen.c          |    2 ++
 arch/x86/xen/platform-pci-unplug.c      |    1 +
 drivers/pci/xen-pcifront.c              |    2 +-
 drivers/xen/gntdev.c                    |    2 +-
 drivers/xen/grant-table.c               |   13 ++++++-------
 drivers/xen/swiotlb-xen.c               |    2 +-
 drivers/xen/tmem.c                      |    1 +
 drivers/xen/xenbus/xenbus_dev_backend.c |    2 +-
 drivers/xen/xenbus/xenbus_probe.c       |    4 ++--
 11 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
index ec57bd3..7005ced 100644
--- a/arch/x86/xen/apic.c
+++ b/arch/x86/xen/apic.c
@@ -6,8 +6,9 @@
 
 #include <xen/xen.h>
 #include <xen/interface/physdev.h>
+#include "xen-ops.h"
 
-unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
+static unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
 {
 	struct physdev_apic apic_op;
 	int ret;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index acf906c..2c36e6e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -81,6 +81,8 @@
 #include "smp.h"
 #include "multicalls.h"
 
+#include <xen/events.h>
+
 EXPORT_SYMBOL_GPL(hypercall_page);
 
 DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 1667602..db4db14 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -13,6 +13,8 @@
 #include <asm/dma.h>
 #endif
 #include <linux/export.h>
+
+#include <asm/xen/swiotlb-xen.h>
 int xen_swiotlb __read_mostly;
 
 static struct dma_map_ops xen_swiotlb_dma_ops = {
diff --git a/arch/x86/xen/platform-pci-unplug.c
b/arch/x86/xen/platform-pci-unplug.c
index ffcf261..0a78524 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 
 #include <xen/platform_pci.h>
+#include "xen-ops.h"
 
 #define XEN_PLATFORM_ERR_MAGIC -1
 #define XEN_PLATFORM_ERR_PROTOCOL -2
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index ca92801..ebd1ddb 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -237,7 +237,7 @@ static int pcifront_bus_write(struct pci_bus *bus, unsigned
int devfn,
 	return errno_to_pcibios_err(do_pci_op(pdev, &op));
 }
 
-struct pci_ops pcifront_bus_ops = {
+static struct pci_ops pcifront_bus_ops = {
 	.read = pcifront_bus_read,
 	.write = pcifront_bus_write,
 };
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 1ffd03b..163b7e9 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -445,7 +445,7 @@ static void mn_release(struct mmu_notifier *mn,
 	spin_unlock(&priv->lock);
 }
 
-struct mmu_notifier_ops gntdev_mmu_ops = {
+static struct mmu_notifier_ops gntdev_mmu_ops = {
 	.release                = mn_release,
 	.invalidate_page        = mn_invl_page,
 	.invalidate_range_start = mn_invl_range_start,
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 0bfc1ef..4061943 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -285,10 +285,9 @@ int gnttab_grant_foreign_access(domid_t domid, unsigned
long frame,
 }
 EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
 
-void gnttab_update_subpage_entry_v2(grant_ref_t ref, domid_t domid,
-				    unsigned long frame, int flags,
-				    unsigned page_off,
-				    unsigned length)
+static void gnttab_update_subpage_entry_v2(grant_ref_t ref, domid_t domid,
+					   unsigned long frame, int flags,
+					   unsigned page_off, unsigned length)
 {
 	gnttab_shared.v2[ref].sub_page.frame = frame;
 	gnttab_shared.v2[ref].sub_page.page_off = page_off;
@@ -345,9 +344,9 @@ bool gnttab_subpage_grants_available(void)
 }
 EXPORT_SYMBOL_GPL(gnttab_subpage_grants_available);
 
-void gnttab_update_trans_entry_v2(grant_ref_t ref, domid_t domid,
-				  int flags, domid_t trans_domid,
-				  grant_ref_t trans_gref)
+static void gnttab_update_trans_entry_v2(grant_ref_t ref, domid_t domid,
+					 int flags, domid_t trans_domid,
+					 grant_ref_t trans_gref)
 {
 	gnttab_shared.v2[ref].transitive.trans_domid = trans_domid;
 	gnttab_shared.v2[ref].transitive.gref = trans_gref;
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 09e25a6..307a908 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -52,7 +52,7 @@ static unsigned long xen_io_tlb_nslabs;
  * Quick lookup value of the bus address of the IOTLB.
  */
 
-u64 start_dma_addr;
+static u64 start_dma_addr;
 
 static dma_addr_t xen_phys_to_bus(phys_addr_t paddr)
 {
diff --git a/drivers/xen/tmem.c b/drivers/xen/tmem.c
index 89f264c..144564e 100644
--- a/drivers/xen/tmem.c
+++ b/drivers/xen/tmem.c
@@ -21,6 +21,7 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/page.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/tmem.h>
 
 #define TMEM_CONTROL               0
 #define TMEM_NEW_POOL              1
diff --git a/drivers/xen/xenbus/xenbus_dev_backend.c
b/drivers/xen/xenbus/xenbus_dev_backend.c
index be738c4..d730008 100644
--- a/drivers/xen/xenbus/xenbus_dev_backend.c
+++ b/drivers/xen/xenbus/xenbus_dev_backend.c
@@ -107,7 +107,7 @@ static int xenbus_backend_mmap(struct file *file, struct
vm_area_struct *vma)
 	return 0;
 }
 
-const struct file_operations xenbus_backend_fops = {
+static const struct file_operations xenbus_backend_fops = {
 	.open = xenbus_backend_open,
 	.mmap = xenbus_backend_mmap,
 	.unlocked_ioctl = xenbus_backend_ioctl,
diff --git a/drivers/xen/xenbus/xenbus_probe.c
b/drivers/xen/xenbus/xenbus_probe.c
index b793723..91d3d654 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -324,8 +324,8 @@ static int cmp_dev(struct device *dev, void *data)
 	return 0;
 }
 
-struct xenbus_device *xenbus_device_find(const char *nodename,
-					 struct bus_type *bus)
+static struct xenbus_device *xenbus_device_find(const char *nodename,
+						struct bus_type *bus)
 {
 	struct xb_find_info info = { .dev = NULL, .nodename = nodename };
 
-- 
1.7.7.6

linux/arch/x86/xen/mmu.c:1788:14: warning: comparison between pointer and
integer [enabled by default]

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/mmu.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 90d31a2..4911354 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1786,11 +1786,11 @@ void __init xen_setup_machphys_mapping(void)
 {
 	struct xen_machphys_mapping mapping;
 
-	if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
+	if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, (void *)&mapping) == 0)
{
 		machine_to_phys_mapping = (unsigned long *)mapping.v_start;
 		machine_to_phys_nr = mapping.max_mfn + 1;
 	} else {
-		machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
+		machine_to_phys_nr = (unsigned long)MACH2PHYS_NR_ENTRIES;
 	}
 #ifdef CONFIG_X86_32
 	WARN_ON((machine_to_phys_mapping + (machine_to_phys_nr - 1))
-- 
1.7.7.6

drivers/block/xen-blkback/xenbus.c:260:5: warning: symbol
''xenvbd_sysfs_addif'' was not declared. Should it be static?
drivers/block/xen-blkback/xenbus.c:284:6: warning: symbol
''xenvbd_sysfs_delif'' was not declared. Should it be static?

CC: JEns Axboe <axboe@kernel.dk>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/block/xen-blkback/xenbus.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c
b/drivers/block/xen-blkback/xenbus.c
index 4f66171..d0fed55 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -196,7 +196,7 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
 	}
 }
 
-void xen_blkif_free(struct xen_blkif *blkif)
+static void xen_blkif_free(struct xen_blkif *blkif)
 {
 	if (!atomic_dec_and_test(&blkif->refcnt))
 		BUG();
@@ -257,7 +257,7 @@ static struct attribute_group xen_vbdstat_group = {
 VBD_SHOW(physical_device, "%x:%x\n", be->major, be->minor);
 VBD_SHOW(mode, "%s\n", be->mode);
 
-int xenvbd_sysfs_addif(struct xenbus_device *dev)
+static int xenvbd_sysfs_addif(struct xenbus_device *dev)
 {
 	int error;
 
@@ -281,7 +281,7 @@ fail1:	device_remove_file(&dev->dev,
&dev_attr_physical_device);
 	return error;
 }
 
-void xenvbd_sysfs_delif(struct xenbus_device *dev)
+static void xenvbd_sysfs_delif(struct xenbus_device *dev)
 {
 	sysfs_remove_group(&dev->dev.kobj, &xen_vbdstat_group);
 	device_remove_file(&dev->dev, &dev_attr_mode);
-- 
1.7.7.6

arch/x86/kernel/pci-dma.c:275:1: warning: duplicate const
arch/x86/kernel/pci-swiotlb.c:68:1: warning: duplicate const
arch/x86/kernel/pci-swiotlb.c:86:1: warning: duplicate const
arch/x86/kernel/amd_gart_64.c:899:1: warning: duplicate const
arch/x86/kernel/pci-calgary_64.c:1600:1: warning: duplicate const
arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: duplicate const
drivers/iommu/amd_iommu_init.c:1831:1: warning: duplicate const
drivers/iommu/dmar.c:1350:1: warning: duplicate const

when using smatch.

CC: x86@kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/include/asm/iommu_table.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/iommu_table.h
b/arch/x86/include/asm/iommu_table.h
index f229b13..bbf8fb2 100644
--- a/arch/x86/include/asm/iommu_table.h
+++ b/arch/x86/include/asm/iommu_table.h
@@ -48,7 +48,7 @@ struct iommu_table_entry {
 
 
 #define __IOMMU_INIT(_detect, _depend, _early_init, _late_init, _finish)\
-	static const struct iommu_table_entry const			\
+	static const struct iommu_table_entry				\
 		__iommu_entry_##_detect __used				\
 	__attribute__ ((unused, __section__(".iommu_table"),		\
 			aligned((sizeof(void *)))))	\
-- 
1.7.7.6

On Thu, 2012-08-16 at 17:12 +0100, Konrad Rzeszutek Wilk
wrote:
> linux/arch/x86/xen/mmu.c:1788:14: warning: comparison between pointer and
integer [enabled by default]
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/xen/mmu.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 90d31a2..4911354 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -1786,11 +1786,11 @@ void __init xen_setup_machphys_mapping(void)
>  {
>  	struct xen_machphys_mapping mapping;
>  
> -	if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
> +	if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, (void *)&mapping)
== 0) {
This changes seems to be unnecessary and not related to the commit
message.
>  		machine_to_phys_mapping = (unsigned long *)mapping.v_start;
>  		machine_to_phys_nr = mapping.max_mfn + 1;
>  	} else {
> -		machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
> +		machine_to_phys_nr = (unsigned long)MACH2PHYS_NR_ENTRIES;
I must be missing something. Given:
        #define MACH2PHYS_VIRT_START  mk_unsigned_long(__MACH2PHYS_VIRT_START)
        #define MACH2PHYS_VIRT_END    mk_unsigned_long(__MACH2PHYS_VIRT_END)
        #define MACH2PHYS_NR_ENTRIES 
((MACH2PHYS_VIRT_END-MACH2PHYS_VIRT_START)>>__MACH2PHYS_SHIFT)
How is MACH2PHYS_NR_ENTRIES not already unsigned long? Or at the very
least how is it not an integer type of some sort, it certainly doesn''t
look like it can be a pointer (as suggested by the commit message) to
me.

>  	}
>  #ifdef CONFIG_X86_32
>  	WARN_ON((machine_to_phys_mapping + (machine_to_phys_nr - 1))