Mingming Cao <[email protected]> writes:
> ibmveth_open() registers the logical LAN, then allocates the RX
> buffer pools. A pool failure jumps to out_free_buffer_pools without
> h_free_logical_lan(), so PHYP still owns the buffer-list page when
> it is unmapped. request_irq() failure already issued the hypercall
> before taking the same label.
>
> Move that h_free to the shared post-register unwind so both paths
> deregister before the buffer list is unmapped.
>
> Fixes: d43732ce021f ("ibmveth: properly unwind on init errors")
> Cc: [email protected]
> Signed-off-by: Mingming Cao <[email protected]>
> ---
> drivers/net/ethernet/ibm/ibmveth.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c
> b/drivers/net/ethernet/ibm/ibmveth.c
> index 73e051d26b9d..3e44f6b714d4 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -718,10 +718,6 @@ static int ibmveth_open(struct net_device *netdev)
> if (rc != 0) {
> netdev_err(netdev, "unable to request irq 0x%x, rc %d\n",
> netdev->irq, rc);
> - do {
> - lpar_rc =
> h_free_logical_lan(adapter->vdev->unit_address);
> - } while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
> -
> goto out_free_buffer_pools;
> }
This code change is good. But I noticed another thing that's a pretty
minor bug of useless code just after this:
rc = -ENOMEM;
rc isn't used in that code path after it is set. Is there some reason I
do not understand to have this here?
> @@ -737,6 +733,9 @@ static int ibmveth_open(struct net_device *netdev)
> return 0;
>
> out_free_buffer_pools:
> + do {
> + lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
> + } while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
> while (--i >= 0) {
> if (adapter->rx_buff_pool[i].active)
> ibmveth_free_buffer_pool(adapter,
Reviewed-by: Dave Marquardt <[email protected]>
