[ofa-general] Multi-threaded diags (Was: Re: [PATCH 4/5] infiniband-diags/libibnetdisc: Introduce a context object.)

[Ira Weiny]

On Sun, 23 Aug 2009 15:06:09 +0300
Sasha Khapyorsky <sashak at voltaire.com> wrote:

> Hi Ira,
> 
> On 08:30 Mon 17 Aug     , Ira Weiny wrote:
> > 
> > The immediate benefit is coming with the multi-threaded implementation where
> > I plan on adding the following function.[*]
> 
> Ok, but could we discuss first how will multithreading architecture be

Of course!  :-)  But first I would like to mention some numbers from the
prototype code I have.  When running on a small fabric the additional overhead
of thread creation actually slows down the scan.  :-(

Current master:         Threaded version:
real    0m0.101s         0m0.266s
user    0m0.000s         0m0.000s
sys     0m0.011s         0m0.014s


But, as expected, on a large system (1152 nodes) there is a decent speed up.

Current Master:         Threaded version:
real    0m3.046s         0m1.748s
user    0m0.073s         0m0.331s
sys     0m0.158s         0m0.822s

However, the biggest speed up comes when there are errors on the fabric.  This
is the same 1152 node cluster with just 14 "bad" ports on the fabric.  This is
of course because the scan continues "around" the bad ports.

Current Master:         Threaded version:
real    0m33.051s        0m5.609s
user    0m0.071s         0m0.353s
sys     0m0.156s         0m1.113s

Since you are usually running these tools when things are bad I think there is
a big gain here.  Even running with a faster timeout of 200ms results in a big
difference.

Current Master:        Threaded version:
real    0m9.149s        0m2.223s
user    0m0.016s        0m0.374s
sys     0m0.372s        0m1.056s

With that in mind...

> implemented with libibnetdisc: goals (in particular is it support for
> multithreaded apps or just multithreaded discovery function), interaction
> with caller application, etc.?

My initial goal was to make the libibnetdisc safe for multithreaded apps and
make a multithreaded discovery function.  However, since libibmad itself is
not thread safe, and you expressed a desire to keep it that way[*], I reduced
that goal to just making the discovery function multithreaded (using
mad_[send|receive]_via).

Although I don't like this restriction I can see it as a valid design decision
as long as it is documented that the discover function is not thread safe in
regards to the ibmad_port object.  This is because the ibnd_discover_fabric
uses libibmad calls and would require a complicated API to allow the user app
to synchronize with those calls.

In order to make things thread safe for the user apps as well as the library I
can see 3 options.

   1) make libibmad thread safe (which you were hesitant to do)

   2) add a thread safe interface to libibmad.  User apps will need to know to
      use this interface while using libibnetdisc and libibnetdisc will use
      this interface.

   3) Create a wrapper lib which is thread safe.  In this case the apps and
      libibnetdisc would call into this wrapper lib and we would have to
      change the API to libibnetdisc.

Right now I have the multithreaded discover code separated out somewhat.  I
think it would not be hard to extract the multithreaded parts and either
create the wrapper lib or extend libibmad with thread safe calls.

That said, I personally do not like option 2.  I think it further complicates
an already overly complex API in libibmad.  As far as option 1 vs 3 I can see
arguments for and against each.  1 makes things very nice because it would be
taken care of for all apps currently using libibmad.  On the down side it
would add some overhead for single threaded apps.  Although I do not believe
too much.[$]

The downside of 3 is that to be done correctly it would change the
libibnetdisc API and apps which use it.

> 
> One of the desired feature of this I could think would be to keep API
> simple for single threaded stuff.

Agreed.  I don't think the API is going to get to complicated.  A big reason
for adding the context is to allow the API to be flexible without breaking
things.

Ira

[*] http://lists.openfabrics.org/pipermail/general/2009-July/060677.html

   "madrpc() is too primitive interface for such applications. There would be
   better to use umad_send/recv() directly or may be mad_send_via().  Example
   is mcast_storm.c distributed with ibsim."

[$] It is my opinion that mad_rpc is _not_ primitive.  In my mind it _is_
   the wrapper around the primitive umad_send/recv calls.  If you are
   interested perhaps I can try to explain what I wanted to do in the library
   to make it thread safe more clearly.  The point I might not have made clear
   was that I don't think the library will have to do any threading on it's
   own, just some locks and storing of responses.  Of course the down side to
   this is the libibmad code would be slightly slower.  But I don't think by
   very much.


-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2 at llnl.gov