drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible types i
Subsystems (2)
PATCH 13/14 drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible types i
Hi Mike,
There are new sparse warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git for-next head: 04c83d3ce5bbb85f81744b51104522f657fd4ecf commit: e12deda1747e1cc479ea219c5907fd92d0158566 [13/14] IB/qib: RCU locking for MR validation
All sparse warnings:
+ drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible types in comparison expression (different address spaces) drivers/infiniband/hw/qib/qib_keys.c:168:22: sparse: incompatible types in comparison expression (different address spaces) drivers/infiniband/hw/qib/qib_keys.c:183:14: sparse: incompatible types in comparison expression (different address spaces) drivers/infiniband/hw/qib/qib_keys.c:266:22: sparse: incompatible types in comparison expression (different address spaces) drivers/infiniband/hw/qib/qib_keys.c:282:14: sparse: incompatible types in comparison expression (different address spaces) drivers/infiniband/hw/qib/qib_verbs.c:2010:9: sparse: incorrect type in assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2010:9: expected struct qib_qp *qp0
drivers/infiniband/hw/qib/qib_verbs.c:2010:9: got void [noderef] <asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2011:9: sparse: incorrect type in assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2011:9: expected struct qib_qp *qp1
drivers/infiniband/hw/qib/qib_verbs.c:2011:9: got void [noderef] <asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2036:17: sparse: incorrect type in assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2036:17: expected struct qib_qp *<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2036:17: got void [noderef] <asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2069:9: sparse: incorrect type in assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2069:9: expected struct qib_mregion *dma_mr
drivers/infiniband/hw/qib/qib_verbs.c:2069:9: got void [noderef] <asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2071:17: sparse: incorrect type in assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2071:17: expected struct qib_mregion *<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2071:17: got void [noderef] <asn:4>*<noident>
vim +64 drivers/infiniband/hw/qib/qib_keys.c
61 if (dma_region) {
62 struct qib_mregion *tmr;
63 Quoted 1 line
64 tmr =
rcu_dereference(dev->dma_mr);
65 if (!tmr) {
66 qib_get_mr(mr);
67 rcu_assign_pointer(dev->dma_mr, mr);---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu <[email protected]> Intel Corporation
From e12deda1747e1cc479ea219c5907fd92d0158566 Mon Sep 17 00:00:00 2001
From: Mike Marciniszyn <[email protected]>
Date: Wed, 27 Jun 2012 18:33:19 -0400
Subject: [PATCH] IB/qib: RCU locking for MR validation
Profiling indicates that MR validation locking is expensive. The MR table is largely read-only and is a suitable candidate for RCU locking.
The patch uses RCU locking during validation to eliminate one lock/unlock during that validation.
--- drivers/infiniband/hw/qib/qib_keys.c | 98 +++++++++++++++++---------------- drivers/infiniband/hw/qib/qib_mr.c | 7 +++ drivers/infiniband/hw/qib/qib_verbs.c | 4 +- drivers/infiniband/hw/qib/qib_verbs.h | 7 ++- 4 files changed, 66 insertions(+), 50 deletions(-)
drivers/infiniband/hw/qib/qib_keys.c +51 -47
@@ -34,42 +34,43 @@#include "qib.h" /** * qib_alloc_lkey - allocate an lkey * @mr: memory region that this lkey protects * @dma_region: 0->normal key, 1->restricted DMA key * * Returns 0 if successful, otherwise returns -errno. * -* Increments mr reference count and sets published -* as required. +* Increments mr reference count as required. * * Sets the lkey field mr for non-dma regions. * */ int qib_alloc_lkey (struct qib_mregion *mr,int dma_region) {unsigned long flags;u32 r;u32 n;int ret =0 ;struct qib_ibdev *dev =to_idev (mr->pd->device);struct qib_lkey_table *rkt = &dev->lk_table;spin_lock_irqsave (&rkt->lock, flags);/* special case for dma_mr lkey == 0 */ if (dma_region) { -/* should the dma_mr be relative to the pd? */ -if (!dev->dma_mr) { +struct qib_mregion *tmr; + + tmr =rcu_dereference (dev->dma_mr); +if (!tmr) {qib_get_mr (mr); - dev->dma_mr = mr; +rcu_assign_pointer (dev->dma_mr, mr); mr->lkey_published =1 ; } goto success; }/* Find the next available LKEY */ r = rkt->next; n = r;for (;;) {
@@ -87,19 +88,19 @@ int qib_alloc_lkey(struct qib_mregion *mr, int dma_region) rkt->gen++; mr->lkey = (r << (32 - ib_qib_lkey_table_size)) | ((((1 << (24 - ib_qib_lkey_table_size)) -1 ) & rkt->gen) <<8 );if (mr->lkey ==0 ) { mr->lkey |=1 <<8 ; rkt->gen++; }qib_get_mr (mr); - rkt->table[r] = mr; +rcu_assign_pointer (rkt->table[r], mr); mr->lkey_published =1 ; success:spin_unlock_irqrestore (&rkt->lock, flags); out:return ret; bail:spin_unlock_irqrestore (&rkt->lock, flags); ret = -ENOMEM ; goto out;
@@ -114,91 +115,89 @@ void qib_free_lkey(struct qib_mregion *mr)unsigned long flags;u32 lkey = mr->lkey;u32 r;struct qib_ibdev *dev =to_idev (mr->pd->device);struct qib_lkey_table *rkt = &dev->lk_table;spin_lock_irqsave (&rkt->lock, flags);if (!mr->lkey_published) goto out; - mr->lkey_published =0 ; - - -spin_lock_irqsave (&dev->lk_table.lock, flags); -if (lkey ==0 ) { -if (dev->dma_mr && dev->dma_mr == mr) { -qib_put_mr (dev->dma_mr); - dev->dma_mr =NULL ; - } - }else { +if (lkey ==0 ) +rcu_assign_pointer (dev->dma_mr,NULL ); +else { r = lkey >> (32 - ib_qib_lkey_table_size); -qib_put_mr (dev->dma_mr); - rkt->table[r] =NULL ; +rcu_assign_pointer (rkt->table[r],NULL ); } +qib_put_mr (mr); + mr->lkey_published =0 ; out: -spin_unlock_irqrestore (&dev->lk_table.lock, flags); +spin_unlock_irqrestore (&rkt->lock, flags); }/** * qib_lkey_ok - check IB SGE for validity and initialize * @rkt: table containing lkey to check SGE against +* @pd: protection domain * @isge: outgoing internal SGE * @sge: SGE to check * @acc: access flags * * Return 1 if valid and successful, otherwise returns 0. * +* increments the reference count upon success +* * Check the IB SGE for validity and initialize our internal version * of it. */ int qib_lkey_ok (struct qib_lkey_table *rkt,struct qib_pd *pd,struct qib_sge *isge,struct ib_sge *sge,int acc) {struct qib_mregion *mr;unsigned n, m;size_t off; -unsigned long flags;/* * We use LKEY == zero for kernel virtual addresses * (see qib_get_dma_mr and qib_dma.c). */ -spin_lock_irqsave (&rkt->lock, flags); +rcu_read_lock ();if (sge->lkey ==0 ) {struct qib_ibdev *dev =to_idev (pd->ibpd.device);if (pd->user) goto bail; -if (!dev->dma_mr) + mr =rcu_dereference (dev->dma_mr); +if (!mr) + goto bail; +if (unlikely (!atomic_inc_not_zero (&mr->refcount))) goto bail; -qib_get_mr (dev->dma_mr); -spin_unlock_irqrestore (&rkt->lock, flags); +rcu_read_unlock (); - isge->mr = dev->dma_mr; + isge->mr = mr; isge->vaddr = (void *) sge->addr; isge->length = sge->length; isge->sge_length = sge->length; isge->m =0 ; isge->n =0 ; goto ok; } - mr = rkt->table[(sge->lkey >> (32 - ib_qib_lkey_table_size))]; -if (unlikely (mr ==NULL || mr->lkey != sge->lkey || - mr->pd != &pd->ibpd)) + mr =rcu_dereference ( + rkt->table[(sge->lkey >> (32 - ib_qib_lkey_table_size))]); +if (unlikely (!mr || mr->lkey != sge->lkey || mr->pd != &pd->ibpd)) goto bail; off = sge->addr - mr->user_base; -if (unlikely (sge->addr < mr->user_base || - off + sge->length > mr->length || - (mr->access_flags & acc) != acc)) +if (unlikely (sge->addr < mr->iova || off + sge->length > mr->length || + (mr->access_flags & acc) ==0 )) goto bail; -qib_get_mr (mr); -spin_unlock_irqrestore (&rkt->lock, flags); +if (unlikely (!atomic_inc_not_zero (&mr->refcount))) + goto bail; +rcu_read_unlock (); off += mr->offset;if (mr->page_shift) {/* page sizes are uniform power of 2 so no loop is necessary entries_spanned_by_off is the number of times the loop below would have executed. */ size_t entries_spanned_by_off;
@@ -222,77 +221,82 @@ int qib_lkey_ok(struct qib_lkey_table *rkt, struct qib_pd *pd, isge->mr = mr; isge->vaddr = mr->map[m]->segs[n].vaddr + off; isge->length = mr->map[m]->segs[n].length - off; isge->sge_length = sge->length; isge->m = m; isge->n = n; ok:return 1 ; bail: -spin_unlock_irqrestore (&rkt->lock, flags); +rcu_read_unlock ();return 0 ; }/** * qib_rkey_ok - check the IB virtual address, length, and RKEY -* @dev: infiniband device -* @ss: SGE state +* @qp: qp for validation +* @sge: SGE state * @len: length of data * @vaddr: virtual address to place data * @rkey: rkey to check * @acc: access flags * * Return 1 if successful, otherwise 0. +* +* increments the reference count upon success */ int qib_rkey_ok (struct qib_qp *qp,struct qib_sge *sge,u32 len,u64 vaddr,u32 rkey,int acc) {struct qib_lkey_table *rkt = &to_idev (qp->ibqp.device)->lk_table;struct qib_mregion *mr;unsigned n, m;size_t off; -unsigned long flags;/* * We use RKEY == zero for kernel virtual addresses * (see qib_get_dma_mr and qib_dma.c). */ -spin_lock_irqsave (&rkt->lock, flags); +rcu_read_lock ();if (rkey ==0 ) {struct qib_pd *pd =to_ipd (qp->ibqp.pd);struct qib_ibdev *dev =to_idev (pd->ibpd.device);if (pd->user) goto bail; -if (!dev->dma_mr) + mr =rcu_dereference (dev->dma_mr); +if (!mr) goto bail; -qib_get_mr (dev->dma_mr); -spin_unlock_irqrestore (&rkt->lock, flags); +if (unlikely (!atomic_inc_not_zero (&mr->refcount))) + goto bail; +rcu_read_unlock (); - sge->mr = dev->dma_mr; + sge->mr = mr; sge->vaddr = (void *) vaddr; sge->length = len; sge->sge_length = len; sge->m =0 ; sge->n =0 ; goto ok; } - mr = rkt->table[(rkey >> (32 - ib_qib_lkey_table_size))]; -if (unlikely (mr ==NULL || mr->lkey != rkey || qp->ibqp.pd != mr->pd)) + mr =rcu_dereference ( + rkt->table[(rkey >> (32 - ib_qib_lkey_table_size))]); +if (unlikely (!mr || mr->lkey != rkey || qp->ibqp.pd != mr->pd)) goto bail; off = vaddr - mr->iova;if (unlikely (vaddr < mr->iova || off + len > mr->length || (mr->access_flags & acc) ==0 )) goto bail; -qib_get_mr (mr); -spin_unlock_irqrestore (&rkt->lock, flags); +if (unlikely (!atomic_inc_not_zero (&mr->refcount))) + goto bail; +rcu_read_unlock (); off += mr->offset;if (mr->page_shift) {/* page sizes are uniform power of 2 so no loop is necessary entries_spanned_by_off is the number of times the loop below would have executed. */ size_t entries_spanned_by_off;
@@ -316,19 +320,19 @@ int qib_rkey_ok(struct qib_qp *qp, struct qib_sge *sge, sge->mr = mr; sge->vaddr = mr->map[m]->segs[n].vaddr + off; sge->length = mr->map[m]->segs[n].length - off; sge->sge_length = len; sge->m = m; sge->n = n; ok:return 1 ; bail: -spin_unlock_irqrestore (&rkt->lock, flags); +rcu_read_unlock ();return 0 ; }/* * Initialize the memory region specified by the work reqeust. */ int qib_fast_reg_mr (struct qib_qp *qp,struct ib_send_wr *wr) {struct qib_lkey_table *rkt = &to_idev (qp->ibqp.device)->lk_table;
drivers/infiniband/hw/qib/qib_mr.c +7 -0
@@ -521,9 +521,16 @@ int qib_dealloc_fmr(struct ib_fmr *ibfmr)qib_get_mr (&fmr->mr); ret = -EBUSY ; goto out; }deinit_qib_mregion (&fmr->mr);kfree (fmr); out:return ret; } + +void mr_rcu_callback (struct rcu_head *list) +{ + struct qib_mregion *mr = container_of(list, struct qib_mregion, list); + +complete (&mr->comp); +}
drivers/infiniband/hw/qib/qib_verbs.c +3 -1
@@ -2060,19 +2060,21 @@ int qib_register_ib_device(struct qib_devdata *dd)spin_lock_init (&dev->lk_table.lock); dev->lk_table.max =1 << ib_qib_lkey_table_size; lk_tab_size = dev->lk_table.max *sizeof (*dev->lk_table.table); dev->lk_table.table = (struct qib_mregion **)__get_free_pages (GFP_KERNEL ,get_order (lk_tab_size));if (dev->lk_table.table ==NULL ) { ret = -ENOMEM ; goto err_lk; } -memset (dev->lk_table.table,0 , lk_tab_size); +RCU_INIT_POINTER (dev->dma_mr,NULL ); +for (i =0 ; i < dev->lk_table.max; i++) +RCU_INIT_POINTER (dev->lk_table.table[i],NULL );INIT_LIST_HEAD (&dev->pending_mmaps);spin_lock_init (&dev->pending_lock); dev->mmap_offset =PAGE_SIZE ;spin_lock_init (&dev->mmap_offset_lock);INIT_LIST_HEAD (&dev->piowait);INIT_LIST_HEAD (&dev->dmawait);INIT_LIST_HEAD (&dev->txwait);INIT_LIST_HEAD (&dev->memwait);INIT_LIST_HEAD (&dev->txreq_free);
drivers/infiniband/hw/qib/qib_verbs.h +5 -2
@@ -297,20 +297,21 @@ struct qib_mregion {u64 user_base;/* User's address for this region */ u64 iova;/* IB start address of this region */ size_t length;u32 lkey;u32 offset;/* offset (bytes) to start of region */ int access_flags;u32 max_segs;/* number of qib_segs in all the arrays */ u32 mapsz;/* size of the map array */ u8 page_shift;/* 0 - non unform/non powerof2 sizes */ -u8 lkey_published;/* in global table */ +u8 lkey_published;/* in global table */ struct completion comp;/* complete when refcount goes to zero */ +struct rcu_head list;atomic_t refcount;struct qib_segarray *map[0 ];/* the segments */ };/* * These keep track of the copy progress within a memory region. * Used by the verbs layer. */ struct qib_sge {
@@ -1016,22 +1017,24 @@ int qib_map_phys_fmr(struct ib_fmr *ibfmr, u64 *page_list,int qib_unmap_fmr (struct list_head *fmr_list);int qib_dealloc_fmr (struct ib_fmr *ibfmr);static inline void qib_get_mr (struct qib_mregion *mr) {atomic_inc (&mr->refcount); } +void mr_rcu_callback (struct rcu_head *list); +static inline void qib_put_mr (struct qib_mregion *mr) {if (unlikely (atomic_dec_and_test (&mr->refcount))) -complete (&mr->comp); +call_rcu (&mr->list, mr_rcu_callback); }static inline void qib_put_ss (struct qib_sge_state *ss) {while (ss->num_sge) {qib_put_mr (ss->sge.mr);if (--ss->num_sge) ss->sge = *ss->sg_list++; }
Signature · 1 line
1.7.10
RE: [infiniband:for-next 13/14] drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible typ
I'm not seeing this on either of my build test systems.
Can you provide your .config file and gcc version?
Mike
Quoted 74 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Friday, July 06, 2012 4:02 AM
To: Marciniszyn, Mike
Cc: [email protected]
Subject: [infiniband:for-next 13/14]drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible types in comparison expression (different address spaces)Hi Mike,
There are new sparse warnings show up in
tree:
git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.gitfor-
nexthead: 04c83d3ce5bbb85f81744b51104522f657fd4ecf commit: e12deda1747e1cc479ea219c5907fd92d0158566 [13/14] IB/qib: RCUlocking for MR validation
All sparse warnings:
+
drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible types in comparison expression (different address spaces)drivers/infiniband/hw/qib/qib_keys.c:168:22: sparse: incompatible types in
comparison expression (different address spaces)
drivers/infiniband/hw/qib/qib_keys.c:183:14: sparse: incompatible types in
comparison expression (different address spaces)
drivers/infiniband/hw/qib/qib_keys.c:266:22: sparse: incompatible types in
comparison expression (different address spaces)
drivers/infiniband/hw/qib/qib_keys.c:282:14: sparse: incompatible types in
comparison expression (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2010:9: sparse: incorrect type in
assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2010:9: expected structqib_qp
*qp0
drivers/infiniband/hw/qib/qib_verbs.c:2010:9: got void [noderef]
<asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2011:9: sparse: incorrect type in
assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2011:9: expected structqib_qp
*qp1
drivers/infiniband/hw/qib/qib_verbs.c:2011:9: got void [noderef]
<asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2036:17: sparse: incorrect type in
assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2036:17: expected structqib_qp
*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2036:17: got void [noderef]
<asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2069:9: sparse: incorrect type in
assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2069:9: expected structqib_mregion*dma_mr
drivers/infiniband/hw/qib/qib_verbs.c:2069:9: got void [noderef]
<asn:4>*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2071:17: sparse: incorrect type in
assignment (different address spaces)
drivers/infiniband/hw/qib/qib_verbs.c:2071:17: expected structqib_mregion*<noident>
drivers/infiniband/hw/qib/qib_verbs.c:2071:17: got void [noderef]
<asn:4>*<noident>vim +64
drivers/infiniband/hw/qib/qib_keys.c61 if (dma_region) { 62 struct qib_mregion *tmr; 63Quoted 1 line
64 tmr =
rcu_dereference(dev->dma_mr);65 if (!tmr) { 66 qib_get_mr(mr); 67 rcu_assign_pointer(dev->dma_mr, mr);---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu <[email protected]> Intel Corporation
Re: [infiniband:for-next 13/14] drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible typ
In reply to [email protected]Quoted 4 lines · Mike
I'm not seeing this on either of my build test systems.
Can you provide your .config file and gcc version?
This isn't a GCC warning, it's a sparse warning. It's saying that your Sparse rcu anotations aren't correct, probably.
I've added Fengguang's other email address in case wfg isn't fixed yet.
regards,
dan carpenter
Re: [infiniband:for-next 13/14] drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible typ
In reply to [email protected]Quoted 8 lines · Dan Carpenter
Quoted 4 lines · Mike
I'm not seeing this on either of my build test systems.
Can you provide your .config file and gcc version?
This isn't a GCC warning, it's a sparse warning. It's saying that your Sparse rcu anotations aren't correct, probably.
Yeah these warnings can be reproduced with
1) CONFIG_SPARSE_RCU_POINTER=y
2) make C=1 drivers/infiniband/hw/qib/qib_keys.o
Paul has a nice article on the details
The RCU API, 2010 Edition
https://lwn.net/Articles/418853/
For the entitled warning, the fix should be
drivers/infiniband/hw/qib/qib_keys.c +1 -1
@@ -349,7 +349,7 @@ int qib_fast_reg_mr(struct qib_qp *qp, struct ib_send_wr *wr)if (pd->user || rkey =0 ) goto bail; - mr = rkt->table[(rkey >> (32 - ib_qib_lkey_table_size))]; + mr =rcu_dereference (rkt->table[(rkey >> (32 - ib_qib_lkey_table_size))]);if (unlikely (mr =NULL || qp->ibqp.pd != mr->pd)) goto bail;
drivers/infiniband/hw/qib/qib_verbs.h +2 -2
@@ -650,7 +650,7 @@ struct qib_lkey_table {u32 next;/* next unused index (speeds search) */ u32 gen;/* generation count */ u32 max;/* size of the table */ -struct qib_mregion **table; +struct qib_mregion __rcu **table; };struct qib_opcode_stats {
@@ -732,7 +732,7 @@ struct qib_ibdev {struct list_head pending_mmaps;spinlock_t mmap_offset_lock;/* protect mmap_offset */ u32 mmap_offset; -struct qib_mregion *dma_mr; +struct qib_mregion __rcu *dma_mr;/* QP numbers are shared by all IB ports */ struct qib_qpn_table qpn_table;
Quoted 2 lines
I've added Fengguang's other email address in case wfg isn't fixed yet.
Thank you! The wfg address seems to work fine except for the previous abnormal case :)
Thanks,
Fengguang
Re: [infiniband:for-next 13/14] drivers/infiniband/hw/qib/qib_keys.c:64:23: sparse: incompatible typ
In reply to [email protected]Quoted 6 lines · Fengguang Wu
Quoted 2 lines
I've added Fengguang's other email address in case wfg isn't fixed yet.
Thank you! The wfg address seems to work fine except for the previous abnormal case :)
Nope. It's still not working.
regards,
dan carpenter