xfs
[Top] [All Lists]

Re: Wrapped journal record corruption on read at recovery - patch attach

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: Wrapped journal record corruption on read at recovery - patch attached (was Re: XFS corruption with failover)
From: Andy Poling <andy@xxxxxxxxxxx>
Date: Thu, 15 Oct 2009 19:36:33 -0500 (CDT)
Cc: John Quigley <jquigley@xxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.0910141053150.5504@andydesk>
References: <mailman.0.1255458988.141519.xfs@xxxxxxxxxxx> <alpine.DEB.2.00.0910131342440.11546@andydesk> <20091013233324.GA28942@xxxxxxxxxxxxx> <alpine.DEB.2.00.0910131853200.11546@andydesk> <20091014132030.GB15001@xxxxxxxxxxxxx> <alpine.DEB.2.00.0910141053150.5504@andydesk>
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
On Wed, 14 Oct 2009, Christoph Hellwig wrote:
It seems like the more elegant approach would be to set offset before the
first read, and then update it if the first read takes place (in case it was
unaligned).  That also gets rid of bufaddr, and seems like it might read
better.

Yeah.  Note that in Linux 2.6.29 I did some changes in that are to
take the read and offset calculation into a common helper

Looking at the improved code since 2.6.29, and assuming that the beginning of
the log will always be aligned, I have come up with the attached patch against
2.6.31.4.  I think it's cleaner, and it passes my tests.

The bufaddr variable is no longer needed.  I removed the xlog_align() after
the second read for both headers and data since the second read will always be
aligned (even if there was no first read) since it is always at the beginning
of the log.

It sets offset to the beginning of the buffer with XFS_BUF_PTR() before the
first read, and then your improved xlog_bread() will update the offset if the
first read had to be re-aligned.

What do you think?

-Andy

It ain't what you don't know that gets you into trouble.
It's what you know for sure that just ain't so. - Mark Twain

Attachment: xfs_log_recover.pat
Description: xfs_log_recover.pat

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