[Ocfs2-devel] [PATCH] ocfs2: actually free hb set on cluster removal

[Ocfs2-devel] [PATCH] ocfs2: actually free hb set on cluster removal

[Jeff Mahoney]

 This patch actually frees the hb set when the cluster dir is removed.

 Since heartbeat threads are stopped when the individual heartbeat sets are
 removed, this just cleans up the allocation.

 fs/ocfs2/cluster/nodemanager.c |    1 +
 1 files changed, 1 insertion(+)

Signed-off-by: Jeff Mahoney <jeffm@suse.com>

diff -ruNpX dontdiff linux-2.6.15-staging2/fs/ocfs2/cluster/nodemanager.c linux-2.6.15-ocfs2/fs/ocfs2/cluster/nodemanager.c
--- linux-2.6.15-staging2/fs/ocfs2/cluster/nodemanager.c	2006-01-08 20:07:52.077646720 -0500
+++ linux-2.6.15-ocfs2/fs/ocfs2/cluster/nodemanager.c	2006-01-08 18:08:59.630943432 -0500
@@ -685,6 +685,7 @@ static void o2nm_cluster_group_drop_item
 	BUG_ON(o2nm_single_cluster != cluster);
 	o2nm_single_cluster = NULL;
 
+	o2hb_free_hb_set(cluster->cl_group.default_groups[1]);
 	for (i = 0; cluster->cl_group.default_groups[i]; i++) {
 		killme = &cluster->cl_group.default_groups[i]->cg_item;
 		cluster->cl_group.default_groups[i] = NULL;
-- 
Jeff Mahoney
SuSE Labs

[Ocfs2-devel] [PATCH] ocfs2: actually free hb set on cluster removal

[Mark Fasheh]

Hi,

On Mon, Jan 09, 2006 at 12:42:35PM -0500, Jeff Mahoney wrote:
>  Since heartbeat threads are stopped when the individual heartbeat sets are
>  removed, this just cleans up the allocation.
Yes, it seems we're leaking memory there.

> +	o2hb_free_hb_set(cluster->cl_group.default_groups[1]);
>  	for (i = 0; cluster->cl_group.default_groups[i]; i++) {
>  		killme = &cluster->cl_group.default_groups[i]->cg_item;
>  		cluster->cl_group.default_groups[i] = NULL;
What about the for loop on the next line? Won't that try to access memory
freed in the call to o2hb_free_hb_set(...)?                 
	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
mark.fasheh at oracle.com

[Ocfs2-devel] [PATCH] ocfs2: actually free hb set on cluster removal

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark Fasheh wrote:
> Hi,
> 
> On Mon, Jan 09, 2006 at 12:42:35PM -0500, Jeff Mahoney wrote:
>>  Since heartbeat threads are stopped when the individual heartbeat sets are
>>  removed, this just cleans up the allocation.
> Yes, it seems we're leaking memory there.
> 
>> +	o2hb_free_hb_set(cluster->cl_group.default_groups[1]);
>>  	for (i = 0; cluster->cl_group.default_groups[i]; i++) {
>>  		killme = &cluster->cl_group.default_groups[i]->cg_item;
>>  		cluster->cl_group.default_groups[i] = NULL;
> What about the for loop on the next line? Won't that try to access memory
> freed in the call to o2hb_free_hb_set(...)?                 
> 	--Mark

Hi Mark -

Apologies, my OCFS2 tree is quite a ways from the stock one. I guess I
was just overeager pushing patches out that I figured could be applied
before I started posting the bigger changes.

So, yes, of course it will end up freeing memory that will be used
immediately afterwards. In my tree, o2hb_free_hb_set calls an exit
callback and decrements some references - no memory is freed.

I had planned on waiting a bit longer to fix up one of the early
patches, but I guess now is a good a time as any to post them.

So, I'll post them shortly. :)

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDwuUELPWxlyuTD7IRAmyvAJ48v8u7OTHGxdRxwKgnr8/BiepTxQCeLt1G
iEEluYM73wpSYAnUXvjBQAY=
=sgXu
-----END PGP SIGNATURE-----