> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of De Lara Guarch, > Pablo > Sent: Wednesday, July 5, 2017 3:51 PM > To: mstolarchuk <mike.stolarc...@bigswitch.com>; Richardson, Bruce > <bruce.richard...@intel.com> > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] __rte_hash_add_key_with_hash not > releasing multiwriter_lock in failure paths > > > > > -----Original Message----- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of mstolarchuk > > Sent: Friday, May 26, 2017 1:31 PM > > To: bruce.richard...@intel.co > > Cc: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH] __rte_hash_add_key_with_hash not > releasing > > multiwriter_lock in failure paths > > > > Signed-off-by: mstolarchuk <mike.stolarc...@bigswitch.com> > > --- > > lib/librte_hash/rte_cuckoo_hash.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/lib/librte_hash/rte_cuckoo_hash.c > > b/lib/librte_hash/rte_cuckoo_hash.c > > index 645c0cf..37a8110 100644 > > --- a/lib/librte_hash/rte_cuckoo_hash.c > > +++ b/lib/librte_hash/rte_cuckoo_hash.c > > @@ -538,8 +538,10 @@ struct rte_hash * > > n_slots = rte_ring_mc_dequeue_burst(h- > > >free_slots, > > cached_free_slots->objs, > > LCORE_CACHE_SIZE, NULL); > > - if (n_slots == 0) > > - return -ENOSPC; > > + if (n_slots == 0) { > > + ret = -ENOSPC; > > + goto failure; > > + } > > > > cached_free_slots->len += n_slots; > > } > > @@ -548,8 +550,10 @@ struct rte_hash * > > cached_free_slots->len--; > > slot_id = cached_free_slots->objs[cached_free_slots->len]; > > } else { > > - if (rte_ring_sc_dequeue(h->free_slots, &slot_id) != 0) > > - return -ENOSPC; > > + if (rte_ring_sc_dequeue(h->free_slots, &slot_id) != 0) { > > + ret = -ENOSPC; > > + goto failure; > > + } > > } > > > > new_k = RTE_PTR_ADD(keys, (uintptr_t)slot_id * h- > > >key_entry_size); @@ -659,6 +663,7 @@ struct rte_hash * > > /* Error in addition, store new slot back in the ring and return > > error */ > > enqueue_slot_back(h, cached_free_slots, (void *)((uintptr_t) > > new_idx)); > > > > +failure: > > if (h->add_key == ADD_KEY_MULTIWRITER) > > rte_spinlock_unlock(h->multiwriter_lock); > > return ret; > > -- > > 1.9.1 > > Hi Mike, > > Thanks for the patch, it looks correct to me. > The code is fine, but the commit message needs some rework. > Title should start with: "hash: ...", since this is aiming the hash library, > So > the title could be: "hash: fix lock release on add" > > Then, some explanation on what the fix is doing would be good. > > Lastly, since this is a fix, it requires a fixes line and CC to the stable > branch: > > Fixes: be856325cba3 ("hash: add scalable multi-writer insertion with Intel > TSX") > CC: sta...@dpdk.org
FYI, I just submitted a v2 with the commit fixed for you, as you might not be available these days and the RC1 will be created soon. Pablo > > Thanks, > Pablo
