From: Eric Biggers <ebiggers3@gmail.com>
To: linux-btrfs@vger.kernel.org, Chris Mason <clm@fb.com>
Cc: linux-fsdevel@vger.kernel.org
Subject: d_instantiate() and unlock_new_inode() order in btrfs_mkdir()
Date: Wed, 18 Apr 2018 17:00:29 -0700 [thread overview]
Message-ID: <20180419000029.GA133757@gmail.com> (raw)
Hi Chris and other btrfs folks,
btrfs_mkdir() calls d_instantiate() before unlock_new_inode(), which is wrong
because it exposes the inode to lookups before it's been fully initialized.
Most filesystems get it right, but f2fs and btrfs don't. I sent a f2fs patch
(https://marc.info/?l=linux-fsdevel&m=152409178431350) and was going to send a
btrfs patch too, but in btrfs_mkdir() there is actually a comment claiming that
the existing order is intentional:
d_instantiate(dentry, inode);
/*
* mkdir is special. We're unlocking after we call d_instantiate
* to avoid a race with nfsd calling d_instantiate.
*/
unlock_new_inode(inode);
Unfortunately, I cannot find what it is refering to. The comment was added by
commit b0d5d10f41a0 ("Btrfs: use insert_inode_locked4 for inode creation").
Chris, do you remember exactly what you had in mind when you wrote this?
And in case anyone wants it, here's a reproducer for the deadlock caused by the
current code that calls d_instantiate() before unlock_new_inode(). Note: it
needs CONFIG_DEBUG_LOCK_ALLOC=y.
#include <sys/stat.h>
#include <unistd.h>
int main()
{
struct stat stbuf;
if (fork() == 0) {
for (;;)
stat("dir/file", &stbuf);
} else {
for (;;) {
mkdir("dir", 0777);
stat("dir/file", &stbuf);
rmdir("dir");
}
}
}
next reply other threads:[~2018-04-19 0:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-19 0:00 Eric Biggers [this message]
2018-04-19 0:06 ` d_instantiate() and unlock_new_inode() order in btrfs_mkdir() Al Viro
2018-04-19 0:15 ` Al Viro
2018-04-19 0:54 ` Eric Biggers
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=20180419000029.GA133757@gmail.com \
--to=ebiggers3@gmail.com \
--cc=clm@fb.com \
--cc=linux-btrfs@vger.kernel.org \
--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