| Message ID | 1446858426-30509-11-git-send-email-jubin.john@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Fri, Nov 06, 2015 at 08:07:05PM -0500, Jubin John wrote: > From: Ira Weiny <iweiny@gmail.com> > > sdma_select_engine_vl only needs to protect itself from an invalid VL. > Something higher up the stack should be warning the user when they try > to use an SL which maps to an invalid VL. > > Reviewed-by: Dean Luick <dean.luick@intel.com> > Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com> > Reviewed-by: Kaike Wan <kaike.wan@intel.com> > Signed-off-by: Ira Weiny <iweiny@gmail.com> > Signed-off-by: Jubin John <jubin.john@intel.com> > --- > drivers/staging/rdma/hfi1/sdma.c | 12 ++++++++++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/rdma/hfi1/sdma.c b/drivers/staging/rdma/hfi1/sdma.c > index 2a1da21..998bf43 100644 > --- a/drivers/staging/rdma/hfi1/sdma.c > +++ b/drivers/staging/rdma/hfi1/sdma.c > @@ -777,8 +777,14 @@ struct sdma_engine *sdma_select_engine_vl( > struct sdma_map_elem *e; > struct sdma_engine *rval; > > - if (WARN_ON(vl > 8)) > - return NULL; > + /* NOTE This should only happen if SC->VL changed after the initial > + * checks on the QP/AH > + * Default will return engine 0 below > + */ > + if (unlikely(vl >= num_vls)) { Can you prove that unlikely() makes a measured difference here? If not, please remove it. If you can, please provide the proof in the changelog when you resend it. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > - if (WARN_ON(vl > 8)) > > - return NULL; > > + /* NOTE This should only happen if SC->VL changed after the initial > > + * checks on the QP/AH > > + * Default will return engine 0 below > > + */ > > + if (unlikely(vl >= num_vls)) { > > Can you prove that unlikely() makes a measured difference here? If not, > please remove it. If you can, please provide the proof in the changelog > when you resend it. Unfortunately, I can't prove this so will remove in v2. Thanks, Jubin John -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/staging/rdma/hfi1/sdma.c b/drivers/staging/rdma/hfi1/sdma.c index 2a1da21..998bf43 100644 --- a/drivers/staging/rdma/hfi1/sdma.c +++ b/drivers/staging/rdma/hfi1/sdma.c @@ -777,8 +777,14 @@ struct sdma_engine *sdma_select_engine_vl( struct sdma_map_elem *e; struct sdma_engine *rval; - if (WARN_ON(vl > 8)) - return NULL; + /* NOTE This should only happen if SC->VL changed after the initial + * checks on the QP/AH + * Default will return engine 0 below + */ + if (unlikely(vl >= num_vls)) { + rval = NULL; + goto done; + } rcu_read_lock(); m = rcu_dereference(dd->sdma_map); @@ -790,6 +796,8 @@ struct sdma_engine *sdma_select_engine_vl( rval = e->sde[selector & e->mask]; rcu_read_unlock(); +done: + rval = !rval ? &dd->per_sdma[0] : rval; trace_hfi1_sdma_engine_select(dd, selector, vl, rval->this_idx); return rval; }