On Wednesday, October 26, 2016 1:31:13 PM CEST Binoy Jayan wrote: > +static inline int mlx5_ib_post_send_wait(struct mlx5_ib_dev *dev, > + struct mlx5_umr_wr *umrwr) > +{ > + struct umr_common *umrc = &dev->umrc; > + struct ib_send_wr *bad; > + int err; > + struct mlx5_ib_umr_context umr_context; > + > + mlx5_ib_init_umr_context(&umr_context); > + umrwr->wr.wr_cqe = &umr_context.cqe; > + > + down(&umrc->sem); > + err = ib_post_send(umrc->qp, &umrwr->wr, &bad); > + if (err) { > + mlx5_ib_warn(dev, "UMR post send failed, err %d\n", err); > + } else { > + wait_for_completion(&umr_context.done); > + if (umr_context.status != IB_WC_SUCCESS) { > + mlx5_ib_warn(dev, "reg umr failed (%u)\n", > + umr_context.status); > + err = -EFAULT; > + } > + } > + up(&umrc->sem); > + return err; > +} > Looks nice! Now that this has become the only use of the semaphore (and the last one in infiniband I guess), I wonder if we can agree on a way to get rid of that one too. How about /* limit number of concurrent ib_post_send() on qp */ wait_event(&umrc->wq, atomic_add_unless(&umrc->users, 1, MAX_UMR_WR); ... atomic_dec(&umrc->users); wake_up(&umrc->wq); That would be a fairly simple conversion and document better what we actually do here: the down() looks like a simple mutex, which it isn't. In terms of efficiency, the wait_event() is actually better here for the common case that the semaphore is not contented as it only needs a single atomic operation and a branch instead of an external function call and a spinlock in down(). However, the wake_up() is slightly better than atomic_dec()+up() as it avoids the additional atomic. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html