On Mon, Feb 25, 2013 at 12:39:21PM +0400, Dmitry Monakhov wrote:
> On Sun, 24 Feb 2013 22:58:37 +0800, Zheng Liu <gnehzuil.liu@xxxxxxxxx> wrote:
> > Hi Dmirty,
> >
> > On Fri, Feb 22, 2013 at 09:17:57PM +0400, Dmitry Monakhov wrote:
> > [snip]
> > > From 65c5fc212b1c684c76899c6e5e1f24d88550c6fc Mon Sep 17 00:00:00 2001
> > > From: Dmitry Monakhov <dmonakhov@xxxxxxxxxx>
> > > Date: Fri, 22 Feb 2013 20:55:52 +0400
> > > Subject: [PATCH] ext4 add sanity ext4_es_lookup_extent
> > >
> > > This patch does very simple thing: it recheck result returned from
> > > ext4_es_lookup_extent() by comparing it old-good lookup via
> > > ext4_{ind,ext}_map_blocks() under i_data_sem
> > >
> > > Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx>
> >
> > I try to apply the following patch in my tree, but I realize that it
> > seems that we couldn't add this sanity check in ext4_map_blocks and
> > ext4_da_map_blocks now. The reason is that when we try to initialize an
> > unwritten extent this extent could be zeroed out. But we could know it
> > in *_map_blocks, and status tree couldn't be updated. So we will hit a
> > BUG_ON(1) after added this sanity check.
> This means that extent-status tree should be updated once zeroout
> happen.
>
> >
> > I agree with you that we need to add self-testing infrastructre after
> > applied status tree patch series. But it seems that we need to mix
> > updating status tree code with extent tree code. IMHO it is too
> > complicated. Any thought?
> Many developers have invested significant amount of man-hours in to
> ext4's delay allocation state machine. It is now known as stable and reliable
> So it reasonable to use it as a primary for self-testing of
> extent-status tree. If es_cache != real_state this is means that we have
> forget to update es_cache.
> For example when extents was introduced it have many self testing things
> such as:
> 1) CHECK_BINSEARCH__: Which recheck binarry search result.
>
> 2) AGGRESSIVE_TEST : Which force deep extent tree constructions and
> allow to cover unusual branches.
> 3) DOUBLE_CHECK : Recheck block alloc bitmap one more time
Thanks for your reminder. I will add it.
>
> I strongly believe in testing. Ext4 is production-grade filesystem
> so we can not break it in the name of new features, unless we are sure
> that features are safe and valuable, that's why investments in
> self-testing infrastructure for ES should have very high priority.
> But off course Theodore's decision whenever feature is looks stable
> enough to get ready go upstream.
Yes, I agree with you that we shouldn't break a production-grade file
system. Certainly Ted has the final decision.
Regards,
- Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
