[ofa-general] Possible ipath bugs detected by compiler/sparse warnings

Roland Dreier rdreier at cisco.com
Fri Jan 11 11:40:53 PST 2008


I've been looking at warnings coming from running sparse on
drivers/infiniband, and I've spotted a few things in ipath that look
like real bugs, but I don't know enough about the HW to know how to
fix things properly.  It all looks like error path stuff that wouldn't
get exercised very often.  Here's what I've seen:

ipath_iba6110.c:840:

			pci_write_config_byte(pdev, link_off,
					      linkctrl & (0xf << 8));

    The low byte of linkctrl & (0xf << 8) is always 0 -- should this
    be pci_write_config_word() to match the way linkctrl is read a few
    lines earlier?

ipath_intr.c:835:

	__le64 val;
	//...
		val = ipath_read_kreg64(dd, (0x1000 / sizeof(u64)) + im);
		dd->ipath_pioavailregs_dma[i] = dd->ipath_pioavailshadow[i]
			= le64_to_cpu(val);

    ipath_read_kreg64() seems as if it will return a value in CPU
    endian, since it is just readq().  And there seems to be some
    confusion here -- ipath_pioavailregs_dma seems to be in
    little-endian and ipath_pioavailshadow is in host-endian, so you
    can't assign both with the same value, right?  Has this code ever
    triggered on a big-endian system?

ipath_intr.c:62:

	u32 __iomem *pbuf;
	//...
	*pbuf = dwcnt+1; /* no flush required, since already in freeze */

    pbuf is declared as __iomem, but then you write directly to it in
    CPU endian.  Should this be a writel() to handle byte-swapping and
    general IO space stuff properly?

Finally, there are a few warnings about symbols shadowing earlier
declarations.  Most of them look trivial to fix... any objection to
merging the following for 2.6.25?

diff --git a/drivers/infiniband/hw/ipath/ipath_eeprom.c b/drivers/infiniband/hw/ipath/ipath_eeprom.c
index a5b6299..e28a42f 100644
--- a/drivers/infiniband/hw/ipath/ipath_eeprom.c
+++ b/drivers/infiniband/hw/ipath/ipath_eeprom.c
@@ -574,7 +574,7 @@ void ipath_get_eeprom_info(struct ipath_devdata *dd)
 	struct ipath_devdata *dd0 = ipath_lookup(0);
 
 	if (t && dd0->ipath_nguid > 1 && t <= dd0->ipath_nguid) {
-		u8 *bguid, oguid;
+		u8 oguid;
 		dd->ipath_guid = dd0->ipath_guid;
 		bguid = (u8 *) & dd->ipath_guid;
 
@@ -674,7 +674,6 @@ void ipath_get_eeprom_info(struct ipath_devdata *dd)
 		 * elsewhere for backward-compatibility.
 		 */
 		char *snp = dd->ipath_serial;
-		int len;
 		memcpy(snp, ifp->if_sprefix, sizeof ifp->if_sprefix);
 		snp[sizeof ifp->if_sprefix] = '\0';
 		len = strlen(snp);
diff --git a/drivers/infiniband/hw/ipath/ipath_qp.c b/drivers/infiniband/hw/ipath/ipath_qp.c
index b405906..80dc623 100644
--- a/drivers/infiniband/hw/ipath/ipath_qp.c
+++ b/drivers/infiniband/hw/ipath/ipath_qp.c
@@ -855,8 +855,6 @@ struct ib_qp *ipath_create_qp(struct ib_pd *ibpd,
 	 * See ipath_mmap() for details.
 	 */
 	if (udata && udata->outlen >= sizeof(__u64)) {
-		int err;
-
 		if (!qp->r_rq.wq) {
 			__u64 offset = 0;
 



More information about the general mailing list