linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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?

       reply	other threads:[~2023-10-17  5:50 UTC|newest]

Thread overview: 4+ 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-26 16:16                     ` [RFC] weirdness in f2fs_rename() with RENAME_WHITEOUT Jan Kara
2023-10-26 16:44                       ` Jaegeuk Kim
2023-11-07 13:55                       ` 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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).