Skip to contentSkip to conversation

ocfs2: Free sc->sc_page in sc_kref_release()

PATCH New Younger Liu 3 participants 5 messages Jun 18, 2013 OCFS2

lore
Share & tools
lore

Younger Liu

author
#0 lore raw

PATCH 1/1 ocfs2: Free sc->sc_page in sc_kref_release()

There is a memory leak in sc_kref_release().
When free struct o2net_sock_container (sc),
we should release sc->sc_page.

Signed-off-by Younger Liu <[email protected]>
---
 fs/ocfs2/cluster/tcp.c |    5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/cluster/tcp.c +5 -0
index aa88bd8..f0272b9 100644
@@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
 	sc->sc_node = NULL;
 
 	o2net_debug_del_sc(sc);
+
+	if (sc->sc_page) {
+		__free_page(sc->sc_page);
+		sc->sc_page = NULL;
+	}
 	kfree(sc);
 }

-- 1.7.9.7

#1 lore raw

PATCH 1/1 ocfs2: Free sc->sc_page in sc_kref_release()

Quoted 5 lines · Younger Liu

There is a memory leak in sc_kref_release().
When free struct o2net_sock_container (sc),
we should release sc->sc_page.

Signed-off-by Younger Liu <[email protected]>

Looks fine to me, thanks!

Reviewed-by Jie Liu <[email protected]>
Quoted 21 lines
---
 fs/ocfs2/cluster/tcp.c |    5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/cluster/tcp.c +5 -1
index aa88bd8..f0272b9 100644
@@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
 	sc->sc_node = NULL;
 
 	o2net_debug_del_sc(sc);
+
+	if (sc->sc_page) {
+		__free_page(sc->sc_page);
+		sc->sc_page = NULL;
+	}
 	kfree(sc);
 }
 
-- 1.7.9.7
#2 lore raw

PATCH 1/1 ocfs2: Free sc->sc_page in sc_kref_release()

Quoted 23 lines · Jeff Liu
Quoted 5 lines · Younger Liu

There is a memory leak in sc_kref_release().
When free struct o2net_sock_container (sc),
we should release sc->sc_page.

Signed-off-by Younger Liu <[email protected]>

Looks fine to me, thanks!

Reviewed-by Jie Liu <[email protected]>
Quoted 11 lines
---
 fs/ocfs2/cluster/tcp.c |    5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/cluster/tcp.c +0 -0
index aa88bd8..f0272b9 100644
@@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
 	sc->sc_node = NULL;
Quoted 5 lines

fs/ocfs2/cluster/tcp.c static void sc_kref_release(struct kref *kref) · line 412

Quoted 5 lines
fs/ocfs2/cluster/tcp.c +4 -0
 	o2net_debug_del_sc(sc);
+
+	if (sc->sc_page) {
+		__free_page(sc->sc_page);
+		sc->sc_page = NULL;

But why set sc->sc_page to NULL, given sc is to be kfreed.

Quoted 3 lines
Quoted 3 lines
fs/ocfs2/cluster/tcp.c +1 -0
+	}
 	kfree(sc);
 }
#3 lore raw

PATCH 1/1 ocfs2: Free sc->sc_page in sc_kref_release()

Quoted 24 lines · Li Zefan
Quoted 23 lines · Jeff Liu
Quoted 5 lines · Younger Liu

There is a memory leak in sc_kref_release().
When free struct o2net_sock_container (sc),
we should release sc->sc_page.

Signed-off-by Younger Liu <[email protected]>

Looks fine to me, thanks!

Reviewed-by Jie Liu <[email protected]>
Quoted 11 lines
---
 fs/ocfs2/cluster/tcp.c |    5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/cluster/tcp.c +0 -0
index aa88bd8..f0272b9 100644
@@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
 	sc->sc_node = NULL;
Quoted 7 lines
Quoted 5 lines

fs/ocfs2/cluster/tcp.c static void sc_kref_release(struct kref *kref) · line 412

Quoted 5 lines
fs/ocfs2/cluster/tcp.c +4 -0
 	o2net_debug_del_sc(sc);
+
+	if (sc->sc_page) {
+		__free_page(sc->sc_page);
+		sc->sc_page = NULL;

But why set sc->sc_page to NULL, given sc is to be kfreed.

Nice catch. I have not reacted from another thing in process at that time.

Thanks,
-Jeff

Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
fs/ocfs2/cluster/tcp.c +1 -0
+	}
 	kfree(sc);
 }
author
#4 lore raw

PATCH 1/1 ocfs2: Free sc->sc_page in sc_kref_release()

Thanks for your review comments.
I will resend this patch.

Quoted 48 lines · Jeff Liu
Quoted 24 lines · Li Zefan
Quoted 23 lines · Jeff Liu
Quoted 5 lines · Younger Liu

There is a memory leak in sc_kref_release().
When free struct o2net_sock_container (sc),
we should release sc->sc_page.

Signed-off-by Younger Liu <[email protected]>

Looks fine to me, thanks!

Reviewed-by Jie Liu <[email protected]>
Quoted 11 lines
---
 fs/ocfs2/cluster/tcp.c |    5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/cluster/tcp.c +0 -0
index aa88bd8..f0272b9 100644
@@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
 	sc->sc_node = NULL;
Quoted 7 lines
Quoted 5 lines

fs/ocfs2/cluster/tcp.c static void sc_kref_release(struct kref *kref) · line 412

Quoted 5 lines
fs/ocfs2/cluster/tcp.c +4 -0
 	o2net_debug_del_sc(sc);
+
+	if (sc->sc_page) {
+		__free_page(sc->sc_page);
+		sc->sc_page = NULL;

But why set sc->sc_page to NULL, given sc is to be kfreed.

Nice catch. I have not reacted from another thing in process at that time.

Thanks,
-Jeff

Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
fs/ocfs2/cluster/tcp.c +1 -0
+	}
 	kfree(sc);
 }