On Mon, 2009-06-01 at 12:10 -0700, Robert Love wrote:
> 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 fc_fcp_ddp_setup is called unconditionally in current code prior to
this patch for any xid and its calling is unchanged by this patch so
what you described is applicable prior to this patch also. This patch
removes separate offload xid and it function fc_em_alloc_xid to allocate
xid from two ranges in fast path, so that may offset your concern of
less likely offload xid chosen once offload xid range is removed by this
patch by have less checks.
Anyway performance should be viewed by overall patch series and that
should be improved at end of this patch series, since
1. less checks on fast patch by having single range for EM
and no more fc_em_alloc_xid on fast path.
2. the fc_fcp_ddp_setup is called only for offload xid at
end of this patch series.
So series is doing improvement for performance in setting up DDP.
Vasu
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
