[ofw] [RFC] [PATCH 3/3] port of libibumad to windows

[Hal Rosenstock]

On Wed, 2008-07-02 at 09:29 -0700, Fab Tillier wrote:
> > __declspec(dllexport)
> > int     umad_set_grh_net(void *umad, void *mad_addr)
> 
> <snip...>
> 
> > __declspec(dllexport)
> > int umad_set_grh(void *umad, void *mad_addr)
> 
> The two functions above are identical.  Are they supposed to be?  And if so, why two?

One is host ordered and the other net ordered. The GRH stuff is half
baked right now and I think there is some chance for API changes there
once routing gets further. This has only been used in a very limited
context so far.

-- Hal

>   Also note your indentation...
> 
> > __declspec(dllexport)
> > int umad_recv(int portid, void *umad, int *length, int timeout_ms)
> > {
> >         WM_MAD          *mad = (WM_MAD *) umad;
> >         um_port_t       *port;
> >         HRESULT         hr;
> >
> >         port = &ports[portid];
> >         hr = port->prov->Receive(mad, (size_t) length, &port->overlap);
> >
> >         if (hr == ERROR_IO_PENDING) {
> 
> Note that ERROR_IO_PENDING isn't an HRESULT.  Either E_PENDING or HRESULT_FROM_WIN32( ERROR_IO_PENDING ).
> 
> >                 if (port->pending && timeout_ms == 0) {
> 
> How does a client call to check if anything's done without actually waiting?  I would have expected 0 to be that, but it seems that 0 means infinite (I would have picked -1 for infinite.)
> 
> >                         do {
> >                                 hr = WaitForSingleObject(port-
> > >overlap.hEvent, 250);
> 
> WaitForSingleObject doesn't return an HRESULT.
> 
> >                         } while (hr == WAIT_TIMEOUT && port->pending);
> 
> Just wait on the event.  If the client deregisters from a different thread, the overlapped request should get cancelled which will set the event.
> 
> >                 } else {
> >                         hr = WaitForSingleObject(port->overlap.hEvent,
> > (DWORD) timeout_ms);
> >                         if (hr == WAIT_TIMEOUT) {
> >                                 return -EWOULDBLOCK;
> >                         }
> >                 }
> >         }
> >
> >         if (FAILED(hr)) {
> >                 return -EIO;
> >         }
> >
> >         if (mad->Length <= (UINT32) *length) {
> >                 port->pending = FALSE;
> >         }
> >
> >         *length = mad->Length;
> >         return 0;
> > }
> >
> > __declspec(dllexport)
> > int umad_poll(int portid, int timeout_ms)
> > {
> >         WM_MAD          mad;
> >         um_port_t       *port;
> >         HRESULT         hr;
> >
> >         port = &ports[portid];
> >         hr = port->prov->Receive(&mad, sizeof mad, &port->overlap);
> 
> What happens to the received WM_MAD?  How does a subsequent umad_recv get the data?
> 
> Also, you don't handle a call to umad_poll on one thread at the same time as a call to umad_recv on another.  As it stands, you'll have two requests using the same overlapped structure.
> 
> >
> >         if (hr == ERROR_IO_PENDING) {
> >                 hr = WaitForSingleObject(port->overlap.hEvent, (DWORD)
> > timeout_ms);
> >                 if (hr == WAIT_TIMEOUT) {
> >                         return -ETIMEDOUT;
> >                 }
> >         }
> >
> >         if (FAILED(hr)) {
> >                 return -EIO;
> >         }
> >
> >         port->pending = TRUE;
> >         return 0;
> > }
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw