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

[Steven Whitehouse <swhiteho@redhat.com>]

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);
>  	}
>