xfs
[Top] [All Lists]

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

To: Andy Poling <andy@xxxxxxxxxxx>
Subject: Re: Wrapped journal record corruption on read at recovery - patch attached (was Re: XFS corruption with failover)
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 13 Oct 2009 19:33:24 -0400
Cc: xfs@xxxxxxxxxxx, John Quigley <jquigley@xxxxxxxxxxxxxx>
In-reply-to: <alpine.DEB.2.00.0910131342440.11546@andydesk>
References: <mailman.0.1255458988.141519.xfs@xxxxxxxxxxx> <alpine.DEB.2.00.0910131342440.11546@andydesk>
User-agent: Mutt/1.5.19 (2009-01-05)
Thanks Andy,

after starting over the code for over half an hour I think both
your analysis and the patch are correct.

-                               bufaddr = XFS_BUF_PTR(hbp);
+                               if (offset)
+                                       bufaddr = offset;
+                               else
+                                       bufaddr = XFS_BUF_PTR(hbp);

However the way this is written is a bit confusing.  Probably less
confusing than some of the surrounding code,  but it adds up.

We calculate the offset for one case above, and then after we're done
with the reading that never touches offset we calculate it if we don't
have it.

So this at very least needs a big comment describing it, or even better
moving up the xlog_align call so that we can simply always use
offset directly, which could also get rid of the now superflous bufaddr
variable by always using offset.

Exactly the same also applies for the second use.

Anyway, thanks a lot again and if you can fix this up I'd welcome it,
otherwise I'll do it once I get some time.

I suspect we might even be able to come up with a small enough testcase
for this for xfsqa, we just need to make sure logs are aligned and then
find a good enough heuristic to reproduce log wrapping.

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