On Tue, 2011-01-25 at 19:50 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> After test 139, kmemleak shows:
>
> unreferenced object 0xffff880078b405d8 (size 400):
> comm "xfs_io", pid 4904, jiffies 4294909383 (age 1186.728s)
> hex dump (first 32 bytes):
. . .
>
> The cause of the leak is that the "remove" parameter of IOP_UNPIN()
> is never set when a CIL push is aborted. This means that the EFI
> item is never freed if it was in the push being cancelled. The
> problem is specific to delayed logging.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Like Christoph, I was a bit confused about xfs_buf_item_unpin()
retaining the call to xfs_trans_del_item(). They all ought to
be done consistently. Hopefully you can straighten that out.
Two other minor comments: I'd prefer that an explicit "1" be
passed to IOP_UNPIN() rather than the variable "aborted" (which is
known to have non-zero value) in xfs_trans_committed_bulk(). And
there are some long-lined comments right near what you're changing
that you could touch up while you're at it.
> ---
> fs/xfs/xfs_extfree_item.c | 1 -
> fs/xfs/xfs_trans.c | 35 ++++++++++++++++++++++++++++++-----
> 2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
. . .
> @@ -1472,6 +1480,16 @@ xfs_trans_committed_bulk(
> if (XFS_LSN_CMP(item_lsn, (xfs_lsn_t)-1) == 0)
> continue;
>
> + /*
> + * if we are aborting the operation, no point in inserting the
> + * object into the AIL as we are in a shutdown situation.
> + */
> + if (aborted) {
> + ASSERT(XFS_FORCED_SHUTDOWN(ailp->xa_mount));
> + IOP_UNPIN(lip, aborted);
Here ^^^
> + continue;
> + }
> +
> if (item_lsn != commit_lsn) {
>
> /*
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
