From: Al Viro <viro@zeniv.linux.org.uk> To: linux-f2fs-devel@lists.sourceforge.net Cc: linux-fsdevel@vger.kernel.org, Jan Kara <jack@suse.cz>, Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org> Subject: [RFC] weirdness in f2fs_rename() with RENAME_WHITEOUT Date: Tue, 17 Oct 2023 06:50:40 +0100 [thread overview] Message-ID: <20231017055040.GN800259@ZenIV> (raw) In-Reply-To: <20231012191551.GZ800259@ZenIV> [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?
WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@zeniv.linux.org.uk> To: linux-f2fs-devel@lists.sourceforge.net Cc: linux-fsdevel@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>, Jan Kara <jack@suse.cz> Subject: [f2fs-dev] [RFC] weirdness in f2fs_rename() with RENAME_WHITEOUT Date: Tue, 17 Oct 2023 06:50:40 +0100 [thread overview] Message-ID: <20231017055040.GN800259@ZenIV> (raw) In-Reply-To: <20231012191551.GZ800259@ZenIV> [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? _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next parent reply other threads:[~2023-10-17 5:50 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20231011195620.GW800259@ZenIV> [not found] ` <20231011203412.GA85476@ZenIV> [not found] ` <CAHk-=wjSbompMCgMwR2-MB59QDB+OZ7Ohp878QoDc9o7z4pbNg@mail.gmail.com> [not found] ` <20231011215138.GX800259@ZenIV> [not found] ` <20231011230105.GA92231@ZenIV> [not found] ` <CAHfrynNbfPtAjY4Y7N0cyWyH35dyF_BcpfR58ASCCC7=-TfSFw@mail.gmail.com> [not found] ` <20231012050209.GY800259@ZenIV> [not found] ` <20231012103157.mmn6sv4e6hfrqkai@quack3> [not found] ` <20231012145758.yopnkhijksae5akp@quack3> [not found] ` <20231012191551.GZ800259@ZenIV> 2023-10-17 5:50 ` Al Viro [this message] 2023-10-17 5:50 ` [f2fs-dev] [RFC] weirdness in f2fs_rename() with RENAME_WHITEOUT Al Viro 2023-10-26 16:16 ` Jan Kara 2023-10-26 16:16 ` [f2fs-dev] " Jan Kara 2023-10-26 16:44 ` Jaegeuk Kim 2023-10-26 16:44 ` [f2fs-dev] " Jaegeuk Kim 2023-11-07 13:55 ` Chao Yu 2023-11-07 13:55 ` [f2fs-dev] " Chao Yu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231017055040.GN800259@ZenIV \ --to=viro@zeniv.linux.org.uk \ --cc=chao@kernel.org \ --cc=jack@suse.cz \ --cc=jaegeuk@kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fsdevel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.