Re: [PATCH 3/5] xfs_repair: fix dir refcount when '.' missing and dir is rebuilt

[Eric Sandeen <sandeen@sandeen.net>]

On 9/8/14 8:45 AM, Brian Foster wrote:
> On Sun, Sep 07, 2014 at 11:41:03AM -0500, Eric Sandeen wrote:
>> In phase 6's longform_dir2_entry_check, if we never
>> find a '.' entry we never add a reference to that entry;
>> if we subsequently rebuild it, '.' gets added, but
>> no ref to it is ever made.  This leads to Phase 7 doing
>> i.e.:
>>
>>    Phase 7 - verify and correct link counts...
>>    resetting inode 5184 nlinks from 2 to 1
>>
>> and the next run will do:
>>
>>    Phase 7 - verify and correct link counts...
>>    resetting inode 5184 nlinks from 1 to 2
>>
>> So if '.' was never found, but the directory got
>> rebuilt, manually add the ref for it.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>   repair/phase6.c |    6 ++++++
>>   1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/repair/phase6.c b/repair/phase6.c
>> index f13069f..cc36a9c 100644
>> --- a/repair/phase6.c
>> +++ b/repair/phase6.c
>> @@ -2288,6 +2288,12 @@ out_fix:
>>   			if (bplist[i])
>>   				libxfs_putbuf(bplist[i]);
>>   		longform_dir2_rebuild(mp, ino, ip, irec, ino_offset, hashtab);
>> +		/*
>> +		 * If we didn't find a dot, we never added a ref for it;
>> +		 * it's there now after the rebuild, so mark it as reached.
>> +		 */
>> +		if (*need_dot)
>> +			add_inode_ref(irec, ino_offset);
>
> So if I follow this correctly, we iterate through the dir, add each name
> to the hashtable and handle the inode reference count in the first
> longform_dir2_entry_check() loop. If something is wrong, we call
> longform_dir2_rebuild() to rebuild the dir from the hashtable of
> names/inodes. We may or may not have added a reference for dot at that
> point, and need_dot is set appropriately.
>
> This seems Ok, but where is the dot entry actually added? Hmm, I see
> that we handle dot in the longform_dir2_rebuild() loop by just skipping
> over it...

longform_dir2_rebuild calls this before the loop:

/*
  * Initialize a directory with its "." and ".." entries.
  */
int
xfs_dir_init()

But it doesn't actually create .; this creates a shortform dir,
and shortform dirs have no '.' - I guess the comment could use
an update.

In the loop we call xfs_dir_createname() for everything in the hash,
eventually things don't fit in shortform and we do xfs_dir2_sf_to_block,
which creates the dot:

         /*
          * Create entry for .
          */
         dep = xfs_dir3_data_dot_entry_p(mp, hdr);
         dep->inumber = cpu_to_be64(dp->i_ino);
         dep->namelen = 1;
         dep->name[0] = '.';

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs