From: Arnd Bergmann <arnd@arndb.de>
To: Giovanni Cabiddu <giovanni.cabiddu@intel.com>,
Salvatore Benedetto <salvatore.benedetto@intel.com>,
Herbert Xu <herbert@gondor.apana.org.au>,
"David S. Miller" <davem@davemloft.net>
Cc: Arnd Bergmann <arnd@arndb.de>, Christoph Hellwig <hch@lst.de>,
Corentin LABBE <clabbe.montjoie@gmail.com>,
qat-linux@intel.com, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH] crypto: qat - avoid an uninitialized variable warning
Date: Thu, 22 Jun 2017 14:35:46 +0200 [thread overview]
Message-ID: <20170622123604.505337-1-arnd@arndb.de> (raw)
After commit 9e442aa6a753 ("x86: remove DMA_ERROR_CODE"), the inlining
decisions in the qat driver changed slightly, introducing a new false-positive
warning:
drivers/crypto/qat/qat_common/qat_algs.c: In function 'qat_alg_sgl_to_bufl.isra.6':
include/linux/dma-mapping.h:228:2: error: 'sz_out' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/crypto/qat/qat_common/qat_algs.c:676:9: note: 'sz_out' was declared here
The patch that introduced this is correct, so let's just avoid the
warning in this driver by rearranging the unwinding after an error
to make it more obvious to the compiler what is going on.
The problem here is the 'if (unlikely(dma_mapping_error(dev, blp)))'
check, in which the 'unlikely' causes gcc to forget what it knew about
the state of the variables. Cleaning up the dma state in the reverse
order it was created means we can simplify the logic so it doesn't have
to know about that state, and also makes it easier to understand.
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/crypto/qat/qat_common/qat_algs.c | 40 +++++++++++++++++---------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 5b5efcc52cb5..baffae817259 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -686,7 +686,7 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance *inst,
blp = dma_map_single(dev, bufl, sz, DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, blp)))
- goto err;
+ goto err_in;
for_each_sg(sgl, sg, n, i) {
int y = sg_nctr;
@@ -699,7 +699,7 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance *inst,
DMA_BIDIRECTIONAL);
bufl->bufers[y].len = sg->length;
if (unlikely(dma_mapping_error(dev, bufl->bufers[y].addr)))
- goto err;
+ goto err_in;
sg_nctr++;
}
bufl->num_bufs = sg_nctr;
@@ -717,10 +717,10 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance *inst,
buflout = kzalloc_node(sz_out, GFP_ATOMIC,
dev_to_node(&GET_DEV(inst->accel_dev)));
if (unlikely(!buflout))
- goto err;
+ goto err_in;
bloutp = dma_map_single(dev, buflout, sz_out, DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, bloutp)))
- goto err;
+ goto err_out;
bufers = buflout->bufers;
for_each_sg(sglout, sg, n, i) {
int y = sg_nctr;
@@ -732,7 +732,7 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance *inst,
sg->length,
DMA_BIDIRECTIONAL);
if (unlikely(dma_mapping_error(dev, bufers[y].addr)))
- goto err;
+ goto err_out;
bufers[y].len = sg->length;
sg_nctr++;
}
@@ -747,9 +747,20 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance *inst,
qat_req->buf.sz_out = 0;
}
return 0;
-err:
- dev_err(dev, "Failed to map buf for dma\n");
- sg_nctr = 0;
+
+err_out:
+ n = sg_nents(sglout);
+ for (i = 0; i < n; i++)
+ if (!dma_mapping_error(dev, buflout->bufers[i].addr))
+ dma_unmap_single(dev, buflout->bufers[i].addr,
+ buflout->bufers[i].len,
+ DMA_BIDIRECTIONAL);
+ if (!dma_mapping_error(dev, bloutp))
+ dma_unmap_single(dev, bloutp, sz_out, DMA_TO_DEVICE);
+ kfree(buflout);
+
+err_in:
+ n = sg_nents(sgl);
for (i = 0; i < n; i++)
if (!dma_mapping_error(dev, bufl->bufers[i].addr))
dma_unmap_single(dev, bufl->bufers[i].addr,
@@ -759,17 +770,8 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance *inst,
if (!dma_mapping_error(dev, blp))
dma_unmap_single(dev, blp, sz, DMA_TO_DEVICE);
kfree(bufl);
- if (sgl != sglout && buflout) {
- n = sg_nents(sglout);
- for (i = 0; i < n; i++)
- if (!dma_mapping_error(dev, buflout->bufers[i].addr))
- dma_unmap_single(dev, buflout->bufers[i].addr,
- buflout->bufers[i].len,
- DMA_BIDIRECTIONAL);
- if (!dma_mapping_error(dev, bloutp))
- dma_unmap_single(dev, bloutp, sz_out, DMA_TO_DEVICE);
- kfree(buflout);
- }
+
+ dev_err(dev, "Failed to map buf for dma\n");
return -ENOMEM;
}
--
2.9.0
next reply other threads:[~2017-06-22 12:36 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-22 12:35 Arnd Bergmann [this message]
2017-06-26 7:03 ` [PATCH] crypto: qat - avoid an uninitialized variable warning Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170622123604.505337-1-arnd@arndb.de \
--to=arnd@arndb.de \
--cc=clabbe.montjoie@gmail.com \
--cc=davem@davemloft.net \
--cc=giovanni.cabiddu@intel.com \
--cc=hch@lst.de \
--cc=herbert@gondor.apana.org.au \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=qat-linux@intel.com \
--cc=salvatore.benedetto@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox