Re: [PATCH rdma-core 5/5] mlx5: Add support for ibv_alloc_null_mr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 24, 2018 at 09:47:21AM +0300, Yonatan Cohen wrote:
> On 6/21/2018 5:13 PM, Jason Gunthorpe wrote:
> > On Thu, Jun 21, 2018 at 04:53:54PM +0300, Yishai Hadas wrote:
> > > On 6/20/2018 10:35 PM, Jason Gunthorpe wrote:
> > > > On Wed, Jun 20, 2018 at 07:28:22PM +0300, Yishai Hadas wrote:
> > > > > @@ -80,7 +80,16 @@ static int copy_to_scat(struct mlx5_wqe_data_seg *scat, void *buf, int *size,
> > > > >   	for (i = 0; i < max; ++i) {
> > > > >   		copy = min_t(long, *size, be32toh(scat->byte_count));
> > > > > -		memcpy((void *)(unsigned long)be64toh(scat->addr), buf, copy);
> > > > > +
> > > > > +		/* When NULL MR is used can't copy to target,
> > > > > +		 * expected to be NULL.
> > > > > +		 */
> > > > > +		if (unlikely(scat->lkey == ctx->dump_fill_mkey_be))
> > > > > +			;
> > > > > +		else
> > > >
> > > > What is that? Better as:
> > > >
> > > > if (likely(scat->lkey != ctx->dump_fill_mkey_be))
> > > > 			memcpy((void *)(unsigned long)be64toh(scat->addr),
> > > > 			       buf, copy);
> > > >
> > >
> > > This is data path flow, upon performance testing we found the unlikely
> > > variant to be better than this suggestion.
> >
> > That is a highly compiler specific statement. I assume you tested this
> > with some ancient compiler?
> gcc --version
> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)

This gcc version is so ancient, so don't do any optimizations decisions based on it.

> >
> > IMHO you should stick to the sane code.
> >
> > Jason
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux