[PATCH 1/2] scsi: megaraid: Remove redundant code in megasas_alloc_cmds

[PATCH 1/2] scsi: megaraid: Remove redundant code in megasas_alloc_cmds

[Yisheng Xie]

megasas_alloc_cmds is to alloc cmd_list of instance instead of fusion,
and fusion is useless in this function. Just remove it.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index e518dad..0640168 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4033,9 +4033,7 @@ int megasas_alloc_cmds(struct megasas_instance *instance)
 	int j;
 	u16 max_cmd;
 	struct megasas_cmd *cmd;
-	struct fusion_context *fusion;
 
-	fusion = instance->ctrl_context;
 	max_cmd = instance->max_mfi_cmds;
 
 	/*
-- 
1.7.12.4

[PATCH 2/2] scsi: megaraid: Track the page allocations for struct fusion_context

[Yisheng Xie]

I have get many kmemleak reports just similar to commit 70c54e210ee9
(scsi: megaraid_sas: fix memleak in megasas_alloc_cmdlist_fusion)
on v4.14-rc6, however it seems have a different stroy:

unreferenced object 0xffff8b5139d9d2c0 (size 192):
  comm "kworker/0:0", pid 3, jiffies 4294689182 (age 11347.731s)
  hex dump (first 32 bytes):
    00 33 84 7b 41 8b ff ff 00 33 84 7b 00 00 00 00  .3.{A....3.{....
    00 30 8c 7b 41 8b ff ff 00 30 8c 7b 00 00 00 00  .0.{A....0.{....
  backtrace:
    [<ffffffff927461ea>] kmemleak_alloc+0x4a/0xa0
    [<ffffffff92215eee>] kmem_cache_alloc_trace+0xce/0x1d0
    [<ffffffffc03f96e4>] megasas_alloc_cmdlist_fusion+0xd4/0x180 [megaraid_sas]
    [<ffffffffc03f9df5>] megasas_alloc_cmds_fusion+0x25/0x410 [megaraid_sas]
    [<ffffffffc03fb05d>] megasas_init_adapter_fusion+0x21d/0x6e0 [megaraid_sas]
    [<ffffffffc03f70e8>] megasas_init_fw+0x338/0xd00 [megaraid_sas]
    [<ffffffffc03f806e>] megasas_probe_one.part.34+0x5be/0x1040 [megaraid_sas]
    [<ffffffffc03f8b36>] megasas_probe_one+0x46/0xc0 [megaraid_sas]
    [<ffffffff923c0ec5>] local_pci_probe+0x45/0xa0
    [<ffffffff9209fcf4>] work_for_cpu_fn+0x14/0x20
    [<ffffffff920a2e09>] process_one_work+0x149/0x360
    [<ffffffff920a3578>] worker_thread+0x1d8/0x3c0
    [<ffffffff920a8bb9>] kthread+0x109/0x140
    [<ffffffff92751bc5>] ret_from_fork+0x25/0x30
    [<ffffffffffffffff>] 0xffffffffffffffff

Struct fusion_context may alloc by get_free_pages, which contain
pointers to other slab allocations(via megasas_alloc_cmdlist_fusion).
Since kmemleak does not track/scan page allocations, the slab objects
will be reported as leaks(false positives). This patch adds kmemleak
callbacks to allow tracking of such pages.

Cc: linux-mm@kvack.org
Cc: Shu Wang <shuwang@redhat.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 11bd2e6..9a1be45 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -48,6 +48,7 @@
 #include <linux/mutex.h>
 #include <linux/poll.h>
 #include <linux/vmalloc.h>
+#include <linux/kmemleak.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -4512,6 +4513,14 @@ void megasas_fusion_ocr_wq(struct work_struct *work)
 			dev_err(&instance->pdev->dev, "Failed from %s %d\n", __func__, __LINE__);
 			return -ENOMEM;
 		}
+	} else {
+		/*
+		 * Allow kmemleak to scan these pages as they contain pointers
+		 * to additional allocations via megasas_alloc_cmdlist_fusion.
+		 */
+		size_t size = (size_t)PAGE_SIZE << instance->ctrl_context_pages;
+
+		kmemleak_alloc(instance->ctrl_context, size, 1, GFP_KERNEL);
 	}
 
 	fusion = instance->ctrl_context;
@@ -4548,9 +4557,15 @@ void megasas_fusion_ocr_wq(struct work_struct *work)
 
 		if (is_vmalloc_addr(fusion))
 			vfree(fusion);
-		else
+		else {
+			/*
+			 * Remove kmemleak object previously allocated in
+			 * megasas_alloc_fusion_context.
+			 */
+			kmemleak_free(fusion);
 			free_pages((ulong)fusion,
 				instance->ctrl_context_pages);
+		}
 	}
 }
 
-- 
1.7.12.4

Re: [PATCH 2/2] scsi: megaraid: Track the page allocations for struct fusion_context

[Martin K. Petersen]

Yisheng,

> I have get many kmemleak reports just similar to commit 70c54e210ee9
> (scsi: megaraid_sas: fix memleak in megasas_alloc_cmdlist_fusion)
> on v4.14-rc6, however it seems have a different stroy:

Do you still see leaks reported with the megaraid driver update recently
merged into 4.15/scsi-queue?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 2/2] scsi: megaraid: Track the page allocations for struct fusion_context

[Yisheng Xie]

hi Martin ,

On 2017/10/25 20:36, Martin K. Petersen wrote:
> 
> Yisheng,
> 
>> I have get many kmemleak reports just similar to commit 70c54e210ee9
>> (scsi: megaraid_sas: fix memleak in megasas_alloc_cmdlist_fusion)
>> on v4.14-rc6, however it seems have a different stroy:
> 
> Do you still see leaks reported with the megaraid driver update recently
> merged into 4.15/scsi-queue?
> 
No, the related code have been optimized and __get_free_pages is not used
to allocate fusion_context anymore. So, please ignore this one, and sorry
for disturbing.

BTW, what about the Patch 1/2, which is just a minor clean up?

Thanks
Yisheng Xie

Re: [PATCH 2/2] scsi: megaraid: Track the page allocations for struct fusion_context

[Martin K. Petersen]

Yisheng,

>> Do you still see leaks reported with the megaraid driver update recently
>> merged into 4.15/scsi-queue?
>> 
> No, the related code have been optimized and __get_free_pages is not used
> to allocate fusion_context anymore. So, please ignore this one, and sorry
> for disturbing.
>
> BTW, what about the Patch 1/2, which is just a minor clean up?

Kashyap? Shivasharan? Sumit?

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH 1/2] scsi: megaraid: Remove redundant code in megasas_alloc_cmds

[Sumit Saxena]

-----Original Message-----
From: Yisheng Xie [mailto:xieyisheng1@huawei.com]
Sent: Wednesday, October 25, 2017 3:27 PM
To: kashyap.desai@broadcom.com; sumit.saxena@broadcom.com;
shivasharan.srikanteshwara@broadcom.com; jejb@linux.vnet.ibm.com;
martin.petersen@oracle.com
Cc: megaraidlinux.pdl@broadcom.com; linux-scsi@vger.kernel.org;
linux-kernel@vger.kernel.org; Yisheng Xie
Subject: [PATCH 1/2] scsi: megaraid: Remove redundant code in
megasas_alloc_cmds

megasas_alloc_cmds is to alloc cmd_list of instance instead of fusion, and
fusion is useless in this function. Just remove it.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index e518dad..0640168 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4033,9 +4033,7 @@ int megasas_alloc_cmds(struct megasas_instance
*instance)
 	int j;
 	u16 max_cmd;
 	struct megasas_cmd *cmd;
-	struct fusion_context *fusion;

-	fusion = instance->ctrl_context;
 	max_cmd = instance->max_mfi_cmds;

 	/*
Acked-by: Sumit Saxena <sumit.saxena@broadcom.com>

--
1.7.12.4

Re: [PATCH 1/2] scsi: megaraid: Remove redundant code in megasas_alloc_cmds

[Martin K. Petersen]

Yisheng,

> megasas_alloc_cmds is to alloc cmd_list of instance instead of fusion,
> and fusion is useless in this function. Just remove it.

Applied to 4.15/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering