mpt3sas: memory allocation for firmware upgrade DMA memory question

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chaitra and Suganath,

I've got a question regarding mpt3sas' memory allocation used when doing a
firmware upgrade. Currently you're doing a pci_alloc_consitent() which tries
to allocate memory via GFP_ATOMIC. This memory then is passed as a single
element on a sg_list.

Jeff reported it returned -ENOMEM on his Server due to highly fragmented
memory.

Is it required to have the memory for the DMA operation contiguous, or can I
just allocate several non-contiguous junks of memory and map it to a sg_list?

If not, is GFP_ATMOIC really needed? I've converted the driver to use
GFP_KERNEL but I'm a bit reluctant to test below patch on real hardware to not
brick the HBA.

Thanks,
	Johannes

RFC patch for GFP_KERNEL allocation, though splitting into multiple sg mapped
elements is the preferred fix here:

>From 06e63654d887df7f740dc5abcb40d441a8da7fa5 Mon Sep 17 00:00:00 2001
From: Johannes Thumshirn <jthumshirn@xxxxxxx>
Date: Tue, 24 May 2016 17:25:59 +0200
Subject: [RFC PATCH] mpt3sas: Don't do atomic memory allocations for firmware
 update DMA

Currently mpt3sas uses pci_alloc_consistent() to allocate memory for the DMA
used to do firmware updates. pci_alloc_consistent() in turn uses GFP_ATOMIC
allocations. On a host with high memory fragmention this can lead to page
allocation failures, as the DMA buffer holds the complete firmware update and
thus can need page allocations of higher orders.

As the firmware update code path may sleep, convert allocation to a normal
kzalloc() call with GFP_KERNEL and map it to the DMA buffers.

Reported-by: Jeff Mahoney <jeffm@xxxxxxxx>
Signed-off-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c | 39 ++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 7d00f09..14be3cf 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -751,8 +751,7 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
 
 	/* obtain dma-able memory for data transfer */
 	if (data_out_sz) /* WRITE */ {
-		data_out = pci_alloc_consistent(ioc->pdev, data_out_sz,
-		    &data_out_dma);
+		data_out = kzalloc(data_out_sz, GFP_KERNEL);
 		if (!data_out) {
 			pr_err("failure at %s:%d/%s()!\n", __FILE__,
 			    __LINE__, __func__);
@@ -760,6 +759,14 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
 			mpt3sas_base_free_smid(ioc, smid);
 			goto out;
 		}
+		data_out_dma = pci_map_single(ioc->pdev, data_out,
+					      data_out_sz, PCI_DMA_TODEVICE);
+		if (dma_mapping_error(&ioc->pdev->dev, data_out_dma)) {
+			ret = -EINVAL;
+			mpt3sas_base_free_smid(ioc, smid);
+			goto out_free_data_out;
+		}
+
 		if (copy_from_user(data_out, karg.data_out_buf_ptr,
 			data_out_sz)) {
 			pr_err("failure at %s:%d/%s()!\n", __FILE__,
@@ -771,8 +778,7 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
 	}
 
 	if (data_in_sz) /* READ */ {
-		data_in = pci_alloc_consistent(ioc->pdev, data_in_sz,
-		    &data_in_dma);
+		data_in = kzalloc(data_in_sz, GFP_KERNEL);
 		if (!data_in) {
 			pr_err("failure at %s:%d/%s()!\n", __FILE__,
 			    __LINE__, __func__);
@@ -780,6 +786,13 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
 			mpt3sas_base_free_smid(ioc, smid);
 			goto out;
 		}
+		data_in_dma = pci_map_single(ioc->pdev, data_in,
+					     data_in_sz, PCI_DMA_FROMDEVICE);
+		if (dma_mapping_error(&ioc->pdev->dev, data_in_dma)) {
+			ret = -EINVAL;
+			mpt3sas_base_free_smid(ioc, smid);
+			goto out_free_data_in;
+		}
 	}
 
 	psge = (void *)request + (karg.data_sge_offset*4);
@@ -1013,13 +1026,19 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
  out:
 
 	/* free memory associated with sg buffers */
-	if (data_in)
-		pci_free_consistent(ioc->pdev, data_in_sz, data_in,
-		    data_in_dma);
+	if (data_in) {
+		pci_unmap_single(ioc->pdev, data_in_dma,
+				 data_in_sz, PCI_DMA_TODEVICE);
+out_free_data_in:
+		kfree(data_in);
+	}
 
-	if (data_out)
-		pci_free_consistent(ioc->pdev, data_out_sz, data_out,
-		    data_out_dma);
+	if (data_out) {
+		pci_unmap_single(ioc->pdev, data_out_dma,
+				 data_out_sz, PCI_DMA_FROMDEVICE);
+out_free_data_out:
+		kfree(data_out);
+	}
 
 	kfree(mpi_request);
 	ioc->ctl_cmds.status = MPT3_CMD_NOT_USED;
-- 
1.8.5.6



-- 
Johannes Thumshirn                                          Storage
jthumshirn@xxxxxxx                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux