[Cluster-devel] [PATCH] gfs2: Put bitmap buffers in put_super

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

While that looks like a good fix for now, we should look at this again 
in due course. Why do we have a ref to the rgrp buffers held here in the 
first place? Unless the buffers are pinned in the journal there should 
not be a ref held, otherwise they cannot respond to memory pressure,

Steve.


On 06/11/18 09:39, Andreas Gruenbacher wrote:
> gfs2_put_super calls gfs2_clear_rgrpd to destroy the gfs2_rgrpd objects
> attached to the resource group glocks.  That function should release the
> buffers attached to the gfs2_bitmap objects (bi_bh), but the call to
> gfs2_rgrp_brelse for doing that is missing.
>
> When gfs2_releasepage later runs across these buffers which are still
> referenced, it refuses to free them.  This causes the pages the buffers
> are attached to to remain referenced as well.  With enough mount/unmount
> cycles, the system will eventually run out of memory.
>
> Fix this by adding the missing call to gfs2_rgrp_brelse in
> gfs2_clear_rgrpd.
>
> (Also fix a gfs2_rgrp_relse -> gfs2_rgrp_brelse typo in a comment.)
>
> Fixes: 39b0f1e92908 ("GFS2: Don't brelse rgrp buffer_heads every allocation")
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>   fs/gfs2/rgrp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index ffe3032b1043..b08a530433ad 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -733,6 +733,7 @@ void gfs2_clear_rgrpd(struct gfs2_sbd *sdp)
>   
>   		if (gl) {
>   			glock_clear_object(gl, rgd);
> +			gfs2_rgrp_brelse(rgd);
>   			gfs2_glock_put(gl);
>   		}
>   
> @@ -1174,7 +1175,7 @@ static u32 count_unlinked(struct gfs2_rgrpd *rgd)
>    * @rgd: the struct gfs2_rgrpd describing the RG to read in
>    *
>    * Read in all of a Resource Group's header and bitmap blocks.
> - * Caller must eventually call gfs2_rgrp_relse() to free the bitmaps.
> + * Caller must eventually call gfs2_rgrp_brelse() to free the bitmaps.
>    *
>    * Returns: errno
>    */