From: Paul Menzel <pmenzel@molgen.mpg.de> To: Salil Mehta <salil.mehta@huawei.com> Cc: linuxarm@openeuler.org, netdev@vger.kernel.org, linuxarm@huawei.com, linux-kernel@vger.kernel.org, Jeff Kirsher <jeffrey.t.kirsher@intel.com>, intel-wired-lan@lists.osuosl.org, "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org> Subject: Re: [Intel-wired-lan] [PATCH V2 net] ice: Re-organizes reqstd/avail {R, T}XQ check/code for efficiency+readability Date: Wed, 21 Apr 2021 07:35:43 +0200 [thread overview] Message-ID: <7974e665-73bd-401c-f023-9da568e1dffc@molgen.mpg.de> (raw) In-Reply-To: <20210413224446.16612-1-salil.mehta@huawei.com> Dear Salil, Thank you very much for your patch. In the git commit message summary, could you please use imperative mood [1]? > Re-organize reqstd/avail {R, T}XQ check/code for efficiency+readability It’s a bit long though. Maybe: > Avoid unnecessary assignment with user specified {R,T}XQs Am 14.04.21 um 00:44 schrieb Salil Mehta: > If user has explicitly requested the number of {R,T}XQs, then it is > unnecessary to get the count of already available {R,T}XQs from the > PF avail_{r,t}xqs bitmap. This value will get overridden by user specified > value in any case. > > This patch does minor re-organization of the code for improving the flow > and readabiltiy. This scope of improvement was found during the review of readabil*it*y > the ICE driver code. > > FYI, I could not test this change due to unavailability of the hardware. > It would be helpful if somebody can test this patch and provide Tested-by > Tag. Many thanks! This should go outside the commit message (below the --- for example). > Fixes: 87324e747fde ("ice: Implement ethtool ops for channels") Did you check the behavior before is actually a bug? Or is it just for the detection heuristic for commits to be applied to the stable series? > Cc: intel-wired-lan@lists.osuosl.org > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > -- > Change V1->V2 > (*) Fixed the comments from Anthony Nguyen(Intel) > Link: https://lkml.org/lkml/2021/4/12/1997 > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index d13c7fc8fb0a..d77133d6baa7 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -161,12 +161,13 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id) > > switch (vsi->type) { > case ICE_VSI_PF: > - vsi->alloc_txq = min3(pf->num_lan_msix, > - ice_get_avail_txq_count(pf), > - (u16)num_online_cpus()); > if (vsi->req_txq) { > vsi->alloc_txq = vsi->req_txq; > vsi->num_txq = vsi->req_txq; > + } else { > + vsi->alloc_txq = min3(pf->num_lan_msix, > + ice_get_avail_txq_count(pf), > + (u16)num_online_cpus()); > } I am curious, did you check the compiler actually creates different code, or did it notice the inefficiency by itself and optimized it already? > > pf->num_lan_tx = vsi->alloc_txq; > @@ -175,12 +176,13 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id) > if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) { > vsi->alloc_rxq = 1; > } else { > - vsi->alloc_rxq = min3(pf->num_lan_msix, > - ice_get_avail_rxq_count(pf), > - (u16)num_online_cpus()); > if (vsi->req_rxq) { > vsi->alloc_rxq = vsi->req_rxq; > vsi->num_rxq = vsi->req_rxq; > + } else { > + vsi->alloc_rxq = min3(pf->num_lan_msix, > + ice_get_avail_rxq_count(pf), > + (u16)num_online_cpus()); > } > } > Kind regards, Paul
WARNING: multiple messages have this Message-ID (diff)
From: Paul Menzel <pmenzel@molgen.mpg.de> To: intel-wired-lan@osuosl.org Subject: [Intel-wired-lan] [PATCH V2 net] ice: Re-organizes reqstd/avail {R, T}XQ check/code for efficiency+readability Date: Wed, 21 Apr 2021 07:35:43 +0200 [thread overview] Message-ID: <7974e665-73bd-401c-f023-9da568e1dffc@molgen.mpg.de> (raw) In-Reply-To: <20210413224446.16612-1-salil.mehta@huawei.com> Dear Salil, Thank you very much for your patch. In the git commit message summary, could you please use imperative mood [1]? > Re-organize reqstd/avail {R, T}XQ check/code for efficiency+readability It?s a bit long though. Maybe: > Avoid unnecessary assignment with user specified {R,T}XQs Am 14.04.21 um 00:44 schrieb Salil Mehta: > If user has explicitly requested the number of {R,T}XQs, then it is > unnecessary to get the count of already available {R,T}XQs from the > PF avail_{r,t}xqs bitmap. This value will get overridden by user specified > value in any case. > > This patch does minor re-organization of the code for improving the flow > and readabiltiy. This scope of improvement was found during the review of readabil*it*y > the ICE driver code. > > FYI, I could not test this change due to unavailability of the hardware. > It would be helpful if somebody can test this patch and provide Tested-by > Tag. Many thanks! This should go outside the commit message (below the --- for example). > Fixes: 87324e747fde ("ice: Implement ethtool ops for channels") Did you check the behavior before is actually a bug? Or is it just for the detection heuristic for commits to be applied to the stable series? > Cc: intel-wired-lan at lists.osuosl.org > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > -- > Change V1->V2 > (*) Fixed the comments from Anthony Nguyen(Intel) > Link: https://lkml.org/lkml/2021/4/12/1997 > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index d13c7fc8fb0a..d77133d6baa7 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -161,12 +161,13 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id) > > switch (vsi->type) { > case ICE_VSI_PF: > - vsi->alloc_txq = min3(pf->num_lan_msix, > - ice_get_avail_txq_count(pf), > - (u16)num_online_cpus()); > if (vsi->req_txq) { > vsi->alloc_txq = vsi->req_txq; > vsi->num_txq = vsi->req_txq; > + } else { > + vsi->alloc_txq = min3(pf->num_lan_msix, > + ice_get_avail_txq_count(pf), > + (u16)num_online_cpus()); > } I am curious, did you check the compiler actually creates different code, or did it notice the inefficiency by itself and optimized it already? > > pf->num_lan_tx = vsi->alloc_txq; > @@ -175,12 +176,13 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id) > if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) { > vsi->alloc_rxq = 1; > } else { > - vsi->alloc_rxq = min3(pf->num_lan_msix, > - ice_get_avail_rxq_count(pf), > - (u16)num_online_cpus()); > if (vsi->req_rxq) { > vsi->alloc_rxq = vsi->req_rxq; > vsi->num_rxq = vsi->req_rxq; > + } else { > + vsi->alloc_rxq = min3(pf->num_lan_msix, > + ice_get_avail_rxq_count(pf), > + (u16)num_online_cpus()); > } > } > Kind regards, Paul
next prev parent reply other threads:[~2021-04-21 5:35 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-13 22:44 [PATCH V2 net] ice: Re-organizes reqstd/avail {R,T}XQ check/code for efficiency+readability Salil Mehta 2021-04-13 22:44 ` [Intel-wired-lan] [PATCH V2 net] ice: Re-organizes reqstd/avail {R, T}XQ " Salil Mehta 2021-04-20 20:25 ` Brelinski, TonyX 2021-04-20 20:25 ` Brelinski, TonyX 2021-04-20 21:28 ` Salil Mehta 2021-04-20 21:28 ` Salil Mehta 2021-04-21 5:35 ` Paul Menzel [this message] 2021-04-21 5:35 ` Paul Menzel 2021-04-21 7:41 ` Salil Mehta 2021-04-21 7:41 ` Salil Mehta 2021-04-21 7:54 ` Paul Menzel 2021-04-21 7:54 ` Paul Menzel 2021-04-21 8:08 ` Salil Mehta 2021-04-21 8:08 ` Salil Mehta
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=7974e665-73bd-401c-f023-9da568e1dffc@molgen.mpg.de \ --to=pmenzel@molgen.mpg.de \ --cc=davem@davemloft.net \ --cc=intel-wired-lan@lists.osuosl.org \ --cc=jeffrey.t.kirsher@intel.com \ --cc=kuba@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=linuxarm@openeuler.org \ --cc=netdev@vger.kernel.org \ --cc=salil.mehta@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.