IB/CORE: sync the resouce access in fmr_pool

orabug: 25677461

There were some problem in the fmr_pool code that either was missing lock
protection or was using wrong lock when allocating/freeing/looking up resource
in the FMR pool.

Covering all above issues, the code turns out that every where we need lock
protection we need both the pool_lock and used_pool_lock. So this patch also
removes the used_pool_lock and keeps the pool lock and make the later sync
all the accesses.

Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
Reviewed-by: Avinash Repaka <avinash.repaka@oracle.com>

diff --git a/drivers/infiniband/core/fmr_pool.c b/drivers/infiniband/core/fmr_pool.c
index ad8e1f0671d8312687c0f1907fc255a3eab0a764..d6d9c6478fae716023f12b1b2fa030764cd1bbca 100644 (file)
--- a/drivers/infiniband/core/fmr_pool.c
+++ b/drivers/infiniband/core/fmr_pool.c
@@ -84,8 +84,6 @@ enum {
 
 struct ib_fmr_pool {
        spinlock_t                pool_lock;
-       spinlock_t                used_pool_lock;
-
        int                       pool_size;
        int                       max_pages;
        int                       max_remaps;
@@ -186,9 +184,13 @@ static void ib_fmr_batch_release(struct ib_fmr_pool *pool, int unmap_usedonce)
                int count = 0;
                LIST_HEAD(temp_list);
 
-               spin_lock_irq(&pool->used_pool_lock);
+               /* The pach of allocating from cache has a bad race with this
+                * path removing fmr from used_list (this path do list splice
+                * against the same list). Add pool_lock lock here to make the
+                * race safe.
+                */
+               spin_lock_irq(&pool->pool_lock);
                list_splice_init(&pool->used_list, &temp_list);
-               spin_unlock_irq(&pool->used_pool_lock);
                list_for_each_entry(fmr, &temp_list, list) {
                        /* find first fmr that is not mapped yet */
                        if (fmr->remap_count ==  0 ||
@@ -196,9 +198,7 @@ static void ib_fmr_batch_release(struct ib_fmr_pool *pool, int unmap_usedonce)
                                /* split the list 2 two */
                                list_cut_position(&unmap_list, &temp_list,
                                                                 &fmr->list);
-                               spin_lock_irq(&pool->used_pool_lock);
                                list_splice(&temp_list, &pool->used_list);
-                               spin_unlock_irq(&pool->used_pool_lock);
                                already_split = 1;
                                break;
                        } else {
@@ -209,6 +209,11 @@ static void ib_fmr_batch_release(struct ib_fmr_pool *pool, int unmap_usedonce)
                        }
                }
 
+               /* fmrs to unmap are detached from cache, now it safe to drop
+                * pool_lock
+                */
+               spin_unlock_irq(&pool->pool_lock);
+
                if (!already_split) {
                        /* All are mapped once */
                        list_splice_tail(&temp_list, &unmap_list);
@@ -403,7 +408,6 @@ struct ib_fmr_pool *ib_create_fmr_pool(struct ib_pd             *pd,
        pool->dirty_watermark = params->dirty_watermark;
        pool->dirty_len       = 0;
        spin_lock_init(&pool->pool_lock);
-       spin_lock_init(&pool->used_pool_lock);
        atomic_set(&pool->req_ser,   0);
        atomic_set(&pool->flush_ser, 0);
        init_waitqueue_head(&pool->force_wait);
@@ -541,9 +545,10 @@ int ib_flush_fmr_pool(struct ib_fmr_pool *pool)
         * Put them on the dirty list now so that the cleanup
         * thread will reap them too.
         */
-       spin_lock_irq(&pool->used_pool_lock);
+       /* lock pool_lock when accessing dirty_list */
+       spin_lock_irq(&pool->pool_lock);
        list_splice_init(&pool->used_list, &pool->dirty_list);
-       spin_unlock_irq(&pool->used_pool_lock);
+       spin_unlock_irq(&pool->pool_lock);
 
        serial = atomic_inc_return(&pool->req_ser);
        wake_up_process(pool->thread);
@@ -590,9 +595,8 @@ struct ib_pool_fmr *ib_fmr_pool_map_phys(struct ib_fmr_pool *pool_handle,
        if (fmr) {
                /* found in cache */
                ++fmr->ref_count;
-               if (fmr->ref_count == 1) {
+               if (fmr->ref_count == 1)
                        list_del(&fmr->list);
-               }
 
                spin_unlock_irqrestore(&pool->pool_lock, flags);
 
@@ -600,31 +604,27 @@ struct ib_pool_fmr *ib_fmr_pool_map_phys(struct ib_fmr_pool *pool_handle,
        }
 
        if (list_empty(&pool->free_list)) {
-               spin_unlock_irqrestore(&pool->pool_lock, flags);
-               spin_lock_irqsave(&pool->used_pool_lock, flags);
                if (list_empty(&pool->used_list)) {
-                       spin_unlock_irqrestore(&pool->used_pool_lock, flags);
+                       spin_unlock_irqrestore(&pool->pool_lock, flags);
                        return ERR_PTR(-EAGAIN);
                }
                fmr = list_entry(pool->used_list.next, struct ib_pool_fmr,
                                 list);
                list_del(&fmr->list);
                hlist_del_init(&fmr->cache_node);
-               spin_unlock_irqrestore(&pool->used_pool_lock, flags);
        } else {
                fmr = list_entry(pool->free_list.next, struct ib_pool_fmr,
                                 list);
                list_del(&fmr->list);
-               hlist_del_init(&fmr->cache_node);
-               spin_unlock_irqrestore(&pool->pool_lock, flags);
        }
+       spin_unlock_irqrestore(&pool->pool_lock, flags);
 
        if (pool->relaxed && fmr->pd != rargs->pd) {
                result = ib_set_fmr_pd(fmr->fmr, rargs->pd);
                if (result) {
-                       spin_lock_irqsave(&pool->used_pool_lock, flags);
+                       spin_lock_irqsave(&pool->pool_lock, flags);
                        list_add(&fmr->list, &pool->used_list);
-                       spin_unlock_irqrestore(&pool->used_pool_lock, flags);
+                       spin_unlock_irqrestore(&pool->pool_lock, flags);
 
                        pr_warn(PFX "set_fmr_pd returns %d\n", result);
 
@@ -636,9 +636,9 @@ struct ib_pool_fmr *ib_fmr_pool_map_phys(struct ib_fmr_pool *pool_handle,
                                 io_virtual_address);
 
        if (result) {
-               spin_lock_irqsave(&pool->used_pool_lock, flags);
+               spin_lock_irqsave(&pool->pool_lock, flags);
                list_add(&fmr->list, &pool->used_list);
-               spin_unlock_irqrestore(&pool->used_pool_lock, flags);
+               spin_unlock_irqrestore(&pool->pool_lock, flags);
 
                pr_warn(PFX "fmr_map returns %d\n", result);
 
@@ -687,7 +687,7 @@ int ib_fmr_pool_unmap(struct ib_pool_fmr *fmr)
 
        pool = fmr->pool;
 
-       spin_lock_irqsave(&pool->used_pool_lock, flags);
+       spin_lock_irqsave(&pool->pool_lock, flags);
 
        --fmr->ref_count;
        if (!fmr->ref_count) {
@@ -708,8 +708,7 @@ int ib_fmr_pool_unmap(struct ib_pool_fmr *fmr)
                       fmr, fmr->ref_count);
 #endif
 
-       spin_unlock_irqrestore(&pool->used_pool_lock, flags);
-
+       spin_unlock_irqrestore(&pool->pool_lock, flags);
        return 0;
 }
 EXPORT_SYMBOL(ib_fmr_pool_unmap);