* [PATCH net-next v2 0/2] Small fixes for redundant checks @ 2021-10-22 3:37 Jεan Sacren 2021-10-22 3:37 ` [PATCH net-next v2 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL Jεan Sacren 2021-10-22 3:37 ` [PATCH net-next v2 1/2] net: qed_ptp: " Jεan Sacren 0 siblings, 2 replies; 6+ messages in thread From: Jεan Sacren @ 2021-10-22 3:37 UTC (permalink / raw) To: Ariel Elior; +Cc: GR-everest-linux-l2, davem, kuba, netdev From: Jean Sacren <sakiwit@gmail.com> This series fixes some redundant checks of certain variables and expressions. Jean Sacren (2): net: qed_ptp: fix redundant check of rc and against -EINVAL net: qed_dev: fix redundant check of rc and against -EINVAL drivers/net/ethernet/qlogic/qed/qed_dev.c | 31 +++++++++++++---------- drivers/net/ethernet/qlogic/qed/qed_ptp.c | 12 +++++---- 2 files changed, 24 insertions(+), 19 deletions(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v2 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL 2021-10-22 3:37 [PATCH net-next v2 0/2] Small fixes for redundant checks Jεan Sacren @ 2021-10-22 3:37 ` Jεan Sacren 2021-10-22 21:47 ` Jakub Kicinski 2021-10-22 3:37 ` [PATCH net-next v2 1/2] net: qed_ptp: " Jεan Sacren 1 sibling, 1 reply; 6+ messages in thread From: Jεan Sacren @ 2021-10-22 3:37 UTC (permalink / raw) To: Ariel Elior; +Cc: GR-everest-linux-l2, davem, kuba, netdev From: Jean Sacren <sakiwit@gmail.com> We should first check rc alone and then check it against -EINVAL to avoid repeating the same operation multiple times. We should also remove the check of !rc in this expression since it is always true: (!rc && !resc_lock_params.b_granted) Signed-off-by: Jean Sacren <sakiwit@gmail.com> --- v2: (1) Fix missing else branch. I'm very sorry. (2) Add text for !rc removal in the changelog. (3) Put two lines of qed_mcp_resc_unlock() call into one. Thank you, Mr. Horman! drivers/net/ethernet/qlogic/qed/qed_dev.c | 31 +++++++++++++---------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c index 18f3bf7c4dfe..4ae9867b2535 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c @@ -3987,26 +3987,29 @@ static int qed_hw_get_resc(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) QED_RESC_LOCK_RESC_ALLOC, false); rc = qed_mcp_resc_lock(p_hwfn, p_ptt, &resc_lock_params); - if (rc && rc != -EINVAL) { - return rc; - } else if (rc == -EINVAL) { + if (rc) { + if (rc != -EINVAL) + return rc; DP_INFO(p_hwfn, "Skip the max values setting of the soft resources since the resource lock is not supported by the MFW\n"); - } else if (!rc && !resc_lock_params.b_granted) { - DP_NOTICE(p_hwfn, - "Failed to acquire the resource lock for the resource allocation commands\n"); - return -EBUSY; } else { - rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); - if (rc && rc != -EINVAL) { + if (!resc_lock_params.b_granted) { DP_NOTICE(p_hwfn, - "Failed to set the max values of the soft resources\n"); - goto unlock_and_exit; - } else if (rc == -EINVAL) { + "Failed to acquire the resource lock for the resource allocation commands\n"); + return -EBUSY; + } + + rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); + if (rc) { + if (rc != -EINVAL) { + DP_NOTICE(p_hwfn, + "Failed to set the max values of the soft resources\n"); + goto unlock_and_exit; + } + DP_INFO(p_hwfn, "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); - rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, - &resc_unlock_params); + rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, &resc_unlock_params); if (rc) DP_INFO(p_hwfn, "Failed to release the resource lock for the resource allocation commands\n"); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL 2021-10-22 3:37 ` [PATCH net-next v2 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL Jεan Sacren @ 2021-10-22 21:47 ` Jakub Kicinski 2021-10-23 9:26 ` Jεan Sacren 0 siblings, 1 reply; 6+ messages in thread From: Jakub Kicinski @ 2021-10-22 21:47 UTC (permalink / raw) To: Jεan Sacren; +Cc: Ariel Elior, GR-everest-linux-l2, davem, netdev On Thu, 21 Oct 2021 21:37:41 -0600 Jεan Sacren wrote: > From: Jean Sacren <sakiwit@gmail.com> > > We should first check rc alone and then check it against -EINVAL to > avoid repeating the same operation multiple times. > > We should also remove the check of !rc in this expression since it is > always true: > > (!rc && !resc_lock_params.b_granted) > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> The code seems to be written like this on purpose. You're adding indentation levels, and making the structure less readable IMO. If you want to avoid checking rc / !rc multiple times you can just code it as: if (rc == -EINVAL) ... else if (rc) ... else if (!granted) ... else ... I'm not sure I see the point of the re-factoring. > (1) Fix missing else branch. I'm very sorry. > (2) Add text for !rc removal in the changelog. > (3) Put two lines of qed_mcp_resc_unlock() call into one. > Thank you, Mr. Horman! > drivers/net/ethernet/qlogic/qed/qed_dev.c | 31 +++++++++++++---------- > 1 file changed, 17 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c > index 18f3bf7c4dfe..4ae9867b2535 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c > @@ -3987,26 +3987,29 @@ static int qed_hw_get_resc(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > QED_RESC_LOCK_RESC_ALLOC, false); > > rc = qed_mcp_resc_lock(p_hwfn, p_ptt, &resc_lock_params); > - if (rc && rc != -EINVAL) { > - return rc; > - } else if (rc == -EINVAL) { > + if (rc) { > + if (rc != -EINVAL) > + return rc; > DP_INFO(p_hwfn, > "Skip the max values setting of the soft resources since the resource lock is not supported by the MFW\n"); > - } else if (!rc && !resc_lock_params.b_granted) { > - DP_NOTICE(p_hwfn, > - "Failed to acquire the resource lock for the resource allocation commands\n"); > - return -EBUSY; > } else { > - rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); > - if (rc && rc != -EINVAL) { > + if (!resc_lock_params.b_granted) { > DP_NOTICE(p_hwfn, > - "Failed to set the max values of the soft resources\n"); > - goto unlock_and_exit; > - } else if (rc == -EINVAL) { > + "Failed to acquire the resource lock for the resource allocation commands\n"); > + return -EBUSY; > + } > + > + rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); > + if (rc) { > + if (rc != -EINVAL) { > + DP_NOTICE(p_hwfn, > + "Failed to set the max values of the soft resources\n"); > + goto unlock_and_exit; > + } > + > DP_INFO(p_hwfn, > "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); > - rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, > - &resc_unlock_params); > + rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, &resc_unlock_params); > if (rc) > DP_INFO(p_hwfn, > "Failed to release the resource lock for the resource allocation commands\n"); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL 2021-10-22 21:47 ` Jakub Kicinski @ 2021-10-23 9:26 ` Jεan Sacren 0 siblings, 0 replies; 6+ messages in thread From: Jεan Sacren @ 2021-10-23 9:26 UTC (permalink / raw) To: Jakub Kicinski; +Cc: Ariel Elior, GR-everest-linux-l2, davem, netdev From: Jakub Kicinski <kuba@kernel.org> Date: Fri, 22 Oct 2021 14:47:20 -0700 > > On Thu, 21 Oct 2021 21:37:41 -0600 Jεan Sacren wrote: > > From: Jean Sacren <sakiwit@gmail.com> > > > > We should first check rc alone and then check it against -EINVAL to > > avoid repeating the same operation multiple times. > > > > We should also remove the check of !rc in this expression since it is > > always true: > > > > (!rc && !resc_lock_params.b_granted) > > > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> > > The code seems to be written like this on purpose. You're adding > indentation levels, and making the structure less readable IMO. > > If you want to avoid checking rc / !rc multiple times you can just > code it as: > > if (rc == -EINVAL) > ... > else if (rc) > ... > else if (!granted) > ... > else > ... > > I'm not sure I see the point of the re-factoring. Agreed. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v2 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL 2021-10-22 3:37 [PATCH net-next v2 0/2] Small fixes for redundant checks Jεan Sacren 2021-10-22 3:37 ` [PATCH net-next v2 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL Jεan Sacren @ 2021-10-22 3:37 ` Jεan Sacren 2021-10-22 5:27 ` [EXT] " Prabhakar Kushwaha 1 sibling, 1 reply; 6+ messages in thread From: Jεan Sacren @ 2021-10-22 3:37 UTC (permalink / raw) To: Ariel Elior; +Cc: GR-everest-linux-l2, davem, kuba, netdev From: Jean Sacren <sakiwit@gmail.com> We should first check rc alone and then check it against -EINVAL to avoid repeating the same operation. We should also remove the check of !rc in (!rc && !params.b_granted) since it is always true. With this change, we could also use constant 0 for return. Signed-off-by: Jean Sacren <sakiwit@gmail.com> Reviewed-by: Simon Horman <horms@kernel.org> --- v2: (1) Add text for !rc removal in the changelog. (2) Add Reviewed-by tag of Mr. Simon Horman. drivers/net/ethernet/qlogic/qed/qed_ptp.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c index 2c62d732e5c2..c927ff409109 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c +++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c @@ -52,9 +52,9 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) qed_mcp_resc_lock_default_init(¶ms, NULL, resource, true); rc = qed_mcp_resc_lock(p_hwfn, p_ptt, ¶ms); - if (rc && rc != -EINVAL) { - return rc; - } else if (rc == -EINVAL) { + if (rc) { + if (rc != -EINVAL) + return rc; /* MFW doesn't support resource locking, first PF on the port * has lock ownership. */ @@ -63,12 +63,14 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) DP_INFO(p_hwfn, "PF doesn't have lock ownership\n"); return -EBUSY; - } else if (!rc && !params.b_granted) { + } + + if (!params.b_granted) { DP_INFO(p_hwfn, "Failed to acquire ptp resource lock\n"); return -EBUSY; } - return rc; + return 0; } static int qed_ptp_res_unlock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [EXT] [PATCH net-next v2 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL 2021-10-22 3:37 ` [PATCH net-next v2 1/2] net: qed_ptp: " Jεan Sacren @ 2021-10-22 5:27 ` Prabhakar Kushwaha 0 siblings, 0 replies; 6+ messages in thread From: Prabhakar Kushwaha @ 2021-10-22 5:27 UTC (permalink / raw) To: Jεan Sacren, Ariel Elior Cc: GR-everest-linux-l2, davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org > -----Original Message----- > From: Jεan Sacren <sakiwit@gmail.com> > Sent: Friday, October 22, 2021 9:08 AM > To: Ariel Elior <aelior@marvell.com> > Cc: GR-everest-linux-l2 <GR-everest-linux-l2@marvell.com>; > davem@davemloft.net; kuba@kernel.org; netdev@vger.kernel.org > Subject: [EXT] [PATCH net-next v2 1/2] net: qed_ptp: fix redundant check of rc > and against -EINVAL > > External Email > > ---------------------------------------------------------------------- > From: Jean Sacren <sakiwit@gmail.com> > > We should first check rc alone and then check it against -EINVAL to > avoid repeating the same operation. > > We should also remove the check of !rc in (!rc && !params.b_granted) > since it is always true. > > With this change, we could also use constant 0 for return. > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> > Reviewed-by: Simon Horman <horms@kernel.org> > --- Acked-by: Prabhakar Kushwaha <pkushwaha@marvell.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-23 9:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-10-22 3:37 [PATCH net-next v2 0/2] Small fixes for redundant checks Jεan Sacren 2021-10-22 3:37 ` [PATCH net-next v2 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL Jεan Sacren 2021-10-22 21:47 ` Jakub Kicinski 2021-10-23 9:26 ` Jεan Sacren 2021-10-22 3:37 ` [PATCH net-next v2 1/2] net: qed_ptp: " Jεan Sacren 2021-10-22 5:27 ` [EXT] " Prabhakar Kushwaha
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).