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?
IMHO you should stick to the sane code.
As you asked, the PR [1] was updated to use 'likely' to enable moving
forward with this series in both kernel and user.
However, a follow-up performance test should be done to check on some
newer compiler. In case needed, an incremental patch will be sent on top
of this series.
[1] https://github.com/linux-rdma/rdma-core/pull/351
--
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
