xfs
[Top] [All Lists]

Re: [PATCH 5/6] xfs: xlog_recover_process_data leaks like a sieve

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/6] xfs: xlog_recover_process_data leaks like a sieve
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Tue, 17 Dec 2013 11:58:57 -0600
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131216152339.GC12360@xxxxxxxxxxxxx>
References: <1386826478-13846-1-git-send-email-david@xxxxxxxxxxxxx> <1386826478-13846-6-git-send-email-david@xxxxxxxxxxxxx> <20131213123205.GA17935@xxxxxxxxxxxxx> <20131213221102.GX10988@dastard> <20131216152339.GC12360@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 12/16/13 09:23, Christoph Hellwig wrote:
On Sat, Dec 14, 2013 at 09:11:02AM +1100, Dave Chinner wrote:
>  I'll rework it, but hte main issue is that it has to be freed
>  regardless of the error value in commit record processing, so it's
>  not as simple as just freeing it on error....
Indeed, but think that's another reason to move the freeing to
xlog_recover_process_data.  Right now or with the proposed patch
xlog_recover_commit_trans frees trans, but there's nothing that
breaks out of the loop in xlog_recover_process_data after that case.

By moving the freeing there for all cases we can add a corruption
check for that case, and we have a single function that controls
the lifetime of the xlog_recover structure.

I like that idea of doing the frees in xlog_recover_process_data().
Each loop allocates a new trans, so each loop is self contained.

--Mark.

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