Hey Eric, On Fri, Jun 14, 2013 at 11:15:41AM -0500, Eric Sandeen wrote: > On 6/14/13 11:09 AM, Ben Myers wrote: > > On Fri, Jun 14, 2013 at 10:13:06AM +1000, Dave Chinner wrote: > >> On Thu, Jun 13, 2013 at 05:09:03PM -0500, Ben Myers wrote: > >>> On Thu, Jun 13, 2013 at 12:08:27PM +1000, Dave Chinner wrote: > >>>> On Wed, Jun 12, 2013 at 08:04:41PM -0500, Ben Myers wrote: > >>>>> On Wed, Jun 12, 2013 at 12:19:06PM +1000, Dave Chinner wrote: > >>>>>> From: Dave Chinner <dchinner@xxxxxxxxxx> > >>>>>> > >>>>>> Unfortunately, we cannot guarantee that items logged multiple times > >>>>>> and replayed by log recovery do not take objects back in time. When > >>>>>> theya re taken back in time, the go into an intermediate state which > >>>>>> is corrupt, and hence verification that occurs on this intermediate > >>>>>> state causes log recovery to abort with a corruption shutdown. > >>>>>> > >>>>>> Instead of causing a shutdown and unmountable filesystem, don't > >>>>>> verify post-recovery items before they are written to disk. This is > >>>>>> less than optimal, but there is no way to detect this issue for > >>>>>> non-CRC filesystems If log recovery successfully completes, this > >>>>>> will be undone and the object will be consistent by subsequent > >>>>>> transactions that are replayed, so in most cases we don't need to > >>>>>> take drastic action. > >>>>>> > >>>>>> For CRC enabled filesystems, leave the verifiers in place - we need > >>>>>> to call them to recalculate the CRCs on the objects anyway. This > >>>>>> recovery problem canbe solved for such filesystems - we have a LSN > >>>>>> stamped in all metadata at writeback time that we can to determine > >>>>>> whether the item should be replayed or not. This is a separate piece > >>>>>> of work, so is not addressed by this patch. > >>>>> > >>>>> Is there a test case for this one? How are you reproducing this? > >>>> > >>>> The test case was Dave Jones running sysrq-b on a hung test machine. > >>>> The machine would occasionally end up with a corrupt home directory. > >>>> > >>>> http://oss.sgi.com/pipermail/xfs/2013-May/026759.html > >>>> > >>>> Analysis from a metdadump provided by Dave: > >>>> > >>>> http://oss.sgi.com/pipermail/xfs/2013-June/026965.html > >>>> > >>>> And Cai also appeared to be hitting this after a crash on 3.10-rc4, > >>>> as it's giving exactly the same "verifier failed during log recovery" > >>>> stack trace: > >>>> > >>>> http://oss.sgi.com/pipermail/xfs/2013-June/026889.html > >>> > >>> Thanks. It appears that the verifiers have found corruption due to a > >>> flaw in log recovery, and the fix you are proposing is to stop using > >>> them. If we do that, we'll have no way of detecting the corruption and > >>> will end up hanging users of older kernels out to dry. > >> > >> We've never detected it before, and it's causing regressions for > >> multiple people. We *can't fix it* because we can't detect the > >> situation sanely, and we are not leaving people with old kernels > >> hanging out to dry. The opposite is true: we are fucking over > >> current users by preventing log recovery on filesystems that will > >> recovery perfectly OK and have almost always recovered just fine in > >> the past. > >> > >>> I think your suggestion that non-debug systems could warn instead of > >>> fail is a good one, but removing the verifier altogether is > >>> inappropriate. > >> > >> Changing every single verifier in a non-trivial way is not something > >> I'm about to do for a -rc6 kernel. Removing the verifiers from log > >> recovery just reverts to the pre-3.8 situation, so is perfectly > >> acceptable short term solution while we do the more invasive verify > >> changes. > >> > >>> Can you make the metadump available? I need to understand this better > >>> before I can sign off. Also: Any idea how far back this one goes? > >> > >> No, I can't make the metadump available to you - it was provided > >> privately and not obfuscated and so you'd have to ask Dave for it. > > > > Dave (Jones), could you make the metadump available to me? I'd like to > > understand this a little bit better. I'm a bit uncomfortable with the > > proposition that we should corrupt silently in this case... > > Ben, isn't it the case that the corruption would only happen if > log replay failed for some reason (as has always been the case, > verifier or not), but with the verifier in place, it kills replay > even w/o other problems due to a logical problem with the > (recently added) verifiers? It seems like the verifier prevented corruption from hitting disk during log replay. It is enforcing a partial replay up to the point where the corruption occurred. Now you should be able to zero the log and the filesystem is not corrupted. > IOW - this seems like an actual functional regression due to the > addition of the verifier, and dchinner's patch gets us back > to the almost-always-fine state we were in prior to the change. Oh, the spin doctor is *in*! This isn't a logical problem with the verifier, it's a logical problem with log replay. We need to find a way for recovery to know whether a given transaction should be replayed. Fixing that is nontrivial. > As we're at -rc6, it seems quite reasonable to me as a quick > fix to just short-circuit it for now. If we're talking about a short term fix, that's fine. This should be conditional on CONFIG_XFS_DEBUG and marked as such. Long term, removing the verifiers is the wrong thing to do here. We need to fix the recovery bug and then remove this temporary workaround. > If you have time to analyze dave's metadump that's cool, but > this seems like something that really needs to be addressed > before 3.10 gets out the door. If this really is a day one bug then it's been out the door almost twenty years. And you want to hurry now? ;) > Whenever you're ready, you can also add: > > Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx> Sure. > (And dchinner, should this cc: stable for 3.9?) Looks like verifiers were added in 3.7. We should Cc stable. Thanks, Ben _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
