Re: [ndctl PATCH] ndctl, check: Add a sigbus handler to detect metadata corruption

From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>,
	"Rudoff, Andy" <andy.rudoff@intel.com>
Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: [ndctl PATCH] ndctl, check: Add a sigbus handler to detect metadata corruption
Date: Fri, 14 Apr 2017 19:40:22 +0000	[thread overview]
Message-ID: <1492198820.1657.3.camel@intel.com> (raw)
In-Reply-To: <D56299A3-A5A4-4B49-AFC6-5F03118F1F85@intel.com>

On Fri, 2017-04-14 at 19:26 +0000, Rudoff, Andy wrote:
>     On Fri, Apr 14, 2017 at 12:00 PM, Verma, Vishal L
[...]
>     
> Yeah, I thought about this a little more too.  Here’s what I think
> should happen:
> 
> Without --repair, I agree with this patch, although the error message
> might
> tell the poor user that running with --repair is the next step if
> they want to
> try to fix it (see below).
> 
> With --repair, the repair code should try to replace the poison with
> repaired metadata, clearing the appropriate error bits when done.
> 
> For example, if the poison is in the map, the repair code should
> figure
> out which entries have no map entry, clear the poison, and update the
> map, printing a strong warning that the blocks were repaired but may
> not be in the right place any more.  

If we hit a known badblock, that is 512B worth of map entries (128).
Should we really (almost certainly) scramble 64 blocks? :)
If it is a latent error, it will still be at least a cache line worth
of map entries, i.e. 16.

If an error is in the log, then in the badblock case, we lose both log
and log' for four lanes. Which means we can't tell is one of those 4
entries needed a map update, leaving a potential corruption window
open.

> If the poison is in something like
> the arena info block, a replacement can be constructed from the
> backup info.  The point is that the tool shouldn’t die on poison, it
> should be the way you fix it and get rid of the poison.

This might be the only case where we can clear the poison
successfully..

> 
> Finally, the kernel should be setting and honoring the error bits,
> both
> in map entries and arena info blocks.  When poison is discovered in
> metadata, the arena info block error state should be set and that
> should not disallow getting to any available data as you stated, but
> should instead make that arena read-only.  

Agreed about the error bit and read-only, I will look at that.

> The only way to get out
> of read-only mode for the arena should be to run the check tool
> with --repair and all the stuff I said above then happens.
>  
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm