On Mon, Aug 10, 2015 at 03:01:49PM -0400, Brian Foster wrote:
> If a failure occurs after the bmap free list is populated and before
> xfs_bmap_finish() completes successfully (which returns a partial list
> on failure), the bmap free list must be cancelled. Otherwise, the extent
> items on the list are never freed and a memory leak occurs.
>
> Several random error paths throughout the code suffer this problem. Fix
> these up such that xfs_bmap_cancel() is always called on error.
Looks sensible. I wonder if we should attach the freelist to the
transaction and make xfs_trans_commit call into bmap_finish and
xfs_trans_cancel into xfs_bmap_cancel in the longer run?
> +STATIC int
> xfs_growfs_rt_alloc(
> - xfs_mount_t *mp, /* file system mount point */
> - xfs_extlen_t oblocks, /* old count of blocks */
> - xfs_extlen_t nblocks, /* new count of blocks */
> - xfs_inode_t *ip) /* inode (bitmap/summary) */
> + struct xfs_mount *mp, /* file system mount point */
> + xfs_extlen_t oblocks, /* old count of blocks */
> + xfs_extlen_t nblocks, /* new count of blocks */
> + struct xfs_inode *ip) /* inode (bitmap/summary) */
> {
> - xfs_fileoff_t bno; /* block number in file */
> - xfs_buf_t *bp; /* temporary buffer for zeroing */
> - int committed; /* transaction committed flag */
> - xfs_daddr_t d; /* disk block address */
> - int error; /* error return value */
> - xfs_fsblock_t firstblock; /* first block allocated in xaction */
> - xfs_bmap_free_t flist; /* list of freed blocks */
> - xfs_fsblock_t fsbno; /* filesystem block for bno */
> - xfs_bmbt_irec_t map; /* block map output */
> - int nmap; /* number of block maps */
> - int resblks; /* space reservation */
> + xfs_fileoff_t bno; /* block number in file */
> + struct xfs_buf *bp; /* temporary buffer for zeroing */
> + int committed; /* transaction committed flag */
> + xfs_daddr_t d; /* disk block address */
> + int error; /* error return value */
> + xfs_fsblock_t firstblock;/* first block allocated in xaction
> */
> + struct xfs_bmap_free flist; /* list of freed blocks */
> + xfs_fsblock_t fsbno; /* filesystem block for bno */
> + struct xfs_bmbt_irec map; /* block map output */
> + int nmap; /* number of block maps */
> + int resblks; /* space reservation */
> + struct xfs_trans *tp;
Do we really need all this unrelated reformatting?
|