Re: [PATCH] opensm: Add per module logging support
On 6/4/2012 8:28 PM, Jim Foraker wrote:
On Wed, 2012-05-30 at 14:30 -0700, Hal Rosenstock wrote:
Add _v2 APIs for osm_log, osm_log_is_active, and osm_log_msg_box
Also, add these to libopensm.map
Add _V2 forms of the various OSM_LOG macros
Why are the _V2 macros necessary? It appears that the calling
semantics/effect are the same, so why create new macros instead of
reusing the old ones? In particular, it seems like you could move the
FILE_ID defines above the #includes, and then in osm_log.h say things
like:
#ifdef FILE_ID
#define OSM_LOG_ENTER( OSM_LOG_PTR ) \
osm_log_v2( OSM_LOG_PTR, OSM_LOG_FUNCS, FILE_ID, \
%s: [\n, __func__);
#else
#define OSM_LOG_ENTER( OSM_LOG_PTR ) \
osm_log( OSM_LOG_PTR, OSM_LOG_FUNCS, \
%s: [\n, __func__);
#endif
Good idea; I'll rework the patch.
Or just define FILE_ID everywhere (it seems to be missing from many
of the libvendor files)
Most of libvendor files are historical and on verge of deprecation so it
wasn't worth the effort there.
and make OSM_LOG* always call the v2 functions.
It seems like this would drastically reduce the patch's footprint.
If by footprint, you mean changed lines of code (rather than object code
size), then yes, this makes sense.
All of above pass additional parameter to identify module.
Each module defines a unique FILE_ID from 0-254.
255 is reserved to indicate name not found.
FILE_ID must match index in osm_subnet.c:module_name_str
table. Note also currently that subnet's
per_mod_log_tbl is 128 entries (which is enough
more than the number of modules).
Why are there so many magic numbers here?
There are 2 magic numbers. One is to use 255 as not found rather than
explicit status. 128 is to limit the array size but that can be
eliminated at the cost of the array size being larger.
It seems like
per_mod_log_tbl's size could be defined in terms of the final value in
osm_file_ids_enum (ie, you could add a OSM_FILE_LAST_ENTRY to the enum).
If find_module_name() is changed to return an int (which is what
osm_log_v2() already uses as file_id's type, so consistency would be
improved), it could return -1 on failure, making the limit at 255
irrelevant as well.
Yes, that is an alternative to using 255 as indicating this.
Also, how is this expected to work with plugins? Will plugins be
expected to use their own logging?
Plugins still use the original/coarse logging or their own logging. In
order to use the per module logging, they'd need to reserve file IDs.
Would it be desirable to put FILE_ID (or better yet, module name)
into the log messages as well? This would make it a bit easier to find
where the relevant code is. We could then phase out the ERR XXYY
scheme, particularly since the XX numbers do not seem to correlate at
all with FILE_ID.
If we decide to change the ERR logs to include name all we need is __FILE__.
If we want to keep current ERR XXYY scheme, for modules with FILE_ID
that could be used as the XX in ERR XXYY. The issues with doing this are
with original logging uses where that's not the case and any conflict
between FILE_ID as the XX in the ERR and the current ERR XXs used now.
Anyhow, this seems like a follow on patch and needs more discussion/thought.
-- Hal
Jim
Use version 2 API (osm_log_v2) and macros (OSM_LOG_V2_)
in OpenSM client code
Added options to enable/disable per module logging and
it's configuration file
Config file format
Set of lines with module name and logging level as follows:
module nameseparatorlogging level
where:
module name is file name including .c
separator is either = , space, or tab
logging level is the same levels as used in the coarse/overall logging
as follows:
BITLOG LEVEL ENABLED
-
0x01 - ERROR (error messages)
0x02 - INFO (basic messages, low volume)
0x04 - VERBOSE (interesting stuff, moderate volume)
0x08 - DEBUG (diagnostic, high volume)
0x10 - FUNCS (function entry/exit, very high volume)
0x20 - FRAMES (dumps all SMP and GMP frames)
0x40 - ROUTING (dump FDB routing information)
0x80 - currently unused
Add parsing of the configuration file into table of log levels
indexed by FILE_ID
Added osm_get_log_per_module routine to obtain the configured info
for a supplied module name. This is a new library function.
Signed-off-by: Hal Rosenstock h...@mellanox.com
---
As this patch is too large for email, please find this @
https://www.openfabrics.org/~halr/0001-opensm-Add-per-module-logging-support.patch
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH] opensm: Add per module logging support
On Wed, 2012-05-30 at 14:30 -0700, Hal Rosenstock wrote:
Add _v2 APIs for osm_log, osm_log_is_active, and osm_log_msg_box
Also, add these to libopensm.map
Add _V2 forms of the various OSM_LOG macros
Why are the _V2 macros necessary? It appears that the calling
semantics/effect are the same, so why create new macros instead of
reusing the old ones? In particular, it seems like you could move the
FILE_ID defines above the #includes, and then in osm_log.h say things
like:
#ifdef FILE_ID
#define OSM_LOG_ENTER( OSM_LOG_PTR ) \
osm_log_v2( OSM_LOG_PTR, OSM_LOG_FUNCS, FILE_ID, \
%s: [\n, __func__);
#else
#define OSM_LOG_ENTER( OSM_LOG_PTR ) \
osm_log( OSM_LOG_PTR, OSM_LOG_FUNCS, \
%s: [\n, __func__);
#endif
Or just define FILE_ID everywhere (it seems to be missing from many
of the libvendor files) and make OSM_LOG* always call the v2 functions.
It seems like this would drastically reduce the patch's footprint.
All of above pass additional parameter to identify module.
Each module defines a unique FILE_ID from 0-254.
255 is reserved to indicate name not found.
FILE_ID must match index in osm_subnet.c:module_name_str
table. Note also currently that subnet's
per_mod_log_tbl is 128 entries (which is enough
more than the number of modules).
Why are there so many magic numbers here? It seems like
per_mod_log_tbl's size could be defined in terms of the final value in
osm_file_ids_enum (ie, you could add a OSM_FILE_LAST_ENTRY to the enum).
If find_module_name() is changed to return an int (which is what
osm_log_v2() already uses as file_id's type, so consistency would be
improved), it could return -1 on failure, making the limit at 255
irrelevant as well.
Also, how is this expected to work with plugins? Will plugins be
expected to use their own logging?
Would it be desirable to put FILE_ID (or better yet, module name)
into the log messages as well? This would make it a bit easier to find
where the relevant code is. We could then phase out the ERR XXYY
scheme, particularly since the XX numbers do not seem to correlate at
all with FILE_ID.
Jim
Use version 2 API (osm_log_v2) and macros (OSM_LOG_V2_)
in OpenSM client code
Added options to enable/disable per module logging and
it's configuration file
Config file format
Set of lines with module name and logging level as follows:
module nameseparatorlogging level
where:
module name is file name including .c
separator is either = , space, or tab
logging level is the same levels as used in the coarse/overall logging
as follows:
BITLOG LEVEL ENABLED
-
0x01 - ERROR (error messages)
0x02 - INFO (basic messages, low volume)
0x04 - VERBOSE (interesting stuff, moderate volume)
0x08 - DEBUG (diagnostic, high volume)
0x10 - FUNCS (function entry/exit, very high volume)
0x20 - FRAMES (dumps all SMP and GMP frames)
0x40 - ROUTING (dump FDB routing information)
0x80 - currently unused
Add parsing of the configuration file into table of log levels
indexed by FILE_ID
Added osm_get_log_per_module routine to obtain the configured info
for a supplied module name. This is a new library function.
Signed-off-by: Hal Rosenstock h...@mellanox.com
---
As this patch is too large for email, please find this @
https://www.openfabrics.org/~halr/0001-opensm-Add-per-module-logging-support.patch
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] opensm: Add per module logging support
On Wed, 30 May 2012 17:30:50 -0400
Hal Rosenstock h...@dev.mellanox.co.il wrote:
Add _v2 APIs for osm_log, osm_log_is_active, and osm_log_msg_box
Also, add these to libopensm.map
Add _V2 forms of the various OSM_LOG macros
All of above pass additional parameter to identify module.
Each module defines a unique FILE_ID from 0-254.
255 is reserved to indicate name not found.
FILE_ID must match index in osm_subnet.c:module_name_str
table. Note also currently that subnet's
per_mod_log_tbl is 128 entries (which is enough
more than the number of modules).
This is a great idea for debugging but do you really need to define the
FILE_ID's? Could you use the __FILE__ macro to do a lookup? Or do you think
that would affect performance?
Also do you really think that 255 files is enough id's to reserve?
Ira
Use version 2 API (osm_log_v2) and macros (OSM_LOG_V2_)
in OpenSM client code
Added options to enable/disable per module logging and
it's configuration file
Config file format
Set of lines with module name and logging level as follows:
module nameseparatorlogging level
where:
module name is file name including .c
separator is either = , space, or tab
logging level is the same levels as used in the coarse/overall logging
as follows:
BITLOG LEVEL ENABLED
-
0x01 - ERROR (error messages)
0x02 - INFO (basic messages, low volume)
0x04 - VERBOSE (interesting stuff, moderate volume)
0x08 - DEBUG (diagnostic, high volume)
0x10 - FUNCS (function entry/exit, very high volume)
0x20 - FRAMES (dumps all SMP and GMP frames)
0x40 - ROUTING (dump FDB routing information)
0x80 - currently unused
Add parsing of the configuration file into table of log levels
indexed by FILE_ID
Added osm_get_log_per_module routine to obtain the configured info
for a supplied module name. This is a new library function.
Signed-off-by: Hal Rosenstock h...@mellanox.com
---
As this patch is too large for email, please find this @
https://www.openfabrics.org/~halr/0001-opensm-Add-per-module-logging-support.patch
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ira Weiny
Member of Technical Staff
Lawrence Livermore National Lab
925-423-8008
wei...@llnl.gov
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] opensm: Add per module logging support
On 5/30/2012 7:03 PM, Ira Weiny wrote:
On Wed, 30 May 2012 17:30:50 -0400
Hal Rosenstock h...@dev.mellanox.co.il wrote:
Add _v2 APIs for osm_log, osm_log_is_active, and osm_log_msg_box
Also, add these to libopensm.map
Add _V2 forms of the various OSM_LOG macros
All of above pass additional parameter to identify module.
Each module defines a unique FILE_ID from 0-254.
255 is reserved to indicate name not found.
FILE_ID must match index in osm_subnet.c:module_name_str
table. Note also currently that subnet's
per_mod_log_tbl is 128 entries (which is enough
more than the number of modules).
This is a great idea for debugging but do you really need to define the
FILE_ID's? Could you use the __FILE__ macro to do a lookup? Or do you
think that would affect performance?
That was the first implementation and the performance impact was too much.
Also do you really think that 255 files is enough id's to reserve?
Yes but it's hard to predict the future. We're only at 88 now. We've
only added a few per year and this leaves 160 more.
-- Hal
Ira
Use version 2 API (osm_log_v2) and macros (OSM_LOG_V2_)
in OpenSM client code
Added options to enable/disable per module logging and
it's configuration file
Config file format
Set of lines with module name and logging level as follows:
module nameseparatorlogging level
where:
module name is file name including .c
separator is either = , space, or tab
logging level is the same levels as used in the coarse/overall logging
as follows:
BITLOG LEVEL ENABLED
-
0x01 - ERROR (error messages)
0x02 - INFO (basic messages, low volume)
0x04 - VERBOSE (interesting stuff, moderate volume)
0x08 - DEBUG (diagnostic, high volume)
0x10 - FUNCS (function entry/exit, very high volume)
0x20 - FRAMES (dumps all SMP and GMP frames)
0x40 - ROUTING (dump FDB routing information)
0x80 - currently unused
Add parsing of the configuration file into table of log levels
indexed by FILE_ID
Added osm_get_log_per_module routine to obtain the configured info
for a supplied module name. This is a new library function.
Signed-off-by: Hal Rosenstock h...@mellanox.com
---
As this patch is too large for email, please find this @
https://www.openfabrics.org/~halr/0001-opensm-Add-per-module-logging-support.patch
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
