Hi, --On 9 March 2007 12:55:11 PM +0100 Christoph Hellwig <[EMAIL PROTECTED]> wrote:
Ed Cashin found a bug in the error handling code for the case where a page allocation fails. Here's the updated version: Index: linux-2.6/fs/xfs/linux-2.6/xfs_buf.c =================================================================== --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_buf.c 2007-03-08 19:08:38.000000000 +0100 +++ linux-2.6/fs/xfs/linux-2.6/xfs_buf.c 2007-03-09 08:59:15.000000000 +0100
....
+ for (i = 0; i < page_count; i++) { + bp->b_pages[i] = alloc_page(GFP_KERNEL); + if (!bp->b_pages[i]) + goto fail_free_mem; + } + bp->b_flags |= _XBF_PAGES; + + error = _xfs_buf_map_pages(bp, XBF_MAPPED); + if (unlikely(error)) { + printk(KERN_WARNING "%s: failed to map pages\n", + __FUNCTION__); goto fail_free_mem; - bp->b_flags |= _XBF_KMEM_ALLOC; + } xfs_buf_unlock(bp); XB_TRACE(bp, "no_daddr", data); return bp; + fail_free_mem: - kmem_free(data, malloc_len); + for ( ; i >= 0; i--) + __free_page(bp->b_pages[i]); fail_free_buf: xfs_buf_free(bp); fail:
It looks like you might need: for (i--; i >= 0; i--) (or: for (j = 0; j < i; j++) etc.) Because if the initial alloc_page loop goes to completion then: i == pagecount and if alloc_page loop terminates early then bp->b_pages[i] == NULL So we have gone 1 too far in both cases and need to start free'ing back one. Unless I missed something. --Tim - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/