Skip to contentSkip to conversation

dmaengine: ioat: Use common error handling code in ioat_xor_val_self_test()

PATCH In review SF Markus Elfring 4 participants 5 messages Oct 22, 2017 – Oct 23, 2017

Subsystems (2)
Kernel janitors dmaengine
lore
Share & tools
lore

SF Markus Elfring

author
#0 lore raw

PATCH 1/1 dmaengine: ioat: Use common error handling code in ioat_xor_val_self_test()

From: Markus Elfring <[email protected]>
Date: Sun, 22 Oct 2017 17:25:09 +0200

Add a jump target so that a specific error code assignment will be in the implementation mostly at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/dma/ioat/init.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)
drivers/dma/ioat/init.c +12 -20
index 93e006c3441d..5bdd67940c81 100644
@@ -853,8 +853,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 
 	if (!tx) {
 		dev_err(dev, "Self-test xor prep failed\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 
 	async_tx_ack(tx);

@@ -864,8 +863,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 	cookie = tx->tx_submit(tx);
 	if (cookie < 0) {
 		dev_err(dev, "Self-test xor setup failed\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 	dma->device_issue_pending(dma_chan);
 

@@ -874,8 +872,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 	if (tmo = 0 ||
 	    dma->device_tx_status(dma_chan, cookie, NULL) != DMA_COMPLETE) {
 		dev_err(dev, "Self-test xor timed out\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 
 	for (i = 0; i < IOAT_NUM_SRC_TEST; i++)

@@ -921,8 +918,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 					  &xor_val_result, DMA_PREP_INTERRUPT);
 	if (!tx) {
 		dev_err(dev, "Self-test zero prep failed\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 
 	async_tx_ack(tx);

@@ -932,8 +928,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 	cookie = tx->tx_submit(tx);
 	if (cookie < 0) {
 		dev_err(dev, "Self-test zero setup failed\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 	dma->device_issue_pending(dma_chan);
 

@@ -942,8 +937,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 	if (tmo = 0 ||
 	    dma->device_tx_status(dma_chan, cookie, NULL) != DMA_COMPLETE) {
 		dev_err(dev, "Self-test validate timed out\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 
 	for (i = 0; i < IOAT_NUM_SRC_TEST + 1; i++)

@@ -974,8 +968,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 					  &xor_val_result, DMA_PREP_INTERRUPT);
 	if (!tx) {
 		dev_err(dev, "Self-test 2nd zero prep failed\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 
 	async_tx_ack(tx);

@@ -985,8 +978,7 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 	cookie = tx->tx_submit(tx);
 	if (cookie < 0) {
 		dev_err(dev, "Self-test  2nd zero setup failed\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 	dma->device_issue_pending(dma_chan);
 

@@ -995,20 +987,20 @@ static int ioat_xor_val_self_test(struct ioatdma_device *ioat_dma)
 	if (tmo = 0 ||
 	    dma->device_tx_status(dma_chan, cookie, NULL) != DMA_COMPLETE) {
 		dev_err(dev, "Self-test 2nd validate timed out\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 
 	if (xor_val_result != SUM_CHECK_P_RESULT) {
 		dev_err(dev, "Self-test validate failed compare\n");
-		err = -ENODEV;
-		goto dma_unmap;
+		goto failure_indication;
 	}
 
 	for (i = 0; i < IOAT_NUM_SRC_TEST + 1; i++)
 		dma_unmap_page(dev, dma_srcs[i], PAGE_SIZE, DMA_TO_DEVICE);
 
 	goto free_resources;
+failure_indication:
+	err = -ENODEV;
 dma_unmap:
 	if (op = IOAT_OP_XOR) {
 		while (--i >= 0)
Signature · 1 line
2.14.2
#2 lore raw
Quoted 5 lines · Dan Carpenter

These patches hurt readability.

regards,
dan carpenter

I agree with Dan. I'm ok with not accepting this patch.

#3 lore raw
Quoted 8 lines · Dave Jiang
Quoted 5 lines · Dan Carpenter

These patches hurt readability.

regards,
dan carpenter

I agree with Dan. I'm ok with not accepting this patch.

And I have no intention to :)

At least they should explain why or how it help, give the Coccinelle scripts.. But sadly that is not done ...

Signature · 1 line
~Vinod
author
#4 lore raw
Quoted 3 lines
Quoted 1 line

I agree with Dan. I'm ok with not accepting this patch.

And I have no intention to :)

This is a pity.

Quoted 1 line

At least they should explain why or how it help,

Do you see useful consequences (like a bit less memory requirements) if the assignment statement “err = -ENODEV” will be stored only once behind the jump label “failure_indication” in the suggested update for the implementation of the function “ioat_xor_val_self_test” (instead of being duplicated several times)?

Quoted 1 line

give the Coccinelle scripts..

How can they matter for a better understanding of the concrete source code adjustment?

Quoted 1 line

But sadly that is not done ...

I did not include a link for special background information explicitly. But how do you think about corresponding details from discussions on a topic like “Comparing statement lists with SmPL”?

https://systeme.lip6.fr/pipermail/cocci/2017-August/004388.html

Regards,
Markus