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 <[EMAIL PROTECTED]>
>
> --- 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;
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
