Commit dc9bc118 authored by Ben Walker's avatar Ben Walker Committed by Tomasz Zawadzki
Browse files

accel: Fix copy elision before a CRC 



CRC operations do not move data themselves, so if the user
put a copy operation into the sequence followed by a CRC, that copy
cannot be removed or the data never gets copied to the destination.

Fixes #3382

Signed-off-by: default avatarBen Walker <ben@nvidia.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/23206

 (master)

(cherry picked from commit 902e6946)
Change-Id: Ic29b99c37ff8ccb2af826184646b1389c995b50f
Signed-off-by: default avatarMarek Chomnicki <marek.chomnicki@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/23220


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
parent 2d613454

lib/accel/accel.c

+1 −1
Original line number Diff line number Diff line
@@ -2049,7 +2049,7 @@ accel_sequence_merge_tasks(struct spdk_accel_sequence *seq, struct spdk_accel_ta 
		    next->op_code != SPDK_ACCEL_OPC_COPY &&

		    next->op_code != SPDK_ACCEL_OPC_ENCRYPT &&

		    next->op_code != SPDK_ACCEL_OPC_DECRYPT &&

		    next->op_code != SPDK_ACCEL_OPC_CRC32C) {

		    next->op_code != SPDK_ACCEL_OPC_COPY_CRC32C) {

			break;

		}

		if (task->dst_domain != next->src_domain) {

test/unit/lib/accel/accel.c/accel_ut.c

+6 −5
Original line number Diff line number Diff line
@@ -3915,8 +3915,8 @@ test_sequence_crc32(void) 
	CU_ASSERT_EQUAL(crc, spdk_crc32c_update(buf, sizeof(buf), ~0u));

	g_seq_operations[SPDK_ACCEL_OPC_CRC32C].count = 0;



	/* Now check copy+crc - this should remove the copy operation and change the buffer for the

	 * crc operation */

	/* Now check copy+crc - This should not remove the copy. Otherwise the data does not

	 * end up where the user expected it to be. */

	seq = NULL;

	completed = 0;

	crc = 0;
@@ -3946,12 +3946,13 @@ test_sequence_crc32(void) 
	CU_ASSERT(ut_seq.complete);

	CU_ASSERT_EQUAL(ut_seq.status, 0);

	CU_ASSERT_EQUAL(g_seq_operations[SPDK_ACCEL_OPC_CRC32C].count, 1);

	CU_ASSERT_EQUAL(g_seq_operations[SPDK_ACCEL_OPC_COPY].count, 0);

	CU_ASSERT_EQUAL(g_seq_operations[SPDK_ACCEL_OPC_COPY].count, 1);

	CU_ASSERT_EQUAL(memcmp(buf, tmp[0], sizeof(buf)), 0);

	CU_ASSERT_EQUAL(crc, spdk_crc32c_update(buf, sizeof(buf), ~0u));

	g_seq_operations[SPDK_ACCEL_OPC_CRC32C].count = 0;

	g_seq_operations[SPDK_ACCEL_OPC_COPY].count = 0;



	/* Check crc+copy - this time the copy cannot be removed, because there's no operation

	 * before crc to change the buffer */

	/* Check crc+copy - Again, the copy cannot be removed. */

	seq = NULL;

	completed = 0;

	crc = 0;