On 21/07/2021 10:58, Yishai Hadas wrote: > On 7/21/2021 10:05 AM, Gal Pressman wrote: >> On 20/07/2021 17:57, Yishai Hadas wrote: >>> On 7/20/2021 3:27 PM, Leon Romanovsky wrote: >>>> On Tue, Jul 20, 2021 at 12:27:46PM +0300, Yishai Hadas wrote: >>>>> On 7/20/2021 11:51 AM, Leon Romanovsky wrote: >>>>>> On Tue, Jul 20, 2021 at 11:16:23AM +0300, Yishai Hadas wrote: >>>>>>> From: Maor Gottlieb <maorg@xxxxxxxxxx> >>>>>>> >>>>>>> Usage will be in next patches. >>>>>>> >>>>>>> Signed-off-by: Maor Gottlieb <maorg@xxxxxxxxxx> >>>>>>> Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxx> >>>>>>> --- >>>>>>> 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. It's used in the field by *people*, which unlike legacy code can learn to use the new debug env variables, they don't need backwards compatibility (the same as users of debugfs). I addressed the granularity issue in the PR.
