On Thu, Nov 01, 2012 at 06:50:30PM -0400, George Bosilca wrote: > > On Nov 1, 2012, at 16:18 , Nathan Hjelm <hje...@lanl.gov> wrote: > > > On Thu, Nov 01, 2012 at 04:07:32PM -0400, George Bosilca wrote: > >> Nathan, > >> > >> Here is a quick question regarding the topi framework. > >> > >> - The mca_topo_base_output is opened unconditionally in topo_base_open.c:62 > >> > >> - with your patch, mca_topo_base_output is closed conditionally in > >> topo_base_close.c:46, but only in case > >> mca_topo_base_components_opened_valid and > >> mca_topo_base_components_available_valid are NULL. However, > >> mca_topo_base_output is set to -1 in all cases right after. > >> > >> Why is that so? > > > > mca_base_components_close closes the output if the third argument is NULL. > > So in this case calling opal_output_close after mca_base_components_close > > will result in a second call to opal_output_close. > > Indeed, the behavior of the mca_base_components_close seems quite weird to > me, as it lack of consistency. > - the symmetric function (mca_components_open) doesn't open the output stream > - the stream is close but the corresponding variable is not set to a > meaningful value (-1)
Exactly. There is more inconsistency between the two calls as well. mca_base_components_open calls OBJ_CONSTRUCT on the component list but mca_base_components_close does not call OBJ_DESTRUCT. Because of this we don't always OBJ_DESTRUCT the component list because some frameworks don't do it. I was going to address this second inconsistency with another patch but now seems like a good time to get a see if anyone has an opinion about how this should be fixed. I can think of two simple fixes: 1) Since mca_base_components_open calls OBJ_CONSTRUCT should mca_base_components_close call OBJ_DESTRUCT? 2) Should the caller be responsible for both the OBJ_CONSTRUCT and OBJ_DESTRUCT calls? > - it force us to have one specific output for each framework This isn't the case at the moment since frameworks can call opal_output_close on any extra output streams. It would be better if frameworks have t close all open output streams using opal_output_close instead of using mca_base_components_close. If we want to change the semantics of mca_base_components_close I can redo this patch. Anyone have an opinion on this? -Nathan Hjelm HPC-3, LANL
