xfs
[Top] [All Lists]

Re: [PATCH] [XFS] Free buffer pages array unconditionally

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] [XFS] Free buffer pages array unconditionally
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 2 Dec 2009 10:17:42 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1259734333-20581-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1259734333-20581-1-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.19 (2009-01-05)
On Wed, Dec 02, 2009 at 05:12:13PM +1100, Dave Chinner wrote:
> The code in xfs_free_buf() only attempts to free the b_pages array if the
> buffer is a page cache backed or page allocated buffer. The extra log buffer
> that is used when the log wraps uses pages that are allocated to a different
> log buffer, but it still has a b_pages array allocated when those pages
> are associated to with the extra buffer in xfs_buf_associate_memory.
> 
> Hence we need to always attempt to free the b_pages array when tearing
> down a buffer, not just on buffers that are explicitly marked as page bearing
> buffers. This fixes a leak detected by the kernel memory leak code.
> 
> Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
> ---
>  fs/xfs/linux-2.6/xfs_buf.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
> index 4ddc973..4b84bbc 100644
> --- a/fs/xfs/linux-2.6/xfs_buf.c
> +++ b/fs/xfs/linux-2.6/xfs_buf.c
> @@ -316,7 +316,7 @@ STATIC void
>  _xfs_buf_free_pages(
>       xfs_buf_t       *bp)
>  {
> -     if (bp->b_pages != bp->b_page_array) {
> +     if (bp->b_pages && bp->b_pages != bp->b_page_array) {
>               kmem_free(bp->b_pages);

kmem_free happily takes a NULL pointer, so this is unessecary.

> @@ -349,9 +349,9 @@ xfs_buf_free(
>                               ASSERT(!PagePrivate(page));
>                       page_cache_release(page);
>               }
> -             _xfs_buf_free_pages(bp);
>       }
>  
> +     _xfs_buf_free_pages(bp);

This part looks correct, good catch.


<Prev in Thread] Current Thread [Next in Thread>