On Mon, 2009-06-01 at 11:49 -0700, Vasu Dev wrote:
> On Mon, 2009-06-01 at 11:27 -0700, Robert Love wrote:
> > On Mon, 2009-06-01 at 10:14 -0700, Vasu Dev wrote:

<snip>

> > >  /*
> > > - * fc_em_alloc_xid - returns an xid based on request type
> > > - * @lp : ptr to associated lport
> > > - * @fp : ptr to the assocated frame
> > > - *
> > > - * check the associated fc_fsp_pkt to get scsi command type and
> > > - * command direction to decide from which range this exch id
> > > - * will be allocated from.
> > > - *
> > > - * Returns : 0 or an valid xid
> > > - */
> > > -static u16 fc_em_alloc_xid(struct fc_exch_mgr *mp, const struct fc_frame 
> > > *fp)
> > > -{
> > > - u16 xid, min, max;
> > > - u16 *plast;
> > > - struct fc_exch *ep = NULL;
> > > -
> > > - if (mp->max_read) {
> > > -         if (fc_fcp_is_read(fr_fsp(fp))) {
> > 
> > Doesn't the removal of this code break offloads? Maybe I don't
> > understand. After this patch is applied how does the EM use the offload
> > XID range?
> > 
> 
> No it doesn't break, since this patch doesn't remove call to
> fc_fcp_ddp_setup() and DDP will be still set for a allocated exchange id
> for read  IO within offload xid range by fc_fcp_ddp_setup() =>
> fc_fcp_ddp_setup, the fc_fcp_ddp_setup already checks for offload xid
> range and read type IO.

You may not be causing a panic with this patch, but you're introducing a
performance regression when it isn't necessary.

You're likely to chose an incorrect XID for offload and that won't be
realized until the base driver rejects the
n->netdev_ops->ndo_fcoe_ddp_setup() call.

The intent of this code is to chose an XID within the range of XIDs
supported by the HW. Instead this patch makes it so that we allocate any
XID and then let the HW fail. This would certainly cause a performance
regression for offloaded devices.

I think we can do a better job of this by doing a bunch of setup patches
and then one that switches from the XID range algorithm to the multiple
EM approach.


_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel

Reply via email to