[openib-general] [RFC] MAD API changes to fix DMA mapping issues

[Sean Hefty]

Roland Dreier wrote:
>       >  struct ib_mad_send_buf {
>       > +	struct ib_mad_send_buf	*next;
>       >  	struct ib_mad		*mad;
>       > -	DECLARE_PCI_UNMAP_ADDR(mapping)
> 
> Do we want to get rid of this field?  It seems like we'll need to keep
> track of the DMA mapping somewhere, and this is as good a place as any.

The implementation merges struct ib_mad_send_buf with struct 
ib_mad_send_wr_private, which was already used internally to track sends. 
Mapping has been moved into ib_mad_send_wr_private:

@@ -118,9 +118,10 @@ struct ib_mad_send_wr_private {
  	struct ib_mad_list_head mad_list;
  	struct list_head agent_list;
  	struct ib_mad_agent_private *mad_agent_priv;
+	struct ib_mad_send_buf send_buf;
+	DECLARE_PCI_UNMAP_ADDR(mapping)
  	struct ib_send_wr send_wr;
  	struct ib_sge sg_list[IB_MAD_SEND_REQ_MAX_SG];
-	u64 wr_id;			/* client WR ID */
  	__be64 tid;
  	unsigned long timeout;
  	int retries;


>       >  	struct ib_mad_agent	*mad_agent;
>       > +	struct ib_ah		*ah;
>       >  	void			*context[2];
> 
> I know this isn't being changed, but what was the original reason for
> needing two context slots?

I usually find that 2 context fields are convenient for middleware.  In this 
case, the CM uses one context to reference the cm_id associated with a message, 
and the second context records the state that the message was sent in.

>       > -	struct ib_send_wr	send_wr;
>       > -	struct ib_sge		sge;
>       > +	int			timeout_ms;
>       > +	int			retries;
>       >  };
> 
> Finally, I don't see anywhere that the length of the data buffer is
> storred in the structure, so how does ib_post_send_mad() know how much
> to send when it gets called?

The length is stored in send_wr in ib_mad_send_wr_private.  ib_post_send_mad() 
was copying the work request passed in by the user to the one stored in 
mad_send_wr_private  If it makes more sense, I can move the work request from 
mad_send_wr_private to mad_send_buf.

- Sean