----- Original Message -----
> ----- Original Message -----
> > Hi Bob,
> > 
> > On 22/01/15 20:41, Bob Peterson wrote:
> > > Hi,
> > >
> > > This patch changes the old block_map structure for fsck.gfs2 to the
> > > simpler bitmap structure so that we have a 1:1 correspondence. This
> > > was done to reduce memory requirements of fsck.gfs2.
> > 
> > I'm curious as to whether we're losing any useful information here. I
> > see an extra bread() for BLKST_USED blocks in check_data, is that the
> > main performance compromise, and how severe is it?
> > 
> > Cheers,
> > Andy
> 
> Hi,
> 
> That's exactly where things get a bit sticky. It used to keep track of
> the various GFS2_METATYPE_* block types. For the most part, dinodes were
> straightforward because we only needed to distinguish between directories
> and non-directories. Free space is pretty straightforward too, with only
> a few corner cases where things were marked a "bad" type. Unlinked also
> wasn't a big problem. The problem comes with blocks marked "Data."
> 
> In GFS1, "data" blocks were data blocks. In GFS2, a data block could be
> data or metadata. In some cases we don't care because the height will
> tell us if it's real data. But things get sticky when we need to
> distinguish between different types of non-dinode metadata in fsck.
> For example, what if a dinode has "indirect blocks" that are not
> really indirect blocks, but say, an extended attribute block? The old
> fsck could distinguish between them pretty easily because of their
> unique types. Going by just the bitmap alone is ambiguous, so sometimes
> we need to know more details, especially in cases where we have a block
> that's referenced multiple times: it's much more likely that one of the
> references is for the wrong type, and in that case, we do the extra
> read. This shouldn't impact performance, except for duplicate references
> (which is rare) and needs to be done.
> 
> There are probably cases where fsck is now making assumptions about the
> previously processed block type for indirect blocks, so in that
> respect, the information is lost. However, I've tried to minimize the
> risk, and rigorous testing has shown that it works in many very difficult
> situations. You can tell my testing has been good, since I've posted
> so many bug fix patches recently that I've uncovered. :)
> 
> In that respect, this new fsck is better than the previous.
> 
> Still, there may very well be cases where the lost information causes
> an unforeseen problem. I'm not sure we can do anything about that but
> test it thoroughly.
> 
> Maybe I'll try to dummy up some potential problems (like the one I gave
> above with indirect versus extended attribute) to see how well it does.
> I suppose I could try to dummy up every permutation. That is likely to
> uncover even more problems, most of which already exist in today's fsck.
> 
> Regards,
> 
> Bob Peterson
> Red Hat File Systems

Hi,

As promised, I dummied up some file systems to intentionally have the wrong
metadata block types. I tried these scenarios:

1. File that had an extended attribute block that was really an indirect block.
2. File that had an indirect block that was really an extended attribute block.
3. Directory that had an extended attribute block that was really a leaf block.
4. Directory that had a leaf block that was really an extended attribute block.
5. File that had an extended attribute block that was really an "ED" block.
6. File whose indirect pointer points to a leaf block
7. Directory whose leaf block pointers all point to an indirect block

In all these scenarios, the fsck.gfs2 with my patch made the same decisions
and produced relatively the same output as the stock fsck.gfs2. So I think
it's safe to push this patch now, unless anyone has an objection or has
specific tests they'd like me to try.

Regards,

Bob Peterson
Red Hat File Systems

Reply via email to