Re: [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jul 13, 2020 at 07:15:19AM +0000, Cheng Jiang wrote:
> Added a flag which controls whether rte_ioat_enqueue_copy
> and rte_ioat_completed_copies function should process
> handle parameters to improve the performance when handle
> parameters are not necessary to use. This is targeting
> 20.11 release.
> 
> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> ---
>  drivers/raw/ioat/ioat_rawdev.c     |  1 +
>  drivers/raw/ioat/rte_ioat_rawdev.h | 14 +++++++++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
<snip>
> @@ -208,6 +213,11 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies,
>  	if (count > max_copies)
>  		count = max_copies;
>  
> +	ioat->next_read = read + count;
> +	ioat->completed += count;
> +	if (!ioat->hdls_enable)
> +		return count;
> +
>  	for (; i < count - 1; i += 2, read += 2) {
>  		__m128i hdls0 = _mm_load_si128(&ioat->hdls[read & mask]);
>  		__m128i hdls1 = _mm_load_si128(&ioat->hdls[(read + 1) & mask]);
> @@ -223,8 +233,6 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies,
>  		dst_hdls[i] = hdls[1];
>  	}
>  
> -	ioat->next_read = read;
> -	ioat->completed += count;
>  	return count;
>  }

This change I think may cause problems if we ever want to have one thread
enqueuing and another taking completions. The next_read and completed
counters should really only be updated after we have finished reading the
completed handles array. Therefore, for safety, I tihnk it might be better
to keep the updates in their original places and put an "end:" label before
them. Then the "return count" in the middle of the function can be "goto
end;"

Regards,
/Bruce