Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
On Tue, Jan 09, 2018 at 08:22:15PM +0200, Tal Gilboa wrote: > On 1/9/2018 6:06 PM, Andy Gospodarek wrote: > > On Mon, Jan 08, 2018 at 11:06:28PM -0800, Saeed Mahameed wrote: > > > > > > > > > On 01/08/2018 10:13 PM, Andy Gospodarek wrote: > > > > From: Andy Gospodarek > > > > > > > > Change all appropriate mlx5_am* and MLX5_AM* references to net_dim and > > > > NET_DIM, respectively, in code that handles dynamic interrupt > > > > moderation. Also change all references from 'am' to 'dim' when used as > > > > local variables and add generic profile references. > > > > > > > > Signed-off-by: Andy Gospodarek > > > > Acked-by: Tal Gilboa > > > > Acked-by: Saeed Mahameed > > > > --- > > > >drivers/net/ethernet/mellanox/mlx5/core/en.h | 9 +- > > > >drivers/net/ethernet/mellanox/mlx5/core/en_dim.c | 14 +- > > > >.../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 +- > > > >drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 40 ++- > > > >drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 8 +- > > > >drivers/net/ethernet/mellanox/mlx5/core/net_dim.c | 286 > > > > ++--- > > > >drivers/net/ethernet/mellanox/mlx5/core/net_dim.h | 63 ++--- > > > >7 files changed, 225 insertions(+), 201 deletions(-) > > > > > > > > > > [...] > > > > > > >#define IS_SIGNIFICANT_DIFF(val, ref) \ > > > > (((100 * abs((val) - (ref))) / (ref)) > 10) /* more than 10% > > > > difference */ > > > > -static int mlx5e_am_stats_compare(struct mlx5e_rx_am_stats *curr, > > > > - struct mlx5e_rx_am_stats *prev) > > > > +static int net_dim_stats_compare(struct net_dim_stats *curr, > > > > +struct net_dim_stats *prev) > > > >{ > > > > if (!prev->bpms) > > > > - return curr->bpms ? MLX5E_AM_STATS_BETTER : > > > > - MLX5E_AM_STATS_SAME; > > > > + return curr->bpms ? NET_DIM_STATS_BETTER : > > > > + NET_DIM_STATS_SAME; > > > > if (IS_SIGNIFICANT_DIFF(curr->bpms, prev->bpms)) > > > > - return (curr->bpms > prev->bpms) ? > > > > MLX5E_AM_STATS_BETTER : > > > > - MLX5E_AM_STATS_WORSE; > > > > + return (curr->bpms > prev->bpms) ? NET_DIM_STATS_BETTER > > > > : > > > > + NET_DIM_STATS_WORSE; > > > > > > Hey Andy, > > > > > > I am currently reviewing a patch internally that fixes a bug in this area, > > > prev->ppms can be 0 and could cause IS_SIGNIFICANT_DIFF ouch ! > > > same goes for prev->eppm, for some reason we had a broken assumption that > > > if > > > ppms is 0 for some reason then the bpms is 0 and the above condition will > > > cover us. > > > > > > Anyway the patch will go to net, which means when this series gets > > > accepted > > > then net-next will fail to merge with net and we need to manually push the > > > fix to the new DIM library. > > > > > > But for now I don't think anything is required for this series other than > > > bringing this division by 0 issue and the future merge conflict to your > > > attention. > > > > > > > Thanks for bringing that to everyone's attention. I agree there is > > probably not much that should be done at this point -- hopefully the > > merge should go pretty smoothly, since net_dim.h is seen as a rename > > from en_rx_am.c. > > I talked with Talat, who is submitting the fix. He will apply it over these > patches after they are accepted. > Awesome! Thanks for doing that. v4 coming in a few mins -- hopefully the last. :-) > > > > > > > > if (IS_SIGNIFICANT_DIFF(curr->ppms, prev->ppms)) > > > > - return (curr->ppms > prev->ppms) ? > > > > MLX5E_AM_STATS_BETTER : > > > > - MLX5E_AM_STATS_WORSE; > > > > + return (curr->ppms > prev->ppms) ? NET_DIM_STATS_BETTER > > > > : > > > > + NET_DIM_STATS_WORSE; > > > > if (IS_SIGNIFICANT_DIFF(curr->epms, prev->epms)) > > > > - return (curr->epms < prev->epms) ? > > > > MLX5E_AM_STATS_BETTER : > > > > - MLX5E_AM_STATS_WORSE; > > > > + return (curr->epms < prev->epms) ? NET_DIM_STATS_BETTER > > > > : > > > > + NET_DIM_STATS_WORSE; > > > > - return MLX5E_AM_STATS_SAME; > > > > + return NET_DIM_STATS_SAME; > > > >}
Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
On 1/9/2018 6:06 PM, Andy Gospodarek wrote: On Mon, Jan 08, 2018 at 11:06:28PM -0800, Saeed Mahameed wrote: On 01/08/2018 10:13 PM, Andy Gospodarek wrote: From: Andy Gospodarek Change all appropriate mlx5_am* and MLX5_AM* references to net_dim and NET_DIM, respectively, in code that handles dynamic interrupt moderation. Also change all references from 'am' to 'dim' when used as local variables and add generic profile references. Signed-off-by: Andy Gospodarek Acked-by: Tal Gilboa Acked-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/en.h | 9 +- drivers/net/ethernet/mellanox/mlx5/core/en_dim.c | 14 +- .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 +- drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 40 ++- drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 8 +- drivers/net/ethernet/mellanox/mlx5/core/net_dim.c | 286 ++--- drivers/net/ethernet/mellanox/mlx5/core/net_dim.h | 63 ++--- 7 files changed, 225 insertions(+), 201 deletions(-) [...] #define IS_SIGNIFICANT_DIFF(val, ref) \ (((100 * abs((val) - (ref))) / (ref)) > 10) /* more than 10% difference */ -static int mlx5e_am_stats_compare(struct mlx5e_rx_am_stats *curr, - struct mlx5e_rx_am_stats *prev) +static int net_dim_stats_compare(struct net_dim_stats *curr, +struct net_dim_stats *prev) { if (!prev->bpms) - return curr->bpms ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_SAME; + return curr->bpms ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_SAME; if (IS_SIGNIFICANT_DIFF(curr->bpms, prev->bpms)) - return (curr->bpms > prev->bpms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->bpms > prev->bpms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; Hey Andy, I am currently reviewing a patch internally that fixes a bug in this area, prev->ppms can be 0 and could cause IS_SIGNIFICANT_DIFF ouch ! same goes for prev->eppm, for some reason we had a broken assumption that if ppms is 0 for some reason then the bpms is 0 and the above condition will cover us. Anyway the patch will go to net, which means when this series gets accepted then net-next will fail to merge with net and we need to manually push the fix to the new DIM library. But for now I don't think anything is required for this series other than bringing this division by 0 issue and the future merge conflict to your attention. Thanks for bringing that to everyone's attention. I agree there is probably not much that should be done at this point -- hopefully the merge should go pretty smoothly, since net_dim.h is seen as a rename from en_rx_am.c. I talked with Talat, who is submitting the fix. He will apply it over these patches after they are accepted. if (IS_SIGNIFICANT_DIFF(curr->ppms, prev->ppms)) - return (curr->ppms > prev->ppms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->ppms > prev->ppms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; if (IS_SIGNIFICANT_DIFF(curr->epms, prev->epms)) - return (curr->epms < prev->epms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->epms < prev->epms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; - return MLX5E_AM_STATS_SAME; + return NET_DIM_STATS_SAME; }
Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
On Mon, Jan 08, 2018 at 11:06:28PM -0800, Saeed Mahameed wrote: > > > On 01/08/2018 10:13 PM, Andy Gospodarek wrote: > > From: Andy Gospodarek > > > > Change all appropriate mlx5_am* and MLX5_AM* references to net_dim and > > NET_DIM, respectively, in code that handles dynamic interrupt > > moderation. Also change all references from 'am' to 'dim' when used as > > local variables and add generic profile references. > > > > Signed-off-by: Andy Gospodarek > > Acked-by: Tal Gilboa > > Acked-by: Saeed Mahameed > > --- > > drivers/net/ethernet/mellanox/mlx5/core/en.h | 9 +- > > drivers/net/ethernet/mellanox/mlx5/core/en_dim.c | 14 +- > > .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 +- > > drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 40 ++- > > drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 8 +- > > drivers/net/ethernet/mellanox/mlx5/core/net_dim.c | 286 > > ++--- > > drivers/net/ethernet/mellanox/mlx5/core/net_dim.h | 63 ++--- > > 7 files changed, 225 insertions(+), 201 deletions(-) > > > > [...] > > > #define IS_SIGNIFICANT_DIFF(val, ref) \ > > (((100 * abs((val) - (ref))) / (ref)) > 10) /* more than 10% difference > > */ > > -static int mlx5e_am_stats_compare(struct mlx5e_rx_am_stats *curr, > > - struct mlx5e_rx_am_stats *prev) > > +static int net_dim_stats_compare(struct net_dim_stats *curr, > > +struct net_dim_stats *prev) > > { > > if (!prev->bpms) > > - return curr->bpms ? MLX5E_AM_STATS_BETTER : > > - MLX5E_AM_STATS_SAME; > > + return curr->bpms ? NET_DIM_STATS_BETTER : > > + NET_DIM_STATS_SAME; > > if (IS_SIGNIFICANT_DIFF(curr->bpms, prev->bpms)) > > - return (curr->bpms > prev->bpms) ? MLX5E_AM_STATS_BETTER : > > - MLX5E_AM_STATS_WORSE; > > + return (curr->bpms > prev->bpms) ? NET_DIM_STATS_BETTER : > > + NET_DIM_STATS_WORSE; > > Hey Andy, > > I am currently reviewing a patch internally that fixes a bug in this area, > prev->ppms can be 0 and could cause IS_SIGNIFICANT_DIFF ouch ! > same goes for prev->eppm, for some reason we had a broken assumption that if > ppms is 0 for some reason then the bpms is 0 and the above condition will > cover us. > > Anyway the patch will go to net, which means when this series gets accepted > then net-next will fail to merge with net and we need to manually push the > fix to the new DIM library. > > But for now I don't think anything is required for this series other than > bringing this division by 0 issue and the future merge conflict to your > attention. > Thanks for bringing that to everyone's attention. I agree there is probably not much that should be done at this point -- hopefully the merge should go pretty smoothly, since net_dim.h is seen as a rename from en_rx_am.c. > > if (IS_SIGNIFICANT_DIFF(curr->ppms, prev->ppms)) > > - return (curr->ppms > prev->ppms) ? MLX5E_AM_STATS_BETTER : > > - MLX5E_AM_STATS_WORSE; > > + return (curr->ppms > prev->ppms) ? NET_DIM_STATS_BETTER : > > + NET_DIM_STATS_WORSE; > > if (IS_SIGNIFICANT_DIFF(curr->epms, prev->epms)) > > - return (curr->epms < prev->epms) ? MLX5E_AM_STATS_BETTER : > > - MLX5E_AM_STATS_WORSE; > > + return (curr->epms < prev->epms) ? NET_DIM_STATS_BETTER : > > + NET_DIM_STATS_WORSE; > > - return MLX5E_AM_STATS_SAME; > > + return NET_DIM_STATS_SAME; > > }
Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
Hi Andy, I love your patch! Perhaps something to improve: [auto build test WARNING on net-next/master] url: https://github.com/0day-ci/linux/commits/Andy-Gospodarek/net-create-dynamic-software-irq-moderation-library/20180109-182838 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:15: sparse: no member 'rx_am_enabled' in struct mlx5e_params >> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:33: sparse: call with >> no type! >> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:33: sparse: call with >> no type! >> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:33: sparse: unknown >> expression (14 0) drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:33: sparse: unknown expression (30 46) >> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:33: sparse: unknown >> expression (14 0) >> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:15: sparse: generating >> address of non-lvalue (8) drivers/net/ethernet/mellanox/mlx5/core/en_rep.c: In function 'mlx5e_build_rep_params': drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:10: error: 'struct mlx5e_params' has no member named 'rx_am_enabled'; did you mean params->rx_am_enabled = MLX5_CAP_GEN(mdev, cq_moderation); ^ rx_dim_enabled vim +887 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c cb67b832 Hadar Hen Zion 2016-07-01 875 6a9764ef Saeed Mahameed 2016-12-21 876 static void mlx5e_build_rep_params(struct mlx5_core_dev *mdev, 6a9764ef Saeed Mahameed 2016-12-21 877 struct mlx5e_params *params) cb67b832 Hadar Hen Zion 2016-07-01 878 { cb67b832 Hadar Hen Zion 2016-07-01 879 u8 cq_period_mode = MLX5_CAP_GEN(mdev, cq_period_start_from_cqe) ? cb67b832 Hadar Hen Zion 2016-07-01 880 MLX5_CQ_PERIOD_MODE_START_FROM_CQE : cb67b832 Hadar Hen Zion 2016-07-01 881 MLX5_CQ_PERIOD_MODE_START_FROM_EQE; cb67b832 Hadar Hen Zion 2016-07-01 882 6a9764ef Saeed Mahameed 2016-12-21 883 params->log_sq_size = MLX5E_PARAMS_MINIMUM_LOG_SQ_SIZE; 6a9764ef Saeed Mahameed 2016-12-21 884 params->rq_wq_type = MLX5_WQ_TYPE_LINKED_LIST; 6a9764ef Saeed Mahameed 2016-12-21 885 params->log_rq_size = MLX5E_PARAMS_MINIMUM_LOG_RQ_SIZE; cb67b832 Hadar Hen Zion 2016-07-01 886 6a9764ef Saeed Mahameed 2016-12-21 @887 params->rx_am_enabled = MLX5_CAP_GEN(mdev, cq_moderation); 6a9764ef Saeed Mahameed 2016-12-21 888 mlx5e_set_rx_cq_mode_params(params, cq_period_mode); cb67b832 Hadar Hen Zion 2016-07-01 889 6a9764ef Saeed Mahameed 2016-12-21 890 params->tx_max_inline = mlx5e_get_max_inline_cap(mdev); 6a9764ef Saeed Mahameed 2016-12-21 891 params->num_tc= 1; 6a9764ef Saeed Mahameed 2016-12-21 892 params->lro_wqe_sz= MLX5E_PARAMS_DEFAULT_LRO_WQE_SZ; 5f195c2c Chris Mi 2017-05-16 893 5f195c2c Chris Mi 2017-05-16 894 mlx5_query_min_inline(mdev, ¶ms->tx_min_inline_mode); cb67b832 Hadar Hen Zion 2016-07-01 895 } cb67b832 Hadar Hen Zion 2016-07-01 896 :: The code at line 887 was first introduced by commit :: 6a9764efb255f49a91e229799c38d5c1c9361987 net/mlx5e: Isolate open_channels from priv->params :: TO: Saeed Mahameed :: CC: Saeed Mahameed --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
Hi Andy, I love your patch! Yet something to improve: [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/Andy-Gospodarek/net-create-dynamic-software-irq-moderation-library/20180109-182838 config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): drivers/net/ethernet/mellanox/mlx5/core/en_rep.c: In function 'mlx5e_build_rep_params': >> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:887:10: error: 'struct >> mlx5e_params' has no member named 'rx_am_enabled'; did you mean >> 'rx_dim_enabled'? params->rx_am_enabled = MLX5_CAP_GEN(mdev, cq_moderation); ^ rx_dim_enabled vim +887 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c cb67b832 Hadar Hen Zion 2016-07-01 875 6a9764ef Saeed Mahameed 2016-12-21 876 static void mlx5e_build_rep_params(struct mlx5_core_dev *mdev, 6a9764ef Saeed Mahameed 2016-12-21 877 struct mlx5e_params *params) cb67b832 Hadar Hen Zion 2016-07-01 878 { cb67b832 Hadar Hen Zion 2016-07-01 879 u8 cq_period_mode = MLX5_CAP_GEN(mdev, cq_period_start_from_cqe) ? cb67b832 Hadar Hen Zion 2016-07-01 880 MLX5_CQ_PERIOD_MODE_START_FROM_CQE : cb67b832 Hadar Hen Zion 2016-07-01 881 MLX5_CQ_PERIOD_MODE_START_FROM_EQE; cb67b832 Hadar Hen Zion 2016-07-01 882 6a9764ef Saeed Mahameed 2016-12-21 883 params->log_sq_size = MLX5E_PARAMS_MINIMUM_LOG_SQ_SIZE; 6a9764ef Saeed Mahameed 2016-12-21 884 params->rq_wq_type = MLX5_WQ_TYPE_LINKED_LIST; 6a9764ef Saeed Mahameed 2016-12-21 885 params->log_rq_size = MLX5E_PARAMS_MINIMUM_LOG_RQ_SIZE; cb67b832 Hadar Hen Zion 2016-07-01 886 6a9764ef Saeed Mahameed 2016-12-21 @887 params->rx_am_enabled = MLX5_CAP_GEN(mdev, cq_moderation); 6a9764ef Saeed Mahameed 2016-12-21 888 mlx5e_set_rx_cq_mode_params(params, cq_period_mode); cb67b832 Hadar Hen Zion 2016-07-01 889 6a9764ef Saeed Mahameed 2016-12-21 890 params->tx_max_inline = mlx5e_get_max_inline_cap(mdev); 6a9764ef Saeed Mahameed 2016-12-21 891 params->num_tc= 1; 6a9764ef Saeed Mahameed 2016-12-21 892 params->lro_wqe_sz= MLX5E_PARAMS_DEFAULT_LRO_WQE_SZ; 5f195c2c Chris Mi 2017-05-16 893 5f195c2c Chris Mi 2017-05-16 894 mlx5_query_min_inline(mdev, ¶ms->tx_min_inline_mode); cb67b832 Hadar Hen Zion 2016-07-01 895 } cb67b832 Hadar Hen Zion 2016-07-01 896 :: The code at line 887 was first introduced by commit :: 6a9764efb255f49a91e229799c38d5c1c9361987 net/mlx5e: Isolate open_channels from priv->params :: TO: Saeed Mahameed :: CC: Saeed Mahameed --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
On 01/08/2018 11:06 PM, Saeed Mahameed wrote: On 01/08/2018 10:13 PM, Andy Gospodarek wrote: From: Andy Gospodarek Change all appropriate mlx5_am* and MLX5_AM* references to net_dim and NET_DIM, respectively, in code that handles dynamic interrupt moderation. Also change all references from 'am' to 'dim' when used as local variables and add generic profile references. Signed-off-by: Andy Gospodarek Acked-by: Tal Gilboa Acked-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/en.h | 9 +- drivers/net/ethernet/mellanox/mlx5/core/en_dim.c | 14 +- .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 +- drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 40 ++- drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 8 +- drivers/net/ethernet/mellanox/mlx5/core/net_dim.c | 286 ++--- drivers/net/ethernet/mellanox/mlx5/core/net_dim.h | 63 ++--- 7 files changed, 225 insertions(+), 201 deletions(-) [...] #define IS_SIGNIFICANT_DIFF(val, ref) \ (((100 * abs((val) - (ref))) / (ref)) > 10) /* more than 10% difference */ -static int mlx5e_am_stats_compare(struct mlx5e_rx_am_stats *curr, - struct mlx5e_rx_am_stats *prev) +static int net_dim_stats_compare(struct net_dim_stats *curr, + struct net_dim_stats *prev) { if (!prev->bpms) - return curr->bpms ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_SAME; + return curr->bpms ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_SAME; if (IS_SIGNIFICANT_DIFF(curr->bpms, prev->bpms)) - return (curr->bpms > prev->bpms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->bpms > prev->bpms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; Hey Andy, I am currently reviewing a patch internally that fixes a bug in this area, prev->ppms can be 0 and could cause IS_SIGNIFICANT_DIFF ouch ! I meant cause division by 0 in "IS_SIGNIFICANT_DIFF" same goes for prev->eppm, for some reason we had a broken assumption that if ppms is 0 for some reason then the bpms is 0 and the above condition will cover us. Anyway the patch will go to net, which means when this series gets accepted then net-next will fail to merge with net and we need to manually push the fix to the new DIM library. But for now I don't think anything is required for this series other than bringing this division by 0 issue and the future merge conflict to your attention. if (IS_SIGNIFICANT_DIFF(curr->ppms, prev->ppms)) - return (curr->ppms > prev->ppms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->ppms > prev->ppms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; if (IS_SIGNIFICANT_DIFF(curr->epms, prev->epms)) - return (curr->epms < prev->epms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->epms < prev->epms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; - return MLX5E_AM_STATS_SAME; + return NET_DIM_STATS_SAME; }
Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
On 01/08/2018 10:13 PM, Andy Gospodarek wrote: From: Andy Gospodarek Change all appropriate mlx5_am* and MLX5_AM* references to net_dim and NET_DIM, respectively, in code that handles dynamic interrupt moderation. Also change all references from 'am' to 'dim' when used as local variables and add generic profile references. Signed-off-by: Andy Gospodarek Acked-by: Tal Gilboa Acked-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/en.h | 9 +- drivers/net/ethernet/mellanox/mlx5/core/en_dim.c | 14 +- .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 +- drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 40 ++- drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 8 +- drivers/net/ethernet/mellanox/mlx5/core/net_dim.c | 286 ++--- drivers/net/ethernet/mellanox/mlx5/core/net_dim.h | 63 ++--- 7 files changed, 225 insertions(+), 201 deletions(-) [...] #define IS_SIGNIFICANT_DIFF(val, ref) \ (((100 * abs((val) - (ref))) / (ref)) > 10) /* more than 10% difference */ -static int mlx5e_am_stats_compare(struct mlx5e_rx_am_stats *curr, - struct mlx5e_rx_am_stats *prev) +static int net_dim_stats_compare(struct net_dim_stats *curr, +struct net_dim_stats *prev) { if (!prev->bpms) - return curr->bpms ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_SAME; + return curr->bpms ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_SAME; if (IS_SIGNIFICANT_DIFF(curr->bpms, prev->bpms)) - return (curr->bpms > prev->bpms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->bpms > prev->bpms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; Hey Andy, I am currently reviewing a patch internally that fixes a bug in this area, prev->ppms can be 0 and could cause IS_SIGNIFICANT_DIFF ouch ! same goes for prev->eppm, for some reason we had a broken assumption that if ppms is 0 for some reason then the bpms is 0 and the above condition will cover us. Anyway the patch will go to net, which means when this series gets accepted then net-next will fail to merge with net and we need to manually push the fix to the new DIM library. But for now I don't think anything is required for this series other than bringing this division by 0 issue and the future merge conflict to your attention. if (IS_SIGNIFICANT_DIFF(curr->ppms, prev->ppms)) - return (curr->ppms > prev->ppms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->ppms > prev->ppms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; if (IS_SIGNIFICANT_DIFF(curr->epms, prev->epms)) - return (curr->epms < prev->epms) ? MLX5E_AM_STATS_BETTER : - MLX5E_AM_STATS_WORSE; + return (curr->epms < prev->epms) ? NET_DIM_STATS_BETTER : + NET_DIM_STATS_WORSE; - return MLX5E_AM_STATS_SAME; + return NET_DIM_STATS_SAME; }
[PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code
From: Andy Gospodarek Change all appropriate mlx5_am* and MLX5_AM* references to net_dim and NET_DIM, respectively, in code that handles dynamic interrupt moderation. Also change all references from 'am' to 'dim' when used as local variables and add generic profile references. Signed-off-by: Andy Gospodarek Acked-by: Tal Gilboa Acked-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/en.h | 9 +- drivers/net/ethernet/mellanox/mlx5/core/en_dim.c | 14 +- .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 +- drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 40 ++- drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 8 +- drivers/net/ethernet/mellanox/mlx5/core/net_dim.c | 286 ++--- drivers/net/ethernet/mellanox/mlx5/core/net_dim.h | 63 ++--- 7 files changed, 225 insertions(+), 201 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h index 4ee06e7..4d1d298 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h @@ -238,8 +238,8 @@ struct mlx5e_params { u16 num_channels; u8 num_tc; bool rx_cqe_compress_def; - struct mlx5e_cq_moder rx_cq_moderation; - struct mlx5e_cq_moder tx_cq_moderation; + struct net_dim_cq_moder rx_cq_moderation; + struct net_dim_cq_moder tx_cq_moderation; bool lro_en; u32 lro_wqe_sz; u16 tx_max_inline; @@ -249,7 +249,7 @@ struct mlx5e_params { u32 indirection_rqt[MLX5E_INDIR_RQT_SIZE]; bool vlan_strip_disable; bool scatter_fcs_en; - bool rx_am_enabled; + bool rx_dim_enabled; u32 lro_timeout; u32 pflags; struct bpf_prog *xdp_prog; @@ -528,7 +528,7 @@ struct mlx5e_rq { unsigned long state; intix; - struct mlx5e_rx_am am; /* Adaptive Moderation */ + struct net_dim dim; /* Dynamic Interrupt Moderation */ /* XDP */ struct bpf_prog *xdp_prog; @@ -1079,4 +1079,5 @@ void mlx5e_build_nic_params(struct mlx5_core_dev *mdev, struct mlx5e_params *params, u16 max_channels); u8 mlx5e_params_calculate_tx_min_inline(struct mlx5_core_dev *mdev); +void mlx5e_rx_dim_work(struct work_struct *work); #endif /* __MLX5_EN_H__ */ diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c b/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c index b9b434b..f620325 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c @@ -32,17 +32,17 @@ #include "en.h" -void mlx5e_rx_am_work(struct work_struct *work) +void mlx5e_rx_dim_work(struct work_struct *work) { - struct mlx5e_rx_am *am = container_of(work, struct mlx5e_rx_am, - work); - struct mlx5e_rq *rq = container_of(am, struct mlx5e_rq, am); - struct mlx5e_cq_moder cur_profile = mlx5e_am_get_profile(am->mode, - am->profile_ix); + struct net_dim *dim = container_of(work, struct net_dim, + work); + struct mlx5e_rq *rq = container_of(dim, struct mlx5e_rq, dim); + struct net_dim_cq_moder cur_profile = net_dim_get_profile(dim->mode, + dim->profile_ix); mlx5_core_modify_cq_moderation(rq->mdev, &rq->cq.mcq, cur_profile.usec, cur_profile.pkts); - am->state = MLX5E_AM_START_MEASURE; + dim->state = NET_DIM_START_MEASURE; } diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c index 8f05efa..51ae6df 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c @@ -480,7 +480,7 @@ int mlx5e_ethtool_get_coalesce(struct mlx5e_priv *priv, coal->rx_max_coalesced_frames = priv->channels.params.rx_cq_moderation.pkts; coal->tx_coalesce_usecs = priv->channels.params.tx_cq_moderation.usec; coal->tx_max_coalesced_frames = priv->channels.params.tx_cq_moderation.pkts; - coal->use_adaptive_rx_coalesce = priv->channels.params.rx_am_enabled; + coal->use_adaptive_rx_coalesce = priv->channels.params.rx_dim_enabled; return 0; } @@ -534,7 +534,7 @@ int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv, new_channels.params.tx_cq_moderation.pkts = coal->tx_max_coalesced_frames; new_channels.params.rx_cq_moderation.usec = coal->rx_coalesce_usecs; new_channels.params.rx_cq_moderation.pkts = coal->rx_max_coalesced_frames; - new_channels.params.rx_am_enabled = !!coal->use_adaptive_rx_coalesce; + new_channels.params.rx_dim_enabled