From: Wengang Wang Date: Fri, 10 Mar 2017 17:21:01 +0000 (-0800) Subject: IB/CORE: sync the resouce access in fmr_pool X-Git-Tag: v4.1.12-102.0.20170529_2200~39 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=c886bf2c4f1f81b76cb8e9d41992bff6ddbd0c47;p=users%2Fjedix%2Flinux-maple.git 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 Reviewed-by: Avinash Repaka --- diff --git a/drivers/infiniband/core/fmr_pool.c b/drivers/infiniband/core/fmr_pool.c index ad8e1f0671d83..d6d9c6478fae7 100644 --- 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);