* [PATCH] misc: mic: fix memory leak
@ 2015-10-13 9:08 Sudip Mukherjee
2015-10-13 9:50 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sudip Mukherjee @ 2015-10-13 9:08 UTC (permalink / raw)
To: Sudeep Dutt, Greg Kroah-Hartman; +Cc: linux-kernel, Sudip Mukherjee
In scif_node_connect() we were returning if the initialization of p2p_ji
fails. But at that time p2p_ij has already been initialized and
resources allocated for it. And since p2p_ij is not added to the list
till now so we will have a leak.
Lets deinitialize and release the resources connected to p2p_ij.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
One sudip sending patch to another sudeep... :)
drivers/misc/mic/scif/scif_nodeqp.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/mic/scif/scif_nodeqp.c b/drivers/misc/mic/scif/scif_nodeqp.c
index 7180d56..5e94fac 100644
--- a/drivers/misc/mic/scif/scif_nodeqp.c
+++ b/drivers/misc/mic/scif/scif_nodeqp.c
@@ -435,6 +435,18 @@ free_p2p:
return NULL;
}
+/* Uninitialize and release resources from a p2p mapping */
+void scif_deinit_p2p_info(struct scif_dev *scifdev, struct scif_p2p_info *p2p)
+{
+ struct scif_hw_dev *sdev = scifdev->sdev;
+
+ dma_unmap_sg(&sdev->dev, p2p->ppi_sg[SCIF_PPI_MMIO],
+ p2p->sg_nentries[SCIF_PPI_MMIO], DMA_BIDIRECTIONAL);
+ scif_p2p_freesg(p2p->ppi_sg[SCIF_PPI_MMIO]);
+ scif_p2p_freesg(p2p->ppi_sg[SCIF_PPI_APER]);
+ kfree(p2p);
+}
+
/**
* scif_node_connect: Respond to SCIF_NODE_CONNECT interrupt message
* @dst: Destination node
@@ -477,8 +489,10 @@ static void scif_node_connect(struct scif_dev *scifdev, int dst)
if (!p2p_ij)
return;
p2p_ji = scif_init_p2p_info(dev_j, dev_i);
- if (!p2p_ji)
+ if (!p2p_ji) {
+ scif_deinit_p2p_info(dev_i, p2p_ij);
return;
+ }
list_add_tail(&p2p_ij->ppi_list, &dev_i->p2p);
list_add_tail(&p2p_ji->ppi_list, &dev_j->p2p);
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] misc: mic: fix memory leak
2015-10-13 9:08 [PATCH] misc: mic: fix memory leak Sudip Mukherjee
@ 2015-10-13 9:50 ` kbuild test robot
2015-10-13 10:23 ` Sudip Mukherjee
2015-10-13 9:50 ` [RFC PATCH] misc: mic: scif_deinit_p2p_info() can be static kbuild test robot
2015-10-13 16:34 ` [PATCH] misc: mic: fix memory leak Sudeep Dutt
2 siblings, 1 reply; 7+ messages in thread
From: kbuild test robot @ 2015-10-13 9:50 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: kbuild-all, Sudeep Dutt, Greg Kroah-Hartman, linux-kernel,
Sudip Mukherjee
Hi Sudip,
[auto build test WARNING on char-misc/char-misc-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Sudip-Mukherjee/misc-mic-fix-memory-leak/20151013-171026
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/misc/mic/scif/scif_nodeqp.c:439:6: sparse: symbol 'scif_deinit_p2p_info' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH] misc: mic: scif_deinit_p2p_info() can be static
2015-10-13 9:08 [PATCH] misc: mic: fix memory leak Sudip Mukherjee
2015-10-13 9:50 ` kbuild test robot
@ 2015-10-13 9:50 ` kbuild test robot
2015-10-13 16:34 ` [PATCH] misc: mic: fix memory leak Sudeep Dutt
2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2015-10-13 9:50 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: kbuild-all, Sudeep Dutt, Greg Kroah-Hartman, linux-kernel,
Sudip Mukherjee
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
scif_nodeqp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/mic/scif/scif_nodeqp.c b/drivers/misc/mic/scif/scif_nodeqp.c
index 5e94fac..81f0b23 100644
--- a/drivers/misc/mic/scif/scif_nodeqp.c
+++ b/drivers/misc/mic/scif/scif_nodeqp.c
@@ -436,7 +436,7 @@ free_p2p:
}
/* Uninitialize and release resources from a p2p mapping */
-void scif_deinit_p2p_info(struct scif_dev *scifdev, struct scif_p2p_info *p2p)
+static void scif_deinit_p2p_info(struct scif_dev *scifdev, struct scif_p2p_info *p2p)
{
struct scif_hw_dev *sdev = scifdev->sdev;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] misc: mic: fix memory leak
2015-10-13 9:08 [PATCH] misc: mic: fix memory leak Sudip Mukherjee
2015-10-13 9:50 ` kbuild test robot
2015-10-13 9:50 ` [RFC PATCH] misc: mic: scif_deinit_p2p_info() can be static kbuild test robot
@ 2015-10-13 16:34 ` Sudeep Dutt
2015-10-13 17:16 ` Sudip Mukherjee
2 siblings, 1 reply; 7+ messages in thread
From: Sudeep Dutt @ 2015-10-13 16:34 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Sudeep Dutt, Greg Kroah-Hartman, linux-kernel
On Tue, 2015-10-13 at 14:38 +0530, Sudip Mukherjee wrote:
> In scif_node_connect() we were returning if the initialization of p2p_ji
> fails. But at that time p2p_ij has already been initialized and
> resources allocated for it. And since p2p_ij is not added to the list
> till now so we will have a leak.
> Lets deinitialize and release the resources connected to p2p_ij.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>
> One sudip sending patch to another sudeep... :)
>
> drivers/misc/mic/scif/scif_nodeqp.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/mic/scif/scif_nodeqp.c b/drivers/misc/mic/scif/scif_nodeqp.c
> index 7180d56..5e94fac 100644
> --- a/drivers/misc/mic/scif/scif_nodeqp.c
> +++ b/drivers/misc/mic/scif/scif_nodeqp.c
> @@ -435,6 +435,18 @@ free_p2p:
> return NULL;
> }
>
> +/* Uninitialize and release resources from a p2p mapping */
> +void scif_deinit_p2p_info(struct scif_dev *scifdev, struct scif_p2p_info *p2p)
> +{
> + struct scif_hw_dev *sdev = scifdev->sdev;
> +
> + dma_unmap_sg(&sdev->dev, p2p->ppi_sg[SCIF_PPI_MMIO],
> + p2p->sg_nentries[SCIF_PPI_MMIO], DMA_BIDIRECTIONAL);
Hi Sudip,
You missed unmapping the aperture here:
dma_unmap_sg(&sdev->dev, p2p->ppi_sg[SCIF_PPI_APER],
p2p->sg_nentries[SCIF_PPI_APER], DMA_BIDIRECTIONAL);
Can you please resubmit this patch with the change above?
Also you should mention [PATCH char-misc-next] in the next revision.
Thanks for the fix!
Sudeep Dutt
> + scif_p2p_freesg(p2p->ppi_sg[SCIF_PPI_MMIO]);
> + scif_p2p_freesg(p2p->ppi_sg[SCIF_PPI_APER]);
> + kfree(p2p);
> +}
> +
> /**
> * scif_node_connect: Respond to SCIF_NODE_CONNECT interrupt message
> * @dst: Destination node
> @@ -477,8 +489,10 @@ static void scif_node_connect(struct scif_dev *scifdev, int dst)
> if (!p2p_ij)
> return;
> p2p_ji = scif_init_p2p_info(dev_j, dev_i);
> - if (!p2p_ji)
> + if (!p2p_ji) {
> + scif_deinit_p2p_info(dev_i, p2p_ij);
> return;
> + }
> list_add_tail(&p2p_ij->ppi_list, &dev_i->p2p);
> list_add_tail(&p2p_ji->ppi_list, &dev_j->p2p);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] misc: mic: fix memory leak
2015-10-13 16:34 ` [PATCH] misc: mic: fix memory leak Sudeep Dutt
@ 2015-10-13 17:16 ` Sudip Mukherjee
2015-10-13 17:25 ` Sudeep Dutt
0 siblings, 1 reply; 7+ messages in thread
From: Sudip Mukherjee @ 2015-10-13 17:16 UTC (permalink / raw)
To: Sudeep Dutt; +Cc: Greg Kroah-Hartman, linux-kernel
On Tue, Oct 13, 2015 at 09:34:21AM -0700, Sudeep Dutt wrote:
> On Tue, 2015-10-13 at 14:38 +0530, Sudip Mukherjee wrote:
<snip>
> > + struct scif_hw_dev *sdev = scifdev->sdev;
> > +
> > + dma_unmap_sg(&sdev->dev, p2p->ppi_sg[SCIF_PPI_MMIO],
> > + p2p->sg_nentries[SCIF_PPI_MMIO], DMA_BIDIRECTIONAL);
>
> Hi Sudip,
>
> You missed unmapping the aperture here:
>
> dma_unmap_sg(&sdev->dev, p2p->ppi_sg[SCIF_PPI_APER],
> p2p->sg_nentries[SCIF_PPI_APER], DMA_BIDIRECTIONAL);
>
> Can you please resubmit this patch with the change above?
Hi Sudeep,
v3 sent for your review.
>
> Also you should mention [PATCH char-misc-next] in the next revision.
Ohhh... yes, I keep forgetting that Fengguang will need this tag for
his build bot.
But I have one more doubt. If scif_nodeqp_send() fails then shouldn't we
remove these two nodes of the p2p connection from the lists and release
the resources?
regards
sudip
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] misc: mic: fix memory leak
2015-10-13 17:16 ` Sudip Mukherjee
@ 2015-10-13 17:25 ` Sudeep Dutt
0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Dutt @ 2015-10-13 17:25 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Sudeep Dutt, Greg Kroah-Hartman, linux-kernel
On Tue, 2015-10-13 at 22:46 +0530, Sudip Mukherjee wrote:
> On Tue, Oct 13, 2015 at 09:34:21AM -0700, Sudeep Dutt wrote:
> > On Tue, 2015-10-13 at 14:38 +0530, Sudip Mukherjee wrote:
> <snip>
> > > + struct scif_hw_dev *sdev = scifdev->sdev;
> > > +
> > > + dma_unmap_sg(&sdev->dev, p2p->ppi_sg[SCIF_PPI_MMIO],
> > > + p2p->sg_nentries[SCIF_PPI_MMIO], DMA_BIDIRECTIONAL);
> >
> > Hi Sudip,
> >
> > You missed unmapping the aperture here:
> >
> > dma_unmap_sg(&sdev->dev, p2p->ppi_sg[SCIF_PPI_APER],
> > p2p->sg_nentries[SCIF_PPI_APER], DMA_BIDIRECTIONAL);
> >
> > Can you please resubmit this patch with the change above?
>
> Hi Sudeep,
> v3 sent for your review.
> >
> > Also you should mention [PATCH char-misc-next] in the next revision.
>
> Ohhh... yes, I keep forgetting that Fengguang will need this tag for
> his build bot.
>
> But I have one more doubt. If scif_nodeqp_send() fails then shouldn't we
> remove these two nodes of the p2p connection from the lists and release
> the resources?
The resources will get cleaned up via scif_destroy_p2p(..) once they are
added to the lists.
Sudeep Dutt
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-13 17:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-13 9:08 [PATCH] misc: mic: fix memory leak Sudip Mukherjee
2015-10-13 9:50 ` kbuild test robot
2015-10-13 10:23 ` Sudip Mukherjee
2015-10-13 9:50 ` [RFC PATCH] misc: mic: scif_deinit_p2p_info() can be static kbuild test robot
2015-10-13 16:34 ` [PATCH] misc: mic: fix memory leak Sudeep Dutt
2015-10-13 17:16 ` Sudip Mukherjee
2015-10-13 17:25 ` Sudeep Dutt
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).