On 22/07/2019 17:25, Jason Gunthorpe wrote:
On Mon, Jul 22, 2019 at 05:14:20PM +0200, Maksym Planeta wrote:Need to ensure that kref_put does not run concurrently with the loop inside rxe_pool_get_key. Signed-off-by: Maksym Planeta <mplaneta@xxxxxxxxxxxxxxxxxxxx> drivers/infiniband/sw/rxe/rxe_pool.c | 18 ++++++++++++++++++ drivers/infiniband/sw/rxe/rxe_pool.h | 4 +--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c index efa9bab01e02..30a887cf9200 100644 +++ b/drivers/infiniband/sw/rxe/rxe_pool.c @@ -536,3 +536,21 @@ void *rxe_pool_get_key(struct rxe_pool *pool, void *key) read_unlock_irqrestore(&pool->pool_lock, flags); return node ? elem : NULL; } + +static void rxe_dummy_release(struct kref *kref) +{ +} + +void rxe_drop_ref(struct rxe_pool_entry *pelem) +{ + int res; + struct rxe_pool *pool = pelem->pool; + unsigned long flags; + + write_lock_irqsave(&pool->pool_lock, flags); + res = kref_put(&pelem->ref_cnt, rxe_dummy_release); + write_unlock_irqrestore(&pool->pool_lock, flags);This doesn't make sense.. If something is making the kref go to 0 while the node is still in the RB tree then that is a bug. You should never need to add locking around a kref_put.
From https://www.kernel.org/doc/Documentation/kref.txt | The last rule (rule 3) is the nastiest one to handle. Say, for | instance, you have a list of items that are each kref-ed, and you wish | to get the first one. You can't just pull the first item off the list | and kref_get() it. That violates rule 3 because you are not already | holding a valid pointer. You must add a mutex (or some other lock).
Jason
-- Regards, Maksym Planeta
