[openib-general] [PATCH 0/7] AMSO1100 RNIC Driver

Tom Tucker tom at opengridcomputing.com
Thu Mar 9 13:57:36 PST 2006


I'll take a whack at fixing this stuff, test, and repost along with the
cosmetic changes you proposed.

Thanks,
On Wed, 2006-03-08 at 14:06 -0800, Roland Dreier wrote:
> I ran sparse against the amso1100 driver, and came up with a bunch of
> cleanups.  This includes at least one fix for a minor memory leak:
> c2_cleanup_qp_table() was not calling c2_array_cleanup() for the QP
> table.
> 
> This leaves the following warnings, which are harder to untangle.  The
> fundamental problem is that the c2_mq stuff is broken: for example,
> req_vq has its pool in system memory, while rep_vq has its pool in
> iomem.  Fixing this will also require fixing things like qp_wr_post(),
> which right now does a memcpy to iomem.
> 
>     drivers/infiniband/hw/amso1100/c2_rnic.c:501:16: warning: incorrect type in argument 5 (different address spaces)
>     drivers/infiniband/hw/amso1100/c2_rnic.c:501:16:    expected unsigned char [usertype] *pool_start
>     drivers/infiniband/hw/amso1100/c2_rnic.c:501:16:    got void [noderef] *[assigned] mmio_regs<asn:2>
>     drivers/infiniband/hw/amso1100/c2_qp.c:365:11: warning: incorrect type in argument 5 (different address spaces)
>     drivers/infiniband/hw/amso1100/c2_qp.c:365:11:    expected unsigned char [usertype] *pool_start
>     drivers/infiniband/hw/amso1100/c2_qp.c:365:11:    got void [noderef] *[assigned] mmap<asn:2>
>     drivers/infiniband/hw/amso1100/c2_qp.c:384:11: warning: incorrect type in argument 5 (different address spaces)
>     drivers/infiniband/hw/amso1100/c2_qp.c:384:11:    expected unsigned char [usertype] *pool_start
>     drivers/infiniband/hw/amso1100/c2_qp.c:384:11:    got void [noderef] *[assigned] mmap<asn:2>
> 
>  - R.
> 
> Various sparse annotation fixes etc. for amso1100.
> 
> Signed-off-by: Roland Dreier <rolandd at cisco.com>
> 
> --- infiniband/hw/amso1100/c2_mq.c	(revision 5693)
> +++ infiniband/hw/amso1100/c2_mq.c	(working copy)
> @@ -69,7 +69,7 @@ void c2_mq_produce(struct c2_mq *q)
>  		BUMP(q, q->priv);
>  		q->hint_count++;
>  		/* Update peer's offset. */
> -		q->peer->shared = cpu_to_be16(q->priv);
> +		writew(cpu_to_be16(q->priv), &q->peer->shared);
>  	}
>  }
>  
> @@ -112,7 +112,7 @@ void c2_mq_free(struct c2_mq *q)
>  #endif
>  		BUMP(q, q->priv);
>  		/* Update peer's offset. */
> -		q->peer->shared = cpu_to_be16(q->priv);
> +		writew(cpu_to_be16(q->priv), &q->peer->shared);
>  	}
>  }
>  
> @@ -148,9 +148,8 @@ u32 c2_mq_count(struct c2_mq *q)
>  	return (u32) count;
>  }
>  
> -void
> -c2_mq_init(struct c2_mq *q, u32 index, u32 q_size,
> -	   u32 msg_size, u8 * pool_start, u16 * peer, u32 type)
> +void c2_mq_init(struct c2_mq *q, u32 index, u32 q_size, u32 msg_size,
> +		u8 *pool_start, u16 __iomem *peer, u32 type)
>  {
>  	assert(q->shared);
>  
> @@ -159,7 +158,7 @@ c2_mq_init(struct c2_mq *q, u32 index, u
>  	q->q_size = q_size;
>  	q->msg_size = msg_size;
>  	q->msg_pool = pool_start;
> -	q->peer = (struct c2_mq_shared *) peer;
> +	q->peer = (struct c2_mq_shared __iomem *) peer;
>  	q->magic = C2_MQ_MAGIC;
>  	q->type = type;
>  	q->priv = 0;
> --- infiniband/hw/amso1100/c2_qp.c	(revision 5693)
> +++ infiniband/hw/amso1100/c2_qp.c	(working copy)
> @@ -66,6 +66,7 @@ static const u8 c2_opcode[] = {
>  	[IB_WR_ATOMIC_FETCH_AND_ADD] = NO_SUPPORT,
>  };
>  
> +#if 0
>  void c2_qp_event(struct c2_dev *c2dev, u32 qpn, enum ib_event_type event_type)
>  {
>  	struct c2_qp *qp;
> @@ -91,6 +92,7 @@ void c2_qp_event(struct c2_dev *c2dev, u
>  	if (atomic_dec_and_test(&qp->refcount))
>  		wake_up(&qp->wait);
>  }
> +#endif
>  
>  static int to_c2_state(enum ib_qp_state ib_state)
>  {
> @@ -258,7 +260,7 @@ int c2_alloc_qp(struct c2_dev *c2dev,
>  	struct c2_cq *recv_cq = to_c2cq(qp_attrs->recv_cq);
>  	unsigned long peer_pa;
>  	u32 q_size, msg_size, mmap_size;
> -	void *mmap;
> +	void __iomem *mmap;
>  	int err;
>  
>  	qp->qpn = c2_alloc(&c2dev->qp_table.alloc);
> @@ -348,7 +350,7 @@ int c2_alloc_qp(struct c2_dev *c2dev,
>  	/* Initialize the SQ MQ */
>  	q_size = be32_to_cpu(reply->sq_depth);
>  	msg_size = be32_to_cpu(reply->sq_msg_size);
> -	peer_pa = (unsigned long) (c2dev->pa + be32_to_cpu(reply->sq_mq_start));
> +	peer_pa = c2dev->pa + be32_to_cpu(reply->sq_mq_start);
>  	mmap_size = PAGE_ALIGN(sizeof(struct c2_mq_shared) + msg_size * q_size);
>  	mmap = ioremap_nocache(peer_pa, mmap_size);
>  	if (!mmap) {
> @@ -367,7 +369,7 @@ int c2_alloc_qp(struct c2_dev *c2dev,
>  	/* Initialize the RQ mq */
>  	q_size = be32_to_cpu(reply->rq_depth);
>  	msg_size = be32_to_cpu(reply->rq_msg_size);
> -	peer_pa = (unsigned long) (c2dev->pa + be32_to_cpu(reply->rq_mq_start));
> +	peer_pa = c2dev->pa + be32_to_cpu(reply->rq_mq_start);
>  	mmap_size = PAGE_ALIGN(sizeof(struct c2_mq_shared) + msg_size * q_size);
>  	mmap = ioremap_nocache(peer_pa, mmap_size);
>  	if (!mmap) {
> @@ -836,4 +838,5 @@ int __devinit c2_init_qp_table(struct c2
>  void __devexit c2_cleanup_qp_table(struct c2_dev *c2dev)
>  {
>  	c2_alloc_cleanup(&c2dev->qp_table.alloc);
> +	c2_array_cleanup(&c2dev->qp_table.qp, c2dev->max_qp);
>  }
> --- infiniband/hw/amso1100/c2.c	(revision 5693)
> +++ infiniband/hw/amso1100/c2.c	(working copy)
> @@ -81,8 +81,6 @@ static int c2_change_mtu(struct net_devi
>  static void c2_reset(struct c2_port *c2_port);
>  static struct net_device_stats *c2_get_stats(struct net_device *netdev);
>  
> -extern void c2_rnic_interrupt(struct c2_dev *c2dev);
> -
>  static struct pci_device_id c2_pci_table[] = {
>  	{0x18b8, 0xb001, PCI_ANY_ID, PCI_ANY_ID},
>  	{0}
> @@ -121,7 +119,7 @@ static int c2_tx_ring_alloc(struct c2_ri
>  			    dma_addr_t base, void __iomem * mmio_txp_ring)
>  {
>  	struct c2_tx_desc *tx_desc;
> -	struct c2_txp_desc *txp_desc;
> +	struct c2_txp_desc __iomem *txp_desc;
>  	struct c2_element *elem;
>  	int i;
>  
> @@ -139,7 +137,7 @@ static int c2_tx_ring_alloc(struct c2_ri
>  		/* Set TXP_HTXD_UNINIT */
>  		writeq(cpu_to_be64(0x1122334455667788ULL),
>  		       (void __iomem *) txp_desc + C2_TXP_ADDR);
> -		writew(cpu_to_be16(0), (void *) txp_desc + C2_TXP_LEN);
> +		writew(cpu_to_be16(0), (void __iomem *) txp_desc + C2_TXP_LEN);
>  		writew(cpu_to_be16(TXP_HTXD_UNINIT),
>  		       (void __iomem *) txp_desc + C2_TXP_FLAGS);
>  
> @@ -170,7 +168,7 @@ static int c2_rx_ring_alloc(struct c2_ri
>  			    dma_addr_t base, void __iomem * mmio_rxp_ring)
>  {
>  	struct c2_rx_desc *rx_desc;
> -	struct c2_rxp_desc *rxp_desc;
> +	struct c2_rxp_desc __iomem *rxp_desc;
>  	struct c2_element *elem;
>  	int i;
>  
> @@ -187,13 +185,13 @@ static int c2_rx_ring_alloc(struct c2_ri
>  
>  		/* Set RXP_HRXD_UNINIT */
>  		writew(cpu_to_be16(RXP_HRXD_OK),
> -		       (void *) rxp_desc + C2_RXP_STATUS);
> -		writew(cpu_to_be16(0), (void *) rxp_desc + C2_RXP_COUNT);
> -		writew(cpu_to_be16(0), (void *) rxp_desc + C2_RXP_LEN);
> +		       (void __iomem *) rxp_desc + C2_RXP_STATUS);
> +		writew(cpu_to_be16(0), (void __iomem *) rxp_desc + C2_RXP_COUNT);
> +		writew(cpu_to_be16(0), (void __iomem *) rxp_desc + C2_RXP_LEN);
>  		writeq(cpu_to_be64(0x99aabbccddeeffULL),
> -		       (void *) rxp_desc + C2_RXP_ADDR);
> +		       (void __iomem *) rxp_desc + C2_RXP_ADDR);
>  		writew(cpu_to_be16(RXP_HRXD_UNINIT),
> -		       (void *) rxp_desc + C2_RXP_FLAGS);
> +		       (void __iomem *) rxp_desc + C2_RXP_FLAGS);
>  
>  		elem->skb = NULL;
>  		elem->ht_desc = rx_desc;
> @@ -1178,7 +1176,7 @@ static int __devinit c2_probe(struct pci
>  	}
>  
>  	/* Remap the PCI registers in adapter BAR4 to kernel VA space */
> -	c2dev->pa = (void *) (reg4_start + C2_PCI_REGS_OFFSET);
> +	c2dev->pa = reg4_start + C2_PCI_REGS_OFFSET;
>  	c2dev->kva = ioremap_nocache(reg4_start + C2_PCI_REGS_OFFSET, 
>  				     kva_map_size);
>  	if (c2dev->kva == 0UL) {
> --- infiniband/hw/amso1100/c2_mq.h	(revision 5693)
> +++ infiniband/hw/amso1100/c2_mq.h	(working copy)
> @@ -68,7 +68,7 @@ struct c2_mq {
>  	u8 *msg_pool;
>  	u16 hint_count;
>  	u16 priv;
> -	struct c2_mq_shared *peer;
> +	struct c2_mq_shared __iomem *peer;
>  	u16 *shared;
>  	u32 q_size;
>  	u32 msg_size;
> @@ -95,7 +95,7 @@ extern void c2_mq_produce(struct c2_mq *
>  extern void *c2_mq_consume(struct c2_mq *q);
>  extern void c2_mq_free(struct c2_mq *q);
>  extern u32 c2_mq_count(struct c2_mq *q);
> -extern void c2_mq_init(struct c2_mq *q, u32 index, u32 q_size,
> -		       u32 msg_size, u8 * pool_start, u16 * peer, u32 type);
> +extern void c2_mq_init(struct c2_mq *q, u32 index, u32 q_size, u32 msg_size,
> +		       u8 *pool_start, u16 __iomem *peer, u32 type);
>  
>  #endif				/* _C2_MQ_H_ */
> --- infiniband/hw/amso1100/c2.h	(revision 5693)
> +++ infiniband/hw/amso1100/c2.h	(working copy)
> @@ -263,7 +263,7 @@ struct c2_array {
>   */
>  struct sp_chunk {
>  	struct sp_chunk *next;
> -	u32 gfp_mask;
> +	gfp_t gfp_mask;
>  	u16 head;
>  	u16 shared_ptr[0];
>  };
> @@ -288,7 +288,7 @@ struct c2_qp_table {
>  struct c2_element {
>  	struct c2_element *next;
>  	void *ht_desc;		/* host     descriptor */
> -	void *hw_desc;		/* hardware descriptor */
> +	void __iomem *hw_desc;	/* hardware descriptor */
>  	struct sk_buff *skb;
>  	dma_addr_t mapaddr;
>  	u32 maplen;
> @@ -319,7 +319,7 @@ struct c2_dev {
>  	u32 vendor_id;
>  	u32 vendor_part_id;
>  	void __iomem *kva;	/* KVA device memory */
> -	void __iomem *pa;	/* PA device memory */
> +	unsigned long pa;	/* PA device memory */
>  	void **qptr_array;
>  
>  	kmem_cache_t *host_msg_cache;
> @@ -514,6 +514,7 @@ extern int c2_register_device(struct c2_
>  extern void c2_unregister_device(struct c2_dev *c2dev);
>  extern int c2_rnic_init(struct c2_dev *c2dev);
>  extern void c2_rnic_term(struct c2_dev *c2dev);
> +extern void c2_rnic_interrupt(struct c2_dev *c2dev);
>  int c2_del_addr(struct c2_dev *c2dev, u32 inaddr, u32 inmask);
>  int c2_add_addr(struct c2_dev *c2dev, u32 inaddr, u32 inmask);
>  
> @@ -569,10 +570,11 @@ extern u32 c2_alloc(struct c2_alloc *all
>  extern void c2_free(struct c2_alloc *alloc, u32 obj);
>  extern int c2_alloc_init(struct c2_alloc *alloc, u32 num, u32 reserved);
>  extern void c2_alloc_cleanup(struct c2_alloc *alloc);
> -extern int c2_init_mqsp_pool(unsigned int gfp_mask, struct sp_chunk **root);
> +extern int c2_init_mqsp_pool(gfp_t gfp_mask, struct sp_chunk **root);
>  extern void c2_free_mqsp_pool(struct sp_chunk *root);
>  extern u16 *c2_alloc_mqsp(struct sp_chunk *head);
>  extern void c2_free_mqsp(u16 * mqsp);
> +extern void c2_array_cleanup(struct c2_array *array, int nent);
>  extern int c2_array_init(struct c2_array *array, int nent);
>  extern void c2_array_clear(struct c2_array *array, int index);
>  extern int c2_array_set(struct c2_array *array, int index, void *value);
> --- infiniband/hw/amso1100/c2_alloc.c	(revision 5693)
> +++ infiniband/hw/amso1100/c2_alloc.c	(working copy)
> @@ -165,7 +165,7 @@ void c2_array_cleanup(struct c2_array *a
>  	kfree(array->page_list);
>  }
>  
> -static int c2_alloc_mqsp_chunk(unsigned int gfp_mask, struct sp_chunk **head)
> +static int c2_alloc_mqsp_chunk(gfp_t gfp_mask, struct sp_chunk **head)
>  {
>  	int i;
>  	struct sp_chunk *new_head;
> @@ -192,7 +192,7 @@ static int c2_alloc_mqsp_chunk(unsigned 
>  	return 0;
>  }
>  
> -int c2_init_mqsp_pool(unsigned int gfp_mask, struct sp_chunk **root)
> +int c2_init_mqsp_pool(gfp_t gfp_mask, struct sp_chunk **root)
>  {
>  	return c2_alloc_mqsp_chunk(gfp_mask, root);
>  }
> @@ -224,13 +224,13 @@ u16 *c2_alloc_mqsp(struct sp_chunk *head
>  				head->head = head->shared_ptr[mqsp];
>  				break;
>  			} else
> -				return 0;
> +				return NULL;
>  		} else
>  			head = head->next;
>  	}
>  	if (head)
>  		return &(head->shared_ptr[mqsp]);
> -	return 0;
> +	return NULL;
>  }
>  
>  void c2_free_mqsp(u16 * mqsp)
> --- infiniband/hw/amso1100/c2_provider.c	(revision 5693)
> +++ infiniband/hw/amso1100/c2_provider.c	(working copy)
> @@ -576,7 +576,7 @@ static int c2_disconnect(struct iw_cm_id
>  	/* Disassociate the QP from this cm_id */
>  	cm_id->provider_id = 0;
>  	qp = to_c2qp(ib_qp);
> -	qp->cm_id = 0;
> +	qp->cm_id = NULL;
>  
>  	memset(&attr, 0, sizeof(struct ib_qp_attr));
>  	if (abrupt)
> @@ -816,10 +816,10 @@ int c2_register_device(struct c2_dev *de
>  	dev->ibdev.reg_user_mr = c2_reg_user_mr;
>  	dev->ibdev.dereg_mr = c2_dereg_mr;
>  
> -	dev->ibdev.alloc_fmr = 0;
> -	dev->ibdev.unmap_fmr = 0;
> -	dev->ibdev.dealloc_fmr = 0;
> -	dev->ibdev.map_phys_fmr = 0;
> +	dev->ibdev.alloc_fmr = NULL;
> +	dev->ibdev.unmap_fmr = NULL;
> +	dev->ibdev.dealloc_fmr = NULL;
> +	dev->ibdev.map_phys_fmr = NULL;
>  
>  	dev->ibdev.attach_mcast = c2_multicast_attach;
>  	dev->ibdev.detach_mcast = c2_multicast_detach;
> --- infiniband/hw/amso1100/c2_cq.c	(revision 5693)
> +++ infiniband/hw/amso1100/c2_cq.c	(working copy)
> @@ -198,30 +198,27 @@ int c2_poll_cq(struct ib_cq *ibcq, int n
>  
>  int c2_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
>  {
> -	struct c2_mq_shared volatile *shared;
> +	struct c2_mq_shared __iomem *shared;
>  	struct c2_cq *cq;
>  
>  	cq = to_c2cq(ibcq);
>  	shared = cq->mq.peer;
>  
>  	if (notify == IB_CQ_NEXT_COMP)
> -		shared->notification_type = C2_CQ_NOTIFICATION_TYPE_NEXT;
> +		writeb(C2_CQ_NOTIFICATION_TYPE_NEXT, &shared->notification_type);
>  	else if (notify == IB_CQ_SOLICITED)
> -		shared->notification_type = C2_CQ_NOTIFICATION_TYPE_NEXT_SE;
> +		writeb(C2_CQ_NOTIFICATION_TYPE_NEXT_SE, &shared->notification_type);
>  	else
>  		return -EINVAL;
>  
> -	shared->armed = CQ_WAIT_FOR_DMA | CQ_ARMED;
> +	writeb(CQ_WAIT_FOR_DMA | CQ_ARMED, &shared->armed);
>  
>  	/*
>  	 * Now read back shared->armed to make the PCI
>  	 * write synchronous.  This is necessary for
>  	 * correct cq notification semantics.
>  	 */
> -	{
> -		volatile char c;
> -		c = shared->armed;
> -	}
> +	readb(&shared->armed);
>  
>  	return 0;
>  }
> @@ -250,7 +247,7 @@ static int c2_alloc_cq_buf(struct c2_mq 
>  		   q_size, 
>  		   msg_size, 
>  		   (u8 *) pool_start, 
> -		   0,		/* peer (currently unknown) */
> +		   NULL,	/* peer (currently unknown) */
>  		   C2_MQ_HOST_TARGET);
>  
>  	return 0;
> @@ -320,8 +317,7 @@ int c2_init_cq(struct c2_dev *c2dev, int
>  	cq->adapter_handle = reply->cq_handle;
>  	cq->mq.index = be32_to_cpu(reply->mq_index);
>  
> -	peer_pa =
> -	    (unsigned long) (c2dev->pa + be32_to_cpu(reply->adapter_shared));
> +	peer_pa = c2dev->pa + be32_to_cpu(reply->adapter_shared);
>  	cq->mq.peer = ioremap_nocache(peer_pa, PAGE_SIZE);
>  	if (!cq->mq.peer) {
>  		err = -ENOMEM;




More information about the general mailing list