Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] openib btl code review
From: Gleb Natapov (glebn_at_[hidden])
Date: 2008-05-18 09:46:14


On Thu, May 15, 2008 at 11:58:02PM -0400, Jeff Squyres wrote:
> I updated the patch on https://svn.open-mpi.org/trac/ompi/ticket/1285
> per Gleb's suggestions (I made a few commits tonight with some of the
> non-receive-queues-patch-related fixes) and with some fixes for issues
> that Nysal found.
>
> Please see the most recent patch on the ticket.
Looks good to me.

>
>
>
> On May 15, 2008, at 11:01 AM, Jeff Squyres wrote:
>
> > On May 15, 2008, at 8:46 AM, Gleb Natapov wrote:
> >
> >>> Any other reviewers would be welcome... :-)
> >> I'll look at it next week too.
> >
> > Thanks.
> >
> >>>> - some random style cleanup
> >>>> - fix a few minor memory leaks
> >
> > These two are the only ones that are really separate from the rest.
> >
> >>>> - adapt _ini.c to accept the "receive_queues" field in the file
> >>>> - move 90% of _setup_qps() from _ini.c to _component.c
> >>>> - move what was left of _setup_qps() into the main
> >>>> _register_mca_params() function
> >>>> - adapt init_one_hca() to detect conflicting receive_queues values
> >>>> from the INI file
> >>>> - after the _component.c loop calling init_one_hca():
> >>>> - call setup_qps() to parse the final receive_queues string value
> >>>> - traverse all resulting btls and initialize their HCAs (if they
> >>>> weren't already): setup some lists and call prepare_hca_for_use()
> >>>>
> >> It is better to have separate patch (and commit) for each of these
> >> items.
> >> Doing review and dialing with bugs is much easier this way.
> >
> >
> > I'll separate out the first two into separate fixes; I can even commit
> > those because they're pretty harmless and small. FWIW: all of the
> > style changes were because I tried several approaches for the
> > receive_queues stuff before I found one that worked (i.e., I adapted
> > style of code that I touched, but then ended up reverting everything
> > except the style changes).
> >
> > --
> > Jeff Squyres
> > Cisco Systems
> >
> > _______________________________________________
> > devel mailing list
> > devel_at_[hidden]
> > http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
>
> --
> Jeff Squyres
> Cisco Systems
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel

--
			Gleb.