[Cluster-devel] GFS2: Fix slab memory leak in gfs2_bufdata

[Cluster-devel] GFS2: Fix slab memory leak in gfs2_bufdata

[Bob Peterson]

Hi,

This patch fixes a slab memory leak that sometimes can occur
for files with a very short lifespan. The problem occurs when
a dinode is deleted before it has gotten to the journal properly.
In the leak scenario, the bd object is pinned for journal
committment (queued to the metadata buffers queue: sd_log_le_buf)
but is subsequently unpinned and dequeued before it finds its way
to the ail or the revoke queue. In this rare circumstance, the bd
object needs to be freed from slab memory, or it is forgotten.
We have to be very careful how we do it, though, because
multiple processes can call gfs2_remove_from_journal. In order to
avoid double-frees, only the process that does the unpinning is
allowed to free the bd.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
---
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index e57f608..c7f2469 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -261,6 +261,7 @@ void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr, int
 	struct address_space *mapping = bh->b_page->mapping;
 	struct gfs2_sbd *sdp = gfs2_mapping2sbd(mapping);
 	struct gfs2_bufdata *bd = bh->b_private;
+	int was_pinned = 0;
 
 	if (test_clear_buffer_pinned(bh)) {
 		trace_gfs2_pin(bd, 0);
@@ -276,12 +277,16 @@ void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr, int
 			tr->tr_num_databuf_rm++;
 		}
 		tr->tr_touched = 1;
+		was_pinned = 1;
 		brelse(bh);
 	}
 	if (bd) {
 		spin_lock(&sdp->sd_ail_lock);
 		if (bd->bd_tr) {
 			gfs2_trans_add_revoke(sdp, bd);
+		} else if (was_pinned) {
+			bh->b_private = NULL;
+			kmem_cache_free(gfs2_bufdata_cachep, bd);
 		}
 		spin_unlock(&sdp->sd_ail_lock);
 	}

[Cluster-devel] GFS2: Fix slab memory leak in gfs2_bufdata

[Steven Whitehouse]

Hi,

Thats a good bit of detective work. I've added it to the -nmw tree.
Thanks,

Steve.

On Fri, 2013-12-13 at 08:31 -0500, Bob Peterson wrote:
> Hi,
> 
> This patch fixes a slab memory leak that sometimes can occur
> for files with a very short lifespan. The problem occurs when
> a dinode is deleted before it has gotten to the journal properly.
> In the leak scenario, the bd object is pinned for journal
> committment (queued to the metadata buffers queue: sd_log_le_buf)
> but is subsequently unpinned and dequeued before it finds its way
> to the ail or the revoke queue. In this rare circumstance, the bd
> object needs to be freed from slab memory, or it is forgotten.
> We have to be very careful how we do it, though, because
> multiple processes can call gfs2_remove_from_journal. In order to
> avoid double-frees, only the process that does the unpinning is
> allowed to free the bd.
> 
> Regards,
> 
> Bob Peterson
> Red Hat File Systems
> 
> Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
> ---
> diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
> index e57f608..c7f2469 100644
> --- a/fs/gfs2/meta_io.c
> +++ b/fs/gfs2/meta_io.c
> @@ -261,6 +261,7 @@ void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr, int
>  	struct address_space *mapping = bh->b_page->mapping;
>  	struct gfs2_sbd *sdp = gfs2_mapping2sbd(mapping);
>  	struct gfs2_bufdata *bd = bh->b_private;
> +	int was_pinned = 0;
>  
>  	if (test_clear_buffer_pinned(bh)) {
>  		trace_gfs2_pin(bd, 0);
> @@ -276,12 +277,16 @@ void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr, int
>  			tr->tr_num_databuf_rm++;
>  		}
>  		tr->tr_touched = 1;
> +		was_pinned = 1;
>  		brelse(bh);
>  	}
>  	if (bd) {
>  		spin_lock(&sdp->sd_ail_lock);
>  		if (bd->bd_tr) {
>  			gfs2_trans_add_revoke(sdp, bd);
> +		} else if (was_pinned) {
> +			bh->b_private = NULL;
> +			kmem_cache_free(gfs2_bufdata_cachep, bd);
>  		}
>  		spin_unlock(&sdp->sd_ail_lock);
>  	}
>