[Ocfs2-tools-devel] [PATCH RESEND 3/3] fsck.ocfs2: support append direct io incompat feature

[Joseph Qi]

Hi Eric,

On 2016/3/21 18:05, Eric Ren wrote:
> Hi Joseph,
> ...
>>> Support to truncate direct io orphan entry in fsck.ocfs2.
>>>
>>> Signed-off-by: Joseph Qi<joseph.qi at huawei.com>
> Acked-by: Eric Ren <zren at suse.com>
>>> ---
>>>   fsck.ocfs2/pass4.c  |  9 +++++++++
>>>   libocfs2/truncate.c | 12 ++++++++++++
>>>   2 files changed, 21 insertions(+)
> ....
>>> It make sense to skip _delte_inode() stuff. But I cannot understand very well what the code
>>> after out_check: is supposed to do? Looks it collects info about link count and inode refs,
>>> and then do check_link_counts() where inode would be moved to lost+found dir if no dirent
>>> refer to it. So is it possible to lose the inode unexpectedly?
>>>
>> Normally it won't. Otherwise I think it is a bug for inode link count.
>>> Could you explain a bit more the reason why we should add this lines of code here?
>>> Because fix_dups_func()
>>>            ->clone_one_inode() or delete_one_inode()
>>>         ->1355         ret = ocfs2_truncate_full(ost->ost_fs, ino, 0,
>>>           1356                                   pass1d_free_clusters, fd);
>>>
>>> As you can see the 3th parameter(new_i_size) is 0, so it won't hit go-out. Why do we
>>> need this line:
>>>     new_i_size = ci->ci_inode->i_size;
>>> ?
>>>
>> Though replay_orphan_iterate->ocfs2_truncate passes new_i_size with 0,
>> but for dio entry, the new_i_size will be re-assigned to the current
>> inode i_size. As I explained before, this is to reclaim allocated
>> clusters.
>> In other words, clusters are allocated, but inode i_size has
>> not been updated and written to disk. So we have to make the inode go
>> back to the former status after fsck.
> Thanks for your explainations. I can understand more. But considering my very limited knowledge about this,
> I think I can just give a acked-by, hope other experts can also review this;-)
> 
> Btw, reflink feature also uses orphan directory to do the similar thing. I'm just wondering did reflink take this into consideration,
> and can both of them has a common handling in fsck?
> 
I'm afraid I won't consider reflink as well in this patch, since my
intent is to support append direct io:)

Thanks,
Joseph

> Eric
> 
> .
>