* [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent()
@ 2019-02-11 15:05 Ewan D. Milne
2019-02-11 19:19 ` James Smart
2019-02-12 8:06 ` Christoph Hellwig
0 siblings, 2 replies; 7+ messages in thread
From: Ewan D. Milne @ 2019-02-11 15:05 UTC (permalink / raw)
To: linux-scsi, linux-nvme
Cc: stable, james.smart, dick.kennedy, martin.petersen, jejb
The change to use dma_set_mask_and_coherent() incorrectly made a second
call with the 32 bit DMA mask value when the call with the 64 bit DMA
mask value succeeded. This resulted in NVMe/FC connections failing due
to corrupted data buffers, and various other SCSI/FCP I/O errors.
Fixes: f30e1bfd6154 ("scsi: lpfc: use dma_set_mask_and_coherent")
Cc: <stable@vger.kernel.org>
Suggested-by: Don Dutile <ddutile@redhat.com>
Signed-off-by: Ewan D. Milne <emilne@redhat.com>
---
drivers/scsi/lpfc/lpfc_init.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index bede11e..60c178d 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -7367,9 +7367,9 @@ struct lpfc_rpi_hdr *
return error;
/* Set the device DMA mask size */
- if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
- dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
- return error;
+ if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) != 0)
+ if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0)
+ return error;
/* Get the bus address of Bar0 and Bar2 and the number of bytes
* required by each mapping.
@@ -9745,9 +9745,9 @@ struct lpfc_cq_event *
return error;
/* Set the device DMA mask size */
- if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
- dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
- return error;
+ if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) != 0)
+ if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0)
+ return error;
/*
* The BARs and register set definitions and offset locations are
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent()
2019-02-11 15:05 [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent() Ewan D. Milne
@ 2019-02-11 19:19 ` James Smart
2019-02-12 8:06 ` Christoph Hellwig
1 sibling, 0 replies; 7+ messages in thread
From: James Smart @ 2019-02-11 19:19 UTC (permalink / raw)
To: Ewan D. Milne, linux-scsi, linux-nvme
Cc: stable, dick.kennedy, martin.petersen, jejb
On 2/11/2019 7:05 AM, Ewan D. Milne wrote:
> The change to use dma_set_mask_and_coherent() incorrectly made a second
> call with the 32 bit DMA mask value when the call with the 64 bit DMA
> mask value succeeded. This resulted in NVMe/FC connections failing due
> to corrupted data buffers, and various other SCSI/FCP I/O errors.
>
> Fixes: f30e1bfd6154 ("scsi: lpfc: use dma_set_mask_and_coherent")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Don Dutile <ddutile@redhat.com>
> Signed-off-by: Ewan D. Milne <emilne@redhat.com>
> ---
> drivers/scsi/lpfc/lpfc_init.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index bede11e..60c178d 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -7367,9 +7367,9 @@ struct lpfc_rpi_hdr *
> return error;
>
> /* Set the device DMA mask size */
> - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
> - dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
> - return error;
> + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) != 0)
> + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0)
> + return error;
>
> /* Get the bus address of Bar0 and Bar2 and the number of bytes
> * required by each mapping.
> @@ -9745,9 +9745,9 @@ struct lpfc_cq_event *
> return error;
>
> /* Set the device DMA mask size */
> - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
> - dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
> - return error;
> + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) != 0)
> + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0)
> + return error;
>
> /*
> * The BARs and register set definitions and offset locations are
Thanks Ewan.
Signed-off-by: James Smart <james.smart@broadcom.com>
-- james
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent()
2019-02-11 15:05 [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent() Ewan D. Milne
2019-02-11 19:19 ` James Smart
@ 2019-02-12 8:06 ` Christoph Hellwig
2019-02-12 15:37 ` Ewan D. Milne
1 sibling, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-02-12 8:06 UTC (permalink / raw)
To: Ewan D. Milne
Cc: linux-scsi, linux-nvme, stable, james.smart, dick.kennedy,
martin.petersen, jejb
On Mon, Feb 11, 2019 at 10:05:02AM -0500, Ewan D. Milne wrote:
> The change to use dma_set_mask_and_coherent() incorrectly made a second
> call with the 32 bit DMA mask value when the call with the 64 bit DMA
> mask value succeeded. This resulted in NVMe/FC connections failing due
> to corrupted data buffers, and various other SCSI/FCP I/O errors.
Ooops, sorry.
> /* Set the device DMA mask size */
> - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
> - dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
> - return error;
> + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) != 0)
> + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0)
But this still looks obsfuctating, why not:
error = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
if (error)
error = dma_set_mask_and_coherent(&pdev->dev,
DMA_BIT_MASK(32)));
if (error)
return error;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent()
2019-02-12 8:06 ` Christoph Hellwig
@ 2019-02-12 15:37 ` Ewan D. Milne
2019-02-12 18:34 ` Christoph Hellwig
2019-02-13 3:10 ` Martin K. Petersen
0 siblings, 2 replies; 7+ messages in thread
From: Ewan D. Milne @ 2019-02-12 15:37 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-scsi, linux-nvme, stable, james.smart, dick.kennedy,
martin.petersen, jejb
On Tue, 2019-02-12 at 00:06 -0800, Christoph Hellwig wrote:
> On Mon, Feb 11, 2019 at 10:05:02AM -0500, Ewan D. Milne wrote:
> > The change to use dma_set_mask_and_coherent() incorrectly made a second
> > call with the 32 bit DMA mask value when the call with the 64 bit DMA
> > mask value succeeded. This resulted in NVMe/FC connections failing due
> > to corrupted data buffers, and various other SCSI/FCP I/O errors.
>
> Ooops, sorry.
>
> > /* Set the device DMA mask size */
> > - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
> > - dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
> > - return error;
> > + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) != 0)
> > + if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0)
>
> But this still looks obsfuctating, why not:
>
> error = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> if (error)
> error = dma_set_mask_and_coherent(&pdev->dev,
> DMA_BIT_MASK(32)));
> if (error)
> return error;
I agree that making the flow of control explicit would be good, but...
It looks like this would introduce a different problem. "error" is set to
-ENODEV earlier in the two functions so that the various error paths will return
that value. If it is overwritten with a successful result from the call to
dma_set_mask_and_coherent() but a later call to e.g. ioremap() fails, the
functions will incorrectly return a value indicating that they have succeeded.
It also appears as if the patches to hptiop, hisi_sas and bfa need to be fixed up.
I don't have a test environment for these, although I might be able to modify the
test environment for bfa. Martin/James, what about the others?
-Ewan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent()
2019-02-12 15:37 ` Ewan D. Milne
@ 2019-02-12 18:34 ` Christoph Hellwig
2019-02-13 3:10 ` Martin K. Petersen
1 sibling, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2019-02-12 18:34 UTC (permalink / raw)
To: Ewan D. Milne
Cc: Christoph Hellwig, linux-scsi, linux-nvme, stable, james.smart,
dick.kennedy, martin.petersen, jejb
On Tue, Feb 12, 2019 at 10:37:45AM -0500, Ewan D. Milne wrote:
>
> It looks like this would introduce a different problem. "error" is set to
> -ENODEV earlier in the two functions so that the various error paths will return
> that value. If it is overwritten with a successful result from the call to
> dma_set_mask_and_coherent() but a later call to e.g. ioremap() fails, the
> functions will incorrectly return a value indicating that they have succeeded.
>
> It also appears as if the patches to hptiop, hisi_sas and bfa need to be fixed up.
> I don't have a test environment for these, although I might be able to modify the
> test environment for bfa. Martin/James, what about the others?
I don't have a test environment for them either, and I doubt many
people have. If you don't feel comfortable I can send a fix for
them.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent()
2019-02-12 15:37 ` Ewan D. Milne
2019-02-12 18:34 ` Christoph Hellwig
@ 2019-02-13 3:10 ` Martin K. Petersen
2019-02-13 9:18 ` John Garry
1 sibling, 1 reply; 7+ messages in thread
From: Martin K. Petersen @ 2019-02-13 3:10 UTC (permalink / raw)
To: Ewan D. Milne
Cc: Christoph Hellwig, linux-scsi, linux-nvme, stable, james.smart,
dick.kennedy, martin.petersen, jejb, John Garry
Ewan,
> It also appears as if the patches to hptiop, hisi_sas and bfa need to
> be fixed up. I don't have a test environment for these, although I
> might be able to modify the test environment for bfa. Martin/James,
> what about the others?
I'm afraid I don't have any of those. But I'm sure John would be happy
to check hisi_sas.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent()
2019-02-13 3:10 ` Martin K. Petersen
@ 2019-02-13 9:18 ` John Garry
0 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2019-02-13 9:18 UTC (permalink / raw)
To: Martin K. Petersen, Ewan D. Milne, Christoph Hellwig
Cc: linux-scsi, linux-nvme, stable, james.smart, dick.kennedy, jejb
On 13/02/2019 03:10, Martin K. Petersen wrote:
>
> Ewan,
>
>> It also appears as if the patches to hptiop, hisi_sas and bfa need to
>> be fixed up. I don't have a test environment for these, although I
>> might be able to modify the test environment for bfa. Martin/James,
>> what about the others?
>
> I'm afraid I don't have any of those. But I'm sure John would be happy
> to check hisi_sas.
>
Happy to test. Or I can just send the fix myself. So what is the
preferred pattern for setting the DMA mask?
John
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-13 9:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 15:05 [PATCH] scsi: lpfc: fix calls to dma_set_mask_and_coherent() Ewan D. Milne
2019-02-11 19:19 ` James Smart
2019-02-12 8:06 ` Christoph Hellwig
2019-02-12 15:37 ` Ewan D. Milne
2019-02-12 18:34 ` Christoph Hellwig
2019-02-13 3:10 ` Martin K. Petersen
2019-02-13 9:18 ` John Garry
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).