Re: [PATCH V2 08/10] i40iw: Control debug error prints using env variable

Leon Romanovsky
2016-12-15T06:48:07Z

[-- Attachment #1: Type: text/plain, Size: 2422 bytes --]

On Wed, Dec 14, 2016 at 02:21:03PM -0700, Jason Gunthorpe wrote:
> On Wed, Dec 14, 2016 at 07:11:11PM +0200, Leon Romanovsky wrote:
>
> > > > This patch duplicates already existing code in most of providers and libraries
> > > > in rdma-core, while two of our main goals for creating this consolidated
> > > > library were simplification for users and reduce code duplication.
> > > >
> > > > It will be very beneficial if you:
> > > > 1. Use and promote general pr_debug(..), srp_tools has nice piece of code,
> > > > to be general code.
> > >
> > > [Tatyana Nikolova] The debug/error printing macros available in
> > > rdma-core use different mechanisms to report information, for
> > > instance, they set/check one or more variables, or they use a bit
> > > mask to enable debug level. They also print to different outputs:
> > > stderr/stdout, debug files or syslog.
> >
> > At the end, all these prints are for debug. It is hard to see any
> > objections to see output from them in one place.
>
> Yes, let us just use stderr for now for provider debugging. If someone
> wants syslog then that can be a later patch. It makes no sense that
> there are difference here.
>
> > in rdma-core/util/env.h|c
> >
> > #define SET_VAR(type, var, field) \
> > (struct ##name*)env->field = get_env_var(...)
> >
> > void *get_env_vars(enum typ)
> > {
> > void *env;
> > switch(type) {
> > case I40W_ENV:
> > env = malloc(sizeof(struct i40w_env));
> > ....
> > SET_VAR(i40w_env, "I40W_DEBUG", debug);
> > ...
> > }
>
> Why?

It will give common place for all different variables and debug was an
example. My request from Tatyana was to come with 2 common mechanisms:
1. Common debug prints.
2. Common place for all various getenv() calls.

And the pseudo-code above was example of second mechsnism.

>
> I was thinking more like a standard:
>
> VERBS_PROVIDER_DEBUG=qp,ah,blah
>
> parser since other than mlx5 that is what providers use env vars for.

It is not providers only, but many other libraries in rdma-core are
using env variables for tuning (libibverbs/librdmacm/rdma-ndd).

>
> I'm not sure I agree at all with what mlx5 is doing with tuning
> parameters via env vars :\
>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]