* [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
@ 2021-01-06 21:35 Lijun Pan
2021-01-09 20:40 ` patchwork-bot+netdevbpf
0 siblings, 1 reply; 8+ messages in thread
From: Lijun Pan @ 2021-01-06 21:35 UTC (permalink / raw)
To: netdev; +Cc: Lijun Pan
Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
linkwatch_event can run") introduced do_change_param_reset function to
solve the rtnl lock issue. Majority of the code in do_change_param_reset
duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset
itself. Hence merge do_change_param_reset back into do_reset to clean up
the code.
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
This patch was accepted into net-next as 16b5f5ce351f but was reverted
in 9f32c27eb4fc to yield to other under-testing patches. Since those
bug fix patches are already accepted, resubmit this one.
drivers/net/ethernet/ibm/ibmvnic.c | 154 +++++++++--------------------
1 file changed, 44 insertions(+), 110 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index f302504faa8a..f6d3b20a5361 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1924,92 +1924,6 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
return rc;
}
-/**
- * do_change_param_reset returns zero if we are able to keep processing reset
- * events, or non-zero if we hit a fatal error and must halt.
- */
-static int do_change_param_reset(struct ibmvnic_adapter *adapter,
- struct ibmvnic_rwi *rwi,
- u32 reset_state)
-{
- struct net_device *netdev = adapter->netdev;
- int i, rc;
-
- netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n",
- rwi->reset_reason);
-
- netif_carrier_off(netdev);
- adapter->reset_reason = rwi->reset_reason;
-
- ibmvnic_cleanup(netdev);
-
- if (reset_state == VNIC_OPEN) {
- rc = __ibmvnic_close(netdev);
- if (rc)
- goto out;
- }
-
- release_resources(adapter);
- release_sub_crqs(adapter, 1);
- release_crq_queue(adapter);
-
- adapter->state = VNIC_PROBED;
-
- rc = init_crq_queue(adapter);
-
- if (rc) {
- netdev_err(adapter->netdev,
- "Couldn't initialize crq. rc=%d\n", rc);
- return rc;
- }
-
- rc = ibmvnic_reset_init(adapter, true);
- if (rc) {
- rc = IBMVNIC_INIT_FAILED;
- goto out;
- }
-
- /* If the adapter was in PROBE state prior to the reset,
- * exit here.
- */
- if (reset_state == VNIC_PROBED)
- goto out;
-
- rc = ibmvnic_login(netdev);
- if (rc) {
- goto out;
- }
-
- rc = init_resources(adapter);
- if (rc)
- goto out;
-
- ibmvnic_disable_irqs(adapter);
-
- adapter->state = VNIC_CLOSED;
-
- if (reset_state == VNIC_CLOSED)
- return 0;
-
- rc = __ibmvnic_open(netdev);
- if (rc) {
- rc = IBMVNIC_OPEN_FAILED;
- goto out;
- }
-
- /* refresh device's multicast list */
- ibmvnic_set_multi(netdev);
-
- /* kick napi */
- for (i = 0; i < adapter->req_rx_queues; i++)
- napi_schedule(&adapter->napi[i]);
-
-out:
- if (rc)
- adapter->state = reset_state;
- return rc;
-}
-
/**
* do_reset returns zero if we are able to keep processing reset events, or
* non-zero if we hit a fatal error and must halt.
@@ -2027,7 +1941,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
adapter->state, adapter->failover_pending,
rwi->reset_reason, reset_state);
- rtnl_lock();
+ adapter->reset_reason = rwi->reset_reason;
+ /* requestor of VNIC_RESET_CHANGE_PARAM already has the rtnl lock */
+ if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+ rtnl_lock();
+
/*
* Now that we have the rtnl lock, clear any pending failover.
* This will ensure ibmvnic_open() has either completed or will
@@ -2037,7 +1955,6 @@ static int do_reset(struct ibmvnic_adapter *adapter,
adapter->failover_pending = false;
netif_carrier_off(netdev);
- adapter->reset_reason = rwi->reset_reason;
old_num_rx_queues = adapter->req_rx_queues;
old_num_tx_queues = adapter->req_tx_queues;
@@ -2049,25 +1966,37 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (reset_state == VNIC_OPEN &&
adapter->reset_reason != VNIC_RESET_MOBILITY &&
adapter->reset_reason != VNIC_RESET_FAILOVER) {
- adapter->state = VNIC_CLOSING;
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = __ibmvnic_close(netdev);
+ if (rc)
+ goto out;
+ } else {
+ adapter->state = VNIC_CLOSING;
- /* Release the RTNL lock before link state change and
- * re-acquire after the link state change to allow
- * linkwatch_event to grab the RTNL lock and run during
- * a reset.
- */
- rtnl_unlock();
- rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
- rtnl_lock();
- if (rc)
- goto out;
+ /* Release the RTNL lock before link state change and
+ * re-acquire after the link state change to allow
+ * linkwatch_event to grab the RTNL lock and run during
+ * a reset.
+ */
+ rtnl_unlock();
+ rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+ rtnl_lock();
+ if (rc)
+ goto out;
- if (adapter->state != VNIC_CLOSING) {
- rc = -1;
- goto out;
+ if (adapter->state != VNIC_CLOSING) {
+ rc = -1;
+ goto out;
+ }
+
+ adapter->state = VNIC_CLOSED;
}
+ }
- adapter->state = VNIC_CLOSED;
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ release_resources(adapter);
+ release_sub_crqs(adapter, 1);
+ release_crq_queue(adapter);
}
if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
@@ -2076,7 +2005,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
*/
adapter->state = VNIC_PROBED;
- if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = init_crq_queue(adapter);
+ } else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
rc = ibmvnic_reenable_crq_queue(adapter);
release_sub_crqs(adapter, 1);
} else {
@@ -2115,7 +2046,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
goto out;
}
- if (adapter->req_rx_queues != old_num_rx_queues ||
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = init_resources(adapter);
+ if (rc)
+ goto out;
+ } else if (adapter->req_rx_queues != old_num_rx_queues ||
adapter->req_tx_queues != old_num_tx_queues ||
adapter->req_rx_add_entries_per_subcrq !=
old_num_rx_slots ||
@@ -2180,7 +2115,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
/* restore the adapter state if reset failed */
if (rc)
adapter->state = reset_state;
- rtnl_unlock();
+ /* requestor of VNIC_RESET_CHANGE_PARAM should still hold the rtnl lock */
+ if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+ rtnl_unlock();
netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Reset done, rc %d\n",
adapter->state, adapter->failover_pending, rc);
@@ -2311,10 +2248,7 @@ static void __ibmvnic_reset(struct work_struct *work)
}
spin_unlock_irqrestore(&adapter->state_lock, flags);
- if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
- /* CHANGE_PARAM requestor holds rtnl_lock */
- rc = do_change_param_reset(adapter, rwi, reset_state);
- } else if (adapter->force_reset_recovery) {
+ if (adapter->force_reset_recovery) {
/*
* Since we are doing a hard reset now, clear the
* failover_pending flag so we don't ignore any
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
2021-01-06 21:35 [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset Lijun Pan
@ 2021-01-09 20:40 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-09 20:40 UTC (permalink / raw)
To: Lijun Pan; +Cc: netdev
Hello:
This patch was applied to netdev/net-next.git (refs/heads/master):
On Wed, 6 Jan 2021 15:35:14 -0600 you wrote:
> Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
> linkwatch_event can run") introduced do_change_param_reset function to
> solve the rtnl lock issue. Majority of the code in do_change_param_reset
> duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset
> itself. Hence merge do_change_param_reset back into do_reset to clean up
> the code.
>
> [...]
Here is the summary with links:
- [net-next] ibmvnic: merge do_change_param_reset into do_reset
https://git.kernel.org/netdev/net-next/c/3f5ec374ae3f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
2020-12-15 8:30 Lijun Pan
@ 2020-12-16 21:26 ` Jakub Kicinski
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2020-12-16 21:26 UTC (permalink / raw)
To: Lijun Pan; +Cc: netdev
On Tue, 15 Dec 2020 02:30:08 -0600 Lijun Pan wrote:
> Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
> linkwatch_event can run") introduced do_change_param_reset function to
> solve the rtnl lock issue. Majority of the code in do_change_param_reset
> duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset
> itself. Hence merge do_change_param_reset back into do_reset to clean up
> the code.
>
> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
> ---
> This patch was accepted into net-next as 16b5f5ce351f but was reverted
> in 9f32c27eb4fc to yield to other under-testing patches. Since those
> bug fix patches are already accepted, resubmit this one.
The merge window has started now, and we're merging net into net-next
pretty much every week, so this could have been done sooner. Let's wait
for net-next to reopen.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
@ 2020-12-15 8:30 Lijun Pan
2020-12-16 21:26 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: Lijun Pan @ 2020-12-15 8:30 UTC (permalink / raw)
To: netdev; +Cc: Lijun Pan
Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
linkwatch_event can run") introduced do_change_param_reset function to
solve the rtnl lock issue. Majority of the code in do_change_param_reset
duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset
itself. Hence merge do_change_param_reset back into do_reset to clean up
the code.
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
This patch was accepted into net-next as 16b5f5ce351f but was reverted
in 9f32c27eb4fc to yield to other under-testing patches. Since those
bug fix patches are already accepted, resubmit this one.
drivers/net/ethernet/ibm/ibmvnic.c | 154 +++++++++--------------------
1 file changed, 44 insertions(+), 110 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index f302504faa8a..f6d3b20a5361 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1924,92 +1924,6 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
return rc;
}
-/**
- * do_change_param_reset returns zero if we are able to keep processing reset
- * events, or non-zero if we hit a fatal error and must halt.
- */
-static int do_change_param_reset(struct ibmvnic_adapter *adapter,
- struct ibmvnic_rwi *rwi,
- u32 reset_state)
-{
- struct net_device *netdev = adapter->netdev;
- int i, rc;
-
- netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n",
- rwi->reset_reason);
-
- netif_carrier_off(netdev);
- adapter->reset_reason = rwi->reset_reason;
-
- ibmvnic_cleanup(netdev);
-
- if (reset_state == VNIC_OPEN) {
- rc = __ibmvnic_close(netdev);
- if (rc)
- goto out;
- }
-
- release_resources(adapter);
- release_sub_crqs(adapter, 1);
- release_crq_queue(adapter);
-
- adapter->state = VNIC_PROBED;
-
- rc = init_crq_queue(adapter);
-
- if (rc) {
- netdev_err(adapter->netdev,
- "Couldn't initialize crq. rc=%d\n", rc);
- return rc;
- }
-
- rc = ibmvnic_reset_init(adapter, true);
- if (rc) {
- rc = IBMVNIC_INIT_FAILED;
- goto out;
- }
-
- /* If the adapter was in PROBE state prior to the reset,
- * exit here.
- */
- if (reset_state == VNIC_PROBED)
- goto out;
-
- rc = ibmvnic_login(netdev);
- if (rc) {
- goto out;
- }
-
- rc = init_resources(adapter);
- if (rc)
- goto out;
-
- ibmvnic_disable_irqs(adapter);
-
- adapter->state = VNIC_CLOSED;
-
- if (reset_state == VNIC_CLOSED)
- return 0;
-
- rc = __ibmvnic_open(netdev);
- if (rc) {
- rc = IBMVNIC_OPEN_FAILED;
- goto out;
- }
-
- /* refresh device's multicast list */
- ibmvnic_set_multi(netdev);
-
- /* kick napi */
- for (i = 0; i < adapter->req_rx_queues; i++)
- napi_schedule(&adapter->napi[i]);
-
-out:
- if (rc)
- adapter->state = reset_state;
- return rc;
-}
-
/**
* do_reset returns zero if we are able to keep processing reset events, or
* non-zero if we hit a fatal error and must halt.
@@ -2027,7 +1941,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
adapter->state, adapter->failover_pending,
rwi->reset_reason, reset_state);
- rtnl_lock();
+ adapter->reset_reason = rwi->reset_reason;
+ /* requestor of VNIC_RESET_CHANGE_PARAM already has the rtnl lock */
+ if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+ rtnl_lock();
+
/*
* Now that we have the rtnl lock, clear any pending failover.
* This will ensure ibmvnic_open() has either completed or will
@@ -2037,7 +1955,6 @@ static int do_reset(struct ibmvnic_adapter *adapter,
adapter->failover_pending = false;
netif_carrier_off(netdev);
- adapter->reset_reason = rwi->reset_reason;
old_num_rx_queues = adapter->req_rx_queues;
old_num_tx_queues = adapter->req_tx_queues;
@@ -2049,25 +1966,37 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (reset_state == VNIC_OPEN &&
adapter->reset_reason != VNIC_RESET_MOBILITY &&
adapter->reset_reason != VNIC_RESET_FAILOVER) {
- adapter->state = VNIC_CLOSING;
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = __ibmvnic_close(netdev);
+ if (rc)
+ goto out;
+ } else {
+ adapter->state = VNIC_CLOSING;
- /* Release the RTNL lock before link state change and
- * re-acquire after the link state change to allow
- * linkwatch_event to grab the RTNL lock and run during
- * a reset.
- */
- rtnl_unlock();
- rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
- rtnl_lock();
- if (rc)
- goto out;
+ /* Release the RTNL lock before link state change and
+ * re-acquire after the link state change to allow
+ * linkwatch_event to grab the RTNL lock and run during
+ * a reset.
+ */
+ rtnl_unlock();
+ rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+ rtnl_lock();
+ if (rc)
+ goto out;
- if (adapter->state != VNIC_CLOSING) {
- rc = -1;
- goto out;
+ if (adapter->state != VNIC_CLOSING) {
+ rc = -1;
+ goto out;
+ }
+
+ adapter->state = VNIC_CLOSED;
}
+ }
- adapter->state = VNIC_CLOSED;
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ release_resources(adapter);
+ release_sub_crqs(adapter, 1);
+ release_crq_queue(adapter);
}
if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
@@ -2076,7 +2005,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
*/
adapter->state = VNIC_PROBED;
- if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = init_crq_queue(adapter);
+ } else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
rc = ibmvnic_reenable_crq_queue(adapter);
release_sub_crqs(adapter, 1);
} else {
@@ -2115,7 +2046,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
goto out;
}
- if (adapter->req_rx_queues != old_num_rx_queues ||
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = init_resources(adapter);
+ if (rc)
+ goto out;
+ } else if (adapter->req_rx_queues != old_num_rx_queues ||
adapter->req_tx_queues != old_num_tx_queues ||
adapter->req_rx_add_entries_per_subcrq !=
old_num_rx_slots ||
@@ -2180,7 +2115,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
/* restore the adapter state if reset failed */
if (rc)
adapter->state = reset_state;
- rtnl_unlock();
+ /* requestor of VNIC_RESET_CHANGE_PARAM should still hold the rtnl lock */
+ if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+ rtnl_unlock();
netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Reset done, rc %d\n",
adapter->state, adapter->failover_pending, rc);
@@ -2311,10 +2248,7 @@ static void __ibmvnic_reset(struct work_struct *work)
}
spin_unlock_irqrestore(&adapter->state_lock, flags);
- if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
- /* CHANGE_PARAM requestor holds rtnl_lock */
- rc = do_change_param_reset(adapter, rwi, reset_state);
- } else if (adapter->force_reset_recovery) {
+ if (adapter->force_reset_recovery) {
/*
* Since we are doing a hard reset now, clear the
* failover_pending flag so we don't ignore any
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
2020-11-04 21:56 ` drt
@ 2020-11-04 22:14 ` Jakub Kicinski
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2020-11-04 22:14 UTC (permalink / raw)
To: drt; +Cc: Lijun Pan, netdev, sukadev, drt
On Wed, 04 Nov 2020 13:56:09 -0800 drt wrote:
> On 2020-11-03 15:09, Jakub Kicinski wrote:
> > On Sat, 31 Oct 2020 04:46:45 -0500 Lijun Pan wrote:
> >> Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
> >> linkwatch_event can run") introduced do_change_param_reset function to
> >> solve the rtnl lock issue. Majority of the code in
> >> do_change_param_reset
> >> duplicates do_reset. Also, we can handle the rtnl lock issue in
> >> do_reset
> >> itself. Hence merge do_change_param_reset back into do_reset to clean
> >> up
> >> the code.
> >>
> >> Fixes: b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
> >> linkwatch_event can run")
> >> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
> >
> > Applied, thanks!
>
> Hi Jakub,
>
> Thank you for applying this patch so promptly. However, I would like to
> ask that this patch be withdrawn.
>
> 1. It needs more time in testing.
> 2. There are a number of bug fix patches being tested. This patch would
> require rework of those patches.
> 3. As the lead maintainer for ibmvnic, I failed to communicate this to
> Lijun. I will do better going forward.
>
> Please revert this commit. We will resubmit this patch later.
>
> I sincerely apologize for any trouble this may have caused.
No worries, please just send a revert patch with this information in
the commit message, and I'll apply right away.
BTW feel free to send a note if you need more time to review a
particular patch. Because of the volume of patches we get, I try to
apply things after a day or two. Otherwise the queue gets unmanageable.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
2020-11-03 23:09 ` Jakub Kicinski
@ 2020-11-04 21:56 ` drt
2020-11-04 22:14 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: drt @ 2020-11-04 21:56 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Lijun Pan, netdev, sukadev, drt
On 2020-11-03 15:09, Jakub Kicinski wrote:
> On Sat, 31 Oct 2020 04:46:45 -0500 Lijun Pan wrote:
>> Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
>> linkwatch_event can run") introduced do_change_param_reset function to
>> solve the rtnl lock issue. Majority of the code in
>> do_change_param_reset
>> duplicates do_reset. Also, we can handle the rtnl lock issue in
>> do_reset
>> itself. Hence merge do_change_param_reset back into do_reset to clean
>> up
>> the code.
>>
>> Fixes: b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
>> linkwatch_event can run")
>> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
>
> Applied, thanks!
Hi Jakub,
Thank you for applying this patch so promptly. However, I would like to
ask that this patch be withdrawn.
1. It needs more time in testing.
2. There are a number of bug fix patches being tested. This patch would
require rework of those patches.
3. As the lead maintainer for ibmvnic, I failed to communicate this to
Lijun. I will do better going forward.
Please revert this commit. We will resubmit this patch later.
I sincerely apologize for any trouble this may have caused.
Regards,
Dany
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
2020-10-31 9:46 Lijun Pan
@ 2020-11-03 23:09 ` Jakub Kicinski
2020-11-04 21:56 ` drt
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2020-11-03 23:09 UTC (permalink / raw)
To: Lijun Pan; +Cc: netdev, sukadev, drt
On Sat, 31 Oct 2020 04:46:45 -0500 Lijun Pan wrote:
> Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
> linkwatch_event can run") introduced do_change_param_reset function to
> solve the rtnl lock issue. Majority of the code in do_change_param_reset
> duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset
> itself. Hence merge do_change_param_reset back into do_reset to clean up
> the code.
>
> Fixes: b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run")
> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset
@ 2020-10-31 9:46 Lijun Pan
2020-11-03 23:09 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: Lijun Pan @ 2020-10-31 9:46 UTC (permalink / raw)
To: netdev; +Cc: sukadev, drt, Lijun Pan
Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
linkwatch_event can run") introduced do_change_param_reset function to
solve the rtnl lock issue. Majority of the code in do_change_param_reset
duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset
itself. Hence merge do_change_param_reset back into do_reset to clean up
the code.
Fixes: b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run")
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 147 +++++++++--------------------
1 file changed, 43 insertions(+), 104 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 8148f796a807..9b35cb46a97c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1822,86 +1822,6 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
return rc;
}
-/**
- * do_change_param_reset returns zero if we are able to keep processing reset
- * events, or non-zero if we hit a fatal error and must halt.
- */
-static int do_change_param_reset(struct ibmvnic_adapter *adapter,
- struct ibmvnic_rwi *rwi,
- u32 reset_state)
-{
- struct net_device *netdev = adapter->netdev;
- int i, rc;
-
- netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n",
- rwi->reset_reason);
-
- netif_carrier_off(netdev);
- adapter->reset_reason = rwi->reset_reason;
-
- ibmvnic_cleanup(netdev);
-
- if (reset_state == VNIC_OPEN) {
- rc = __ibmvnic_close(netdev);
- if (rc)
- return rc;
- }
-
- release_resources(adapter);
- release_sub_crqs(adapter, 1);
- release_crq_queue(adapter);
-
- adapter->state = VNIC_PROBED;
-
- rc = init_crq_queue(adapter);
-
- if (rc) {
- netdev_err(adapter->netdev,
- "Couldn't initialize crq. rc=%d\n", rc);
- return rc;
- }
-
- rc = ibmvnic_reset_init(adapter, true);
- if (rc)
- return IBMVNIC_INIT_FAILED;
-
- /* If the adapter was in PROBE state prior to the reset,
- * exit here.
- */
- if (reset_state == VNIC_PROBED)
- return 0;
-
- rc = ibmvnic_login(netdev);
- if (rc) {
- adapter->state = reset_state;
- return rc;
- }
-
- rc = init_resources(adapter);
- if (rc)
- return rc;
-
- ibmvnic_disable_irqs(adapter);
-
- adapter->state = VNIC_CLOSED;
-
- if (reset_state == VNIC_CLOSED)
- return 0;
-
- rc = __ibmvnic_open(netdev);
- if (rc)
- return IBMVNIC_OPEN_FAILED;
-
- /* refresh device's multicast list */
- ibmvnic_set_multi(netdev);
-
- /* kick napi */
- for (i = 0; i < adapter->req_rx_queues; i++)
- napi_schedule(&adapter->napi[i]);
-
- return 0;
-}
-
/**
* do_reset returns zero if we are able to keep processing reset events, or
* non-zero if we hit a fatal error and must halt.
@@ -1917,10 +1837,12 @@ static int do_reset(struct ibmvnic_adapter *adapter,
netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
rwi->reset_reason);
- rtnl_lock();
+ adapter->reset_reason = rwi->reset_reason;
+ /* requestor of VNIC_RESET_CHANGE_PARAM already has the rtnl lock */
+ if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+ rtnl_lock();
netif_carrier_off(netdev);
- adapter->reset_reason = rwi->reset_reason;
old_num_rx_queues = adapter->req_rx_queues;
old_num_tx_queues = adapter->req_tx_queues;
@@ -1932,25 +1854,37 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (reset_state == VNIC_OPEN &&
adapter->reset_reason != VNIC_RESET_MOBILITY &&
adapter->reset_reason != VNIC_RESET_FAILOVER) {
- adapter->state = VNIC_CLOSING;
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = __ibmvnic_close(netdev);
+ if (rc)
+ goto out;
+ } else {
+ adapter->state = VNIC_CLOSING;
- /* Release the RTNL lock before link state change and
- * re-acquire after the link state change to allow
- * linkwatch_event to grab the RTNL lock and run during
- * a reset.
- */
- rtnl_unlock();
- rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
- rtnl_lock();
- if (rc)
- goto out;
+ /* Release the RTNL lock before link state change and
+ * re-acquire after the link state change to allow
+ * linkwatch_event to grab the RTNL lock and run during
+ * a reset.
+ */
+ rtnl_unlock();
+ rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+ rtnl_lock();
+ if (rc)
+ goto out;
- if (adapter->state != VNIC_CLOSING) {
- rc = -1;
- goto out;
+ if (adapter->state != VNIC_CLOSING) {
+ rc = -1;
+ goto out;
+ }
+
+ adapter->state = VNIC_CLOSED;
}
+ }
- adapter->state = VNIC_CLOSED;
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ release_resources(adapter);
+ release_sub_crqs(adapter, 1);
+ release_crq_queue(adapter);
}
if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
@@ -1959,7 +1893,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
*/
adapter->state = VNIC_PROBED;
- if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = init_crq_queue(adapter);
+ } else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
rc = ibmvnic_reenable_crq_queue(adapter);
release_sub_crqs(adapter, 1);
} else {
@@ -1999,7 +1935,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
goto out;
}
- if (adapter->req_rx_queues != old_num_rx_queues ||
+ if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ rc = init_resources(adapter);
+ if (rc)
+ goto out;
+ } else if (adapter->req_rx_queues != old_num_rx_queues ||
adapter->req_tx_queues != old_num_tx_queues ||
adapter->req_rx_add_entries_per_subcrq !=
old_num_rx_slots ||
@@ -2060,7 +2000,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
rc = 0;
out:
- rtnl_unlock();
+ /* requestor of VNIC_RESET_CHANGE_PARAM should still hold the rtnl lock */
+ if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+ rtnl_unlock();
return rc;
}
@@ -2194,10 +2136,7 @@ static void __ibmvnic_reset(struct work_struct *work)
}
spin_unlock_irqrestore(&adapter->state_lock, flags);
- if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
- /* CHANGE_PARAM requestor holds rtnl_lock */
- rc = do_change_param_reset(adapter, rwi, reset_state);
- } else if (adapter->force_reset_recovery) {
+ if (adapter->force_reset_recovery) {
/* Transport event occurred during previous reset */
if (adapter->wait_for_reset) {
/* Previous was CHANGE_PARAM; caller locked */
--
2.22.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-09 20:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-06 21:35 [PATCH net-next] ibmvnic: merge do_change_param_reset into do_reset Lijun Pan
2021-01-09 20:40 ` patchwork-bot+netdevbpf
-- strict thread matches above, loose matches on Subject: below --
2020-12-15 8:30 Lijun Pan
2020-12-16 21:26 ` Jakub Kicinski
2020-10-31 9:46 Lijun Pan
2020-11-03 23:09 ` Jakub Kicinski
2020-11-04 21:56 ` drt
2020-11-04 22:14 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).