On 2023/10/27 0:16, Jan Kara wrote:
Jaegeuk, Chao, any comment on this? It really looks like a filesystem corruption issue in f2fs when whiteouts are used...
Sorry for delay reply, I was busy handling product issues these days... Let me check this ASAP. Thanks,
Honza On Tue 17-10-23 06:50:40, Al Viro wrote:[f2fs folks Cc'd] There's something very odd in f2fs_rename(); this: f2fs_down_write(&F2FS_I(old_inode)->i_sem); if (!old_dir_entry || whiteout) file_lost_pino(old_inode); else /* adjust dir's i_pino to pass fsck check */ f2fs_i_pino_write(old_inode, new_dir->i_ino); f2fs_up_write(&F2FS_I(old_inode)->i_sem); and this: if (old_dir != new_dir && !whiteout) f2fs_set_link(old_inode, old_dir_entry, old_dir_page, new_dir); else f2fs_put_page(old_dir_page, 0); The latter really stinks, especially considering struct dentry *f2fs_get_parent(struct dentry *child) { struct page *page; unsigned long ino = f2fs_inode_by_name(d_inode(child), &dotdot_name, &page); if (!ino) { if (IS_ERR(page)) return ERR_CAST(page); return ERR_PTR(-ENOENT); } return d_obtain_alias(f2fs_iget(child->d_sb, ino)); } You want correct inumber in the ".." link. And cross-directory rename does move the source to new parent, even if you'd been asked to leave a whiteout in the old place. Why is that stuff conditional on whiteout? AFAICS, that went into the tree in the same commit that added RENAME_WHITEOUT support on f2fs, mentioning "For now, we just try to follow the way that xfs/ext4 use" in commit message. But ext4 does *NOT* do anything of that sort - at the time of that commit the relevant piece had been if (old.dir_bh) { retval = ext4_rename_dir_finish(handle, &old, new.dir->i_ino); and old.dir_bh is set by retval = ext4_rename_dir_prepare(handle, &old); a few lines prior, which is not conditional upon the whiteout. What am I missing there?
