Re: [PATCH net-next v3 06/10] net/mlx5e: Change Mellanox references in DIM code

2018-01-09 Thread Andy Gospodarek 
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

2018-01-09 Thread Tal Gilboa 

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

2018-01-09 Thread Andy Gospodarek 
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

2018-01-09 Thread kbuild test robot 
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, 
>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

2018-01-09 Thread kbuild test robot 
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, 
>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

2018-01-08 Thread Saeed Mahameed 



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

2018-01-08 Thread Saeed Mahameed 



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

2018-01-08 Thread Andy Gospodarek 
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, >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 =