From: Linus Torvalds <torvalds@linux-foundation.org>
To: Jaegeuk Kim <jaegeuk@kernel.org>, Al Viro <viro@zeniv.linux.org.uk>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux F2FS Dev Mailing List
<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [GIT PULL] f2fs update for 6.8-rc1
Date: Thu, 11 Jan 2024 21:05:51 -0800 [thread overview]
Message-ID: <CAHk-=wgTbey3-RCz8ZpmTsMhUGf02YVV068k3OzrmOvJPowXfw@mail.gmail.com> (raw)
In-Reply-To: <ZaAzOgd3iWL0feTU@google.com>
On Thu, 11 Jan 2024 at 10:28, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git tags/f2fs-for-6.8-rc1
Hmm. I got a somewhat confusing conflict in f2fs_rename().
And honestly, I really don't know what the right resolution is. What I
ended up with was this:
if (old_is_dir) {
if (old_dir_entry)
f2fs_set_link(old_inode, old_dir_entry,
old_dir_page, new_dir);
else
f2fs_put_page(old_dir_page, 0);
f2fs_i_links_write(old_dir, false);
}
which seems to me to be the right thing as a resolution. But I note
that linux-next has something different, and it is because Al said in
https://lore.kernel.org/all/20231220013402.GW1674809@ZenIV/
that the resolution should just be
if (old_dir_entry)
f2fs_set_link(old_inode, old_dir_entry, old_dir_page, new_dir);
if (old_is_dir)
f2fs_i_links_write(old_dir, false);
instead.
Now, some of those differences are artificial - old_dir_entry can only
be set if old_is_dir is set, so the nesting difference is kind of a
red herring.
But I feel like that f2fs_put_page() is actually needed, or you end up
with a reference leak.
So despite the fact that Al is never wrong, I ended up going with my
gut, and kept my resolution that is different from linux-next.
End result: I'm now very leery of my merge. On the one hand, I think
it's right. On the other hand, the likelihood that Al is wrong is
pretty low.
So please double- and triple-check that merge, and please send in a
fix for it. Presumably with a comment along the lines of "Al was
right, don't try to overthink things".
Hubris. That's the word for thinking you know better than Al.
Linus
next prev parent reply other threads:[~2024-01-12 5:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-11 18:28 [GIT PULL] f2fs update for 6.8-rc1 Jaegeuk Kim
2024-01-12 5:05 ` Linus Torvalds [this message]
2024-01-12 7:12 ` Al Viro
2024-01-12 17:08 ` Jaegeuk Kim
2024-01-12 17:19 ` Jaegeuk Kim
2024-01-12 18:18 ` Linus Torvalds
2024-01-12 5:07 ` [f2fs-dev] " pr-tracker-bot
2024-01-16 19:02 ` patchwork-bot+f2fs
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='CAHk-=wgTbey3-RCz8ZpmTsMhUGf02YVV068k3OzrmOvJPowXfw@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/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).