IB/mlx4: Allocation of CQ resize structure doesn't need to be atomic

Commit Message

Roland Dreier July 29, 2016, 4:58 a.m. UTC

From: Roland Dreier <[e-mail]>

We allocate a small tracking structure as part of mlx4_ib_resize_cq().
However, we don't need to use GFP_ATOMIC -- immediately after the
allocation, we call mlx4_cq_resize(), which allocates a command
mailbox with GFP_KERNEL and then sleeps on a firmware command, so we
better not be in an atomic context.

This actually has a real impact, because when this GFP_ATOMIC
allocation fails (and GFP_ATOMIC does fail in practice) then a
userspace consumer resizing a CQ will get a spurious failure that we
can easily avoid.

Signed-off-by: Roland Dreier <[e-mail]>
---
 drivers/infiniband/hw/mlx4/cq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Leon Romanovsky July 31, 2016, 6:42 a.m. UTC | #1

On Thu, Jul 28, 2016 at 09:58:43PM -0700, Roland Dreier wrote:
> From: Roland Dreier <[e-mail]>
> 
> We allocate a small tracking structure as part of mlx4_ib_resize_cq().
> However, we don't need to use GFP_ATOMIC -- immediately after the
> allocation, we call mlx4_cq_resize(), which allocates a command
> mailbox with GFP_KERNEL and then sleeps on a firmware command, so we
> better not be in an atomic context.
> 
> This actually has a real impact, because when this GFP_ATOMIC
> allocation fails (and GFP_ATOMIC does fail in practice) then a
> userspace consumer resizing a CQ will get a spurious failure that we
> can easily avoid.
> 
> Signed-off-by: Roland Dreier <[e-mail]>

Thanks,
Reviewed-by: Leon Romanovsky <[e-mail]>

Doug Ledford Aug. 2, 2016, 5:52 p.m. UTC | #2

On Thu, 2016-07-28 at 21:58 -0700, Roland Dreier wrote:
> From: Roland Dreier <[e-mail]>
> 
> We allocate a small tracking structure as part of
> mlx4_ib_resize_cq().
> However, we don't need to use GFP_ATOMIC -- immediately after the
> allocation, we call mlx4_cq_resize(), which allocates a command
> mailbox with GFP_KERNEL and then sleeps on a firmware command, so we
> better not be in an atomic context.
> 
> This actually has a real impact, because when this GFP_ATOMIC
> allocation fails (and GFP_ATOMIC does fail in practice) then a
> userspace consumer resizing a CQ will get a spurious failure that we
> can easily avoid.
> 
> Signed-off-by: Roland Dreier <[e-mail]>
> 

Thanks, I reworded the commit subject for length and applied.

Patch

diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
index 9f8b516eb2b0..d6fc8a6e8c33 100644
--- a/drivers/infiniband/hw/mlx4/cq.c
+++ b/drivers/infiniband/hw/mlx4/cq.c
@@ -288,7 +288,7 @@  static int mlx4_alloc_resize_buf(struct mlx4_ib_dev *dev, struct mlx4_ib_cq *cq,
 	if (cq->resize_buf)
 		return -EBUSY;
 
-	cq->resize_buf = kmalloc(sizeof *cq->resize_buf, GFP_ATOMIC);
+	cq->resize_buf = kmalloc(sizeof *cq->resize_buf, GFP_KERNEL);
 	if (!cq->resize_buf)
 		return -ENOMEM;
 
@@ -316,7 +316,7 @@  static int mlx4_alloc_resize_umem(struct mlx4_ib_dev *dev, struct mlx4_ib_cq *cq
 	if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd))
 		return -EFAULT;
 
-	cq->resize_buf = kmalloc(sizeof *cq->resize_buf, GFP_ATOMIC);
+	cq->resize_buf = kmalloc(sizeof *cq->resize_buf, GFP_KERNEL);
 	if (!cq->resize_buf)
 		return -ENOMEM;