From: Al Viro <viro@ZenIV.linux.org.uk>
To: Jeff Mahoney <jeffm@jeffm.io>
Cc: Roman Lebedev <lebedev.ri@gmail.com>,
David Howells <dhowells@redhat.com>,
linux-btrfs@vger.kernel.org, linux-unionfs@vger.kernel.org,
linux-fsdevel@vger.kernel.org, fstests@vger.kernel.org,
Filipe Manana <fdmanana@suse.com>
Subject: Re: kernel BUG when fsync'ing file in a overlayfs merged dir, located on btrfs
Date: Fri, 6 Nov 2015 03:18:45 +0000 [thread overview]
Message-ID: <20151106031845.GV22011@ZenIV.linux.org.uk> (raw)
In-Reply-To: <563C171F.30702@jeffm.io>
On Thu, Nov 05, 2015 at 09:57:35PM -0500, Jeff Mahoney wrote:
> So now file_operations callbacks can't assume that file->f_path.dentry
> belongs to the same file system that implements the callback. More than
> that, any code that could ultimately get a dentry that comes from an
> open file can't trust that it's from the same file system.
Use file_inode() for inode.
> This crash is due to this issue. Unlike xfs and ext2/3/4, we use
> file->f_path.dentry->d_inode to resolve the inode. Using file_inode()
> is an easy enough fix here, but we run into trouble later. We have
> logic in the btrfs fsync() call path (check_parent_dirs_for_sync) that
> walks back up the dentry chain examining the inode's last transaction
> and last unlink transaction to determine whether a full transaction
> commit is required. This obviously doesn't work if we're walking the
> overlayfs path instead. Regardless of any argument over whether that's
> doing the right thing, it's a pretty common pattern to assume that
> file->f_path.dentry comes from the same file system when using a
> file_operation. Is it intended that that assumption is no longer valid?
It's actually rare, and your example is a perfect demonstration of the
reasons why it is so rare. What's to protect btrfs_log_dentry_safe()
from racing with rename(2)? Sure, you do dget_parent(). Which protects
you from having one-time parent dentry freed under you. What it doesn't
do is making any promises about its relationship with your file.
next prev parent reply other threads:[~2015-11-06 3:18 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-30 19:57 kernel BUG when fsync'ing file in a overlayfs merged dir, located on btrfs Roman Lebedev
2015-09-30 19:57 ` Roman Lebedev
2015-09-30 19:57 ` [RFC PATCH] fstests: generic: Test that fsync works on file in overlayfs merged directory Roman Lebedev
2015-09-30 21:56 ` Dave Chinner
2015-09-30 22:07 ` Eric Sandeen
2015-11-06 2:57 ` kernel BUG when fsync'ing file in a overlayfs merged dir, located on btrfs Jeff Mahoney
2015-11-06 3:18 ` Al Viro [this message]
2015-11-06 4:03 ` Jeff Mahoney
2015-11-06 14:46 ` Jeff Mahoney
2016-03-24 15:20 ` Al Viro
2016-03-24 15:25 ` Al Viro
2016-03-24 15:31 ` Jeff Mahoney
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=20151106031845.GV22011@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=dhowells@redhat.com \
--cc=fdmanana@suse.com \
--cc=fstests@vger.kernel.org \
--cc=jeffm@jeffm.io \
--cc=lebedev.ri@gmail.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-unionfs@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 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.