[PATCH net-next v1 5/6] ibmvnic: allocate a new long term buffer before freeing the old one

[Mingming Cao <mmc@linux.ibm.com>]

When an mtu change grows the pools, alloc_long_term_buff() releases the
existing buffer as soon as it knows the size has changed, and only then
asks for the replacement:

  if (!reuse_ltb(ltb, size)) {
          prev = ltb->size;
          free_long_term_buff(adapter, ltb);
  }

  if (ltb->buff) {
          ...
  } else {
          ltb->buff = dma_alloc_coherent(dev, size, &ltb->addr,
                                         GFP_KERNEL);
          if (!ltb->buff) {
                  dev_err(dev, "Couldn't alloc long term buffer\n");
                  return -ENOMEM;
          }

This one is megabytes of physically contiguous memory, so it can fail
while there is plenty free. By then the old buffer is gone and the pool
it belongs to has nothing, which takes the interface down.

Allocate into a local buffer and only commit it once the allocation has
succeeded. A failure now returns with the old buffer still in place, so
the mtu change is refused and the interface keeps running at the size
it had.

Both buffers are live across the swap, so an mtu increase now needs the
old and the new at once. That makes the allocation somewhat more likely
to fail in the low memory conditions this is meant to survive, but
failing with the old buffer intact is still the better outcome: the
caller can decline the change instead of losing the interface.

The new map id is taken before the old one is released, so the two
differ across the swap. That is harmless with 255 of them.

Fixes: f8ac0bfa7d7a ("ibmvnic: Reuse LTB when possible")
Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>
Tested-by: Vaishnavi Bhat <vaishnavi@linux.ibm.com>
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 24c8acc42a93..88d0c231a74f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -455,6 +455,7 @@ static bool reuse_ltb(struct ibmvnic_long_term_buff *ltb, int size)
 static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
 				struct ibmvnic_long_term_buff *ltb, int size)
 {
+	struct ibmvnic_long_term_buff new_ltb = {};
 	struct device *dev = &adapter->vdev->dev;
 	u64 prev = 0;
 	int rc;
@@ -464,28 +465,29 @@ static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
 			"LTB size changed from 0x%llx to 0x%x, reallocating\n",
 			 ltb->size, size);
 		prev = ltb->size;
-		free_long_term_buff(adapter, ltb);
-	}
 
-	if (ltb->buff) {
-		dev_dbg(dev, "Reusing LTB [map %d, size 0x%llx]\n",
-			ltb->map_id, ltb->size);
-	} else {
-		ltb->buff = dma_alloc_coherent(dev, size, &ltb->addr,
-					       GFP_KERNEL);
-		if (!ltb->buff) {
+		/* Allocate first so failure leaves the old buffer in place. */
+		new_ltb.buff = dma_alloc_coherent(dev, size, &new_ltb.addr,
+						  GFP_KERNEL);
+		if (!new_ltb.buff) {
 			dev_err(dev, "Couldn't alloc long term buffer\n");
 			return -ENOMEM;
 		}
-		ltb->size = size;
+		new_ltb.size = size;
 
-		ltb->map_id = find_first_zero_bit(adapter->map_ids,
-						  MAX_MAP_ID);
-		bitmap_set(adapter->map_ids, ltb->map_id, 1);
+		new_ltb.map_id = find_first_zero_bit(adapter->map_ids,
+						     MAX_MAP_ID);
+		bitmap_set(adapter->map_ids, new_ltb.map_id, 1);
 
 		dev_dbg(dev,
 			"Allocated new LTB [map %d, size 0x%llx was 0x%llx]\n",
-			 ltb->map_id, ltb->size, prev);
+			 new_ltb.map_id, new_ltb.size, prev);
+
+		free_long_term_buff(adapter, ltb);
+		*ltb = new_ltb;
+	} else {
+		dev_dbg(dev, "Reusing LTB [map %d, size 0x%llx]\n",
+			ltb->map_id, ltb->size);
 	}
 
 	/* Ensure ltb is zeroed - specially when reusing it. */
-- 
2.50.1 (Apple Git-155)