[Ocfs2-tools-devel] [PATCH 07/10] fsck.ocfs2 now know inline data.

[Joel Becker]

On Fri, Jul 18, 2008 at 05:02:09PM +0800, Tao Ma wrote:
> You suggestion make me to think more of it. So how should we trust this 
> flag? As you have said, yes, there is a possibility that the INLINE_DATA_FL 
> is happened to be the corruption, then there is also one possibility that 
> ~INLINE_DATA_FL is also the corruption, what should we handle this also?

	Well, let's step back a second.  This is me reasoning about the
choices, so I'm sorry if it is verbose.
	First off, we should trust the contents of id2 less than the main
inode, because it could be any number of things.  If it is i_data,
it will look really weird when treated as i_list.  Or it could look
almost exactly like an i_list if the right data is there.  So we want to
be wary of decisions made on the contents of id2.  We can trust the main
inode fields more (i_size, i_clusters, i_flags, i_dyn_features) because
they are always in the same place for all inodes.
	Second, corruptions happen in two ways.  One, someone scribbles
all over the disk.  If that happens, it is unlikely the inode will be
very recognizable, and really there's probably already data loss.  Let's
not worry about that, we can only make it better.  Two, something flips a
bit or forgets to update a field.  That could be a code error or some
memory error (cosmic rays?).  That's most likely to affect one specific
byte or word.  So if two fields are "right" and one is "wrong", it's
more likely there was a single error than exactly two errors.  Does that
make sense?

>
> So now my thought is that take INLINE_DATA_FL, i_size, i_clusters as one 
> compound judgement. So if the 2 things happens to be right, we will regard 
> them as right.
> 1) if INLINE_DATA_FL is set:
>    a) if either i_size <= max_inline_data or i_clusters=0, we trust this 
> flag and clear the error i_size or i_clusters.
>    b) if both i_size > max_inline_data and i_clusters>0, clear the flag and 
> go on with check_el. The only problem is how should we handle the error in 
> check_el(any error in check_el will not make us reset INLINE_DATA_FL or if 
> there is any error, we will trust INLINE_DATA_FL and set it back?).

	In a normal running filesystem, INLINE_DATA_FL is kind of
superfluous.  Having i_clusters==0 says "inline data" all by itself.  I
mean, if i_size!=0 and i_clusters==0, the data *has* to be inline.
Where else would it be?  But we mark INLINE_DATA_FL to say "yes,
i_clusters==0 is OK when i_size!=0".
	So, if we have both INLINE_DATA_FL and i_clusters==0, we have
two places that say data must be inline.  I think we trust that.
	If the file has clusters (i_clusters!=0), our classic way of
saying "data is in clusters" is true.  But we also find INLINE_DATA_FL,
our explicit way of saying "data is inline".  The only way to break the
tie is with i_size.  This fits with the theory above that exactly one
error is way more likely than exactly two.
	So your scheme above fits this reasoning.  Trust the two that
match.  I think we ignore your concern at the end of (1b).  check_el()
will not reset INLINE_DATA_FL, it will just clean up broken i_list.  A
broken i_list could be data or just be broken, and there's no sane way
to tell the difference.

> 2) if INLINE_DATA_FL isn't set:
>    a) if both i_size <= max_inline_data and i_clusters=0, we set this flag.
>    b) if either i_size > max_inline_data or i_clusters>0, we trust the file 
> isn't inline. so go with check_el.

	If INLINE_DATA_FL isn't set, it is a little harder.  A file can
have i_size<=max_inline_data and still be valid if the filesystem was
formatted without inline data but tunefs.ocfs2 turned it on later.
	Your (2b) is easy.  i_clusters!=0 says "not inline" loud and
clear, and we trust all the fields.  i_size>max_inline_data strongly
suggests that there are clusters, so we can fixup i_clusters==0.
	But if isize<=max_inline_data and i_clusters=0, does that
mean inline, or a pre-tunefs file that just got i_clusters screwed up?
See, unlike above, we're not presuming two errors happened.  It could be
just one error (i_clusters cleared), and INLINE_DATA_FL was never set
because the file was created before inline data was turned on.
	So, what's the more likely error?  i_clusters cleared, or
INLINE_DATA_FL cleared?  Well, I bounced a few ideas in my head (see if
theres any unknown flags in i_dyn_features, add an I_DYN_VALID_FL to
ensure i_dyn_features isn't screwed up...), but in the end, I think we
just set INLINE_DATA_FL like you say in (2a).  Over time, a filesystem
with inline data enabled is far more likley to have been formatted that
way.  Thus, the only way you get i_size<=max_inline_data and
i_clusters==0 is with inline data.

There, I've just convinced myself of your scheme.  Mark? :-)

Joel


-- 

"There are some experiences in life which should not be demanded
 twice from any man, and one of them is listening to the Brahms Requiem."
        - George Bernard Shaw

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127