On 7/21/2021 10:05 AM, Gal Pressman wrote:

On 20/07/2021 17:57, Yishai Hadas wrote:

quoted

On 7/20/2021 3:27 PM, Leon Romanovsky wrote:

quoted

On Tue, Jul 20, 2021 at 12:27:46PM +0300, Yishai Hadas wrote:

quoted

On 7/20/2021 11:51 AM, Leon Romanovsky wrote:

quoted

On Tue, Jul 20, 2021 at 11:16:23AM +0300, Yishai Hadas wrote:

quoted

From: Maor Gottlieb <redacted>

Usage will be in next patches.

Signed-off-by: Maor Gottlieb <redacted>
Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
---
    providers/mlx5/mlx5.c | 28 ++++++++++++++--------------
    providers/mlx5/mlx5.h |  4 ++++
    2 files changed, 18 insertions(+), 14 deletions(-)

Probably, this patch will be needed to be changed after
"Verbs logging API" PR https://github.com/linux-rdma/rdma-core/pull/1030

Thanks

Well, not really, this patch just reorganizes things inside mlx5 for code
sharing.

After Gal's PR, I expect to see all mlx5 file/debug logic gone except
some minimal conversion logic to support old legacy variables.

All that get_env_... code will go.

Thanks

Looking on current VERBs logging PR, it doesn't give a clean path conversion
from mlx5.

For example it missed a debug granularity per object (e.g. QP, CQ, etc.) , in
addition it doesn't define a driver specific options as we have today in mlx5
(e.g. MLX5_DBG_DR).

I believe that this should be added before going forward with the logging PR to
enable a clean transition later on.

The transition of mlx5 should preserve current API/semantics (ENV, etc.) and is
an orthogonal task.

Yishai, if you have any more concerns please share them in the PR.. The
discussion there is going on for a while and you've been quiet so I assumed
you're fine with it.

I disagree about needing to support everything that exists in mlx5 today, the
purpose of the generic mechanism is to unify the environment variables, driver
specific options do the opposite. IMO masking out a few prints isn't worth the
divergence.

The options in mlx5 gives more granularity and supports vendor specific 
options, this may be needed down the road by other vendors as well.

If we come with a new API need to consider such options in the general case.

NP, I'll comment in the logging PR as well.

If the mlx5 provider has backwards compatibility issues it doesn't necessarily
have to use this API, but we can at least covert most existing providers and all
future providers that wish to support such functionality in a unified way.

The point was that current suggestion doesn't allow a clean transition 
for mlx5, so we won't use it unless the API will give a matching 
alternative.

BTW, why even insist on having backwards compatibility here? Do you actually
have useful code that relies on debug environment variables?

Logging options (env, mask, etc.) are kind of API which need to be 
preserved, it's used in the field for debug purposes, no reason to loose 
granularity and trace.

Yishai