Re: [GIT PULL] XFS update for 2.6.23 - revert a commit

Timothy Shimmin
2007-10-02T01:41:49Z

Eric Sandeen wrote:
> Tim Shimmin wrote:
>> Hi Linus,
>>
>> A problem has been found for the XFS commit b394e43e995d08821588a22561c6a71a63b4ff27
>> and it needs to be reverted.
>> It has the potential for worse corruption than what it is meant to fix.
>
>
> Whoops... that's what I get for picking it up too soon for fedora I guess!
>
> Any background on the newly-found problem, for those of us in the peanut
> gallery?
>
> Thanks,
>
> -Eric

Hi Eric,

Lachlan worked this problem so he can probably provide more details.
My understanding is that we were having a problem with the log replay
replaying newly allocated inodes (inodes from buffer items) over the top
of buffers which were actually more up-to-date than what was logged.
The code used a heuristic to determine if the buffer had been written
to for the inode (by checking on magic#, mode and gen# - not going to
comment on this).
Anyway, it comes down to either copying over the inode buf data or not
copying it over (doing or not doing the log replay).
The change could cause the buffer to be not overwritten in replay
where previously it would be.
We want this to not happen as part of the fix and doing the right thing
and not by mistake and failing to replay when we need it to be replayed.
I presume the latter is what is happening.
The symptoms of this is what Lachlan has discovered in QA on a debug
kernel and he can provide the details.

I believe this started from not logging the inode size changes
(as is consistent with the logging model) for performance reasons,
and so we can't rely on inode log items coming up on log replay
to fix things up.

BTW, we currently have 3 ways of logging an inode:
1. in an item buffer and marked as an inode
2. in an item buffer and not marked as an inode
3. in an inode item

and 3 places where they get replayed:
1. xlog_recover_do_inode_buffer - for di_next_unlinked pointer recovery
2. xlog_recover_do_reg_buffer - for newly allocated inode recovery
3. xlog_recover_do_inode_trans - for general inode recovery

The fix was in #2.

Ughh.

--Tim