Re: [f2fs-dev] [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction

[Namjae J. <lin...@gm...>]

2012/12/10, Jaegeuk Kim <jae...@sa...>:
> 2012-12-10 (월), 14:25 +0900, Namjae Jeon:
>> 2012/12/10, Jaegeuk Kim <jae...@sa...>:
>> > 2012-12-10 (월), 12:40 +0900, Namjae Jeon:
>> >> 2012/12/10, Jaegeuk Kim <jae...@sa...>:
>> >> > 2012-12-08 (토), 14:55 +0900, Namjae Jeon:
>> >> >> From: Namjae Jeon <nam...@sa...>
>> >> >>
>> >> >> Test Case:
>> >> >> [NFS Client]
>> >> >> ls -lR .
>> >> >>
>> >> >> [NFS Server]
>> >> >> while [ 1 ]
>> >> >> do
>> >> >> echo 3 > /proc/sys/vm/drop_caches
>> >> >> done
>> >> >>
>> >> >> Error: "No such file or directory"
>> >> >>
>> >> >> When cache is dropped at the server, it results in lookup failure
>> >> >> at
>> >> >> the
>> >> >> NFS client. Even though the file exists. Looking at the code to
>> >> >> rebuild
>> >> >> the inode in case of cache eviction. It tries to initiate a lookup
>> >> >> operation
>> >> >> for ".." to get the parent information using the on-disk inode
>> >> >> number.
>> >> >>
>> >> >
>> >> > Could you describe why this patch resolves that bug?
>> >> > Before applying this, we need to figure out why that bug is
>> >> > occurred.
>> >> > IMO, from the viewpoint of functionality, ".." resolution should
>> >> > work
>> >> > too.
>> >> dotdot entry of f2fs is stored when creating only directory not
>> >> regular file. Am I correct ?
>> >
>> > Yep.
>> >
>> >> So when the parent of file was evicted, I thought we could not get
>> >> parent inode number of file thoughout dotdot entry.
>> >
>> > What do you mean the parent of file? Isn't it a directory?
>> >
>> >> And f2fs inode is having parent inode number unlike other fs. so I
>> >> think we can use this special thing by storing f2fs_inode_info.
>> >
>> > f2fs stores a dotdot dentry *likewise* other fs.
>> > The pino in f2fs_inode is specially added for POR intentionally.
>> >
>> > Still I cannot imagine the bug scenario.
>> When we observed the issue in NFS operations. We found that the issues
>> occurs while trying to “reconnect” with parent ? and shows “No such
>> file or directory” for directory paths.
>
> Again, you didn't describe why this bug is occurred.
> Please, analyze the bug scenario first.
Okay, I will look into more.
>
>>
>> As a matter of fact - we were also checking the on-disk layout for the
>> f2fs_inode. We found that since, F2FS inode is keeping a reference to
>> the parent-inode within its on-disk information. So, we should use the
>> same information to reconstruct the parent in case of eviction.
>>
>> The benefit is:
>> It also saves the lookup() to be performed for “..” in the directory
>> entry block while trying to reconnect with the parent. Instead we can
>> directly use the parent inode number in the inode and generate from
>> that point itself.  Even though the get_parent() code was made similar
>> to other filesystem which generally do not have reference to the
>> parent inode -  so they need to perform a lookup for “..”, but in this
>> case we sincerely thought we can get rid of that method.
>> So, we did not tried to figure out what could be possible solution in
>> the current scenario.
>> As per your reference, parent inode number for on-disk inode is
>> introduced for POR intentionally. So, we should not use information
>> for any reconstruction?
>
> Of course, we can enhance the directory operations by using the "pino".
> But what I concern is how to resolve the bug by this enhancement patch.
> This is not a bug fix patch. Isn't it?
Yes, It is not bug fix. this is new method using f2fs's special thing.
Agreed, I will analyse why this bug occur first. let's discuss again
whether we fix bug or use new method.

And please check the rest of patches except this patch.

Thanks.
>
> Thanks,
>
> --
> Jaegeuk Kim
> Samsung
>