From: ebiederm@xmission.com (Eric W. Biederman)
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Dave Jones <davej@redhat.com>,
Linux Kernel <linux-kernel@vger.kernel.org>,
Miklos Szeredi <mszeredi@suse.cz>, Jan Kara <jack@suse.cz>,
Peter Zijlstra <peterz@infradead.org>,
linux-fsdevel@vger.kernel.org,
"J. Bruce Fields" <bfields@redhat.com>,
Sage Weil <sage@newdream.net>
Subject: Re: processes hung after sys_renameat, and 'missing' processes
Date: Fri, 08 Jun 2012 00:54:00 -0700 [thread overview]
Message-ID: <87pq9a9r3b.fsf@xmission.com> (raw)
In-Reply-To: <20120608054838.GO30000@ZenIV.linux.org.uk> (Al Viro's message of "Fri, 8 Jun 2012 06:48:38 +0100")
Al Viro <viro@ZenIV.linux.org.uk> writes:
> On Thu, Jun 07, 2012 at 10:25:46PM -0700, Eric W. Biederman wrote:
>
>> I am still learly of d_materialise_unique as it allows to create alias's
>> on non-directories. It isn't a functional problem as d_revalidate will
>> catch the issue and make it look like we have a unlink/link pair instead
>> of a proper rename. However since it is possible I would like to aim
>> for the higher quality of implemntation and use show renames as renames.
>
> ???
>
> Please, explain. link/unlink pair in which sense?
In the sense that if we don't use d_move. A rename will look to
userspace like a pair of sys_link and sys_unlink operations.
If I happen to have a file open with the old name and the dentry passes
through d_drop. The /proc/self/fd/N will show the filename as
"...(deleted)".
And in every other way I can think of that is userspace visible this
will look like a pair of link and unlink operations.
> I don't see what kind of *notify hookup do you have in mind. Anything that
> treats "dentry failed revalidation or got evicted by memory pressure" as
> "unlink" is completely nuts, IMO.
In this case much as it might be convinient to have a *notify report,
what I was thinking of were the much simpler userspace visible aspects,
like what /proc/self/fd/N symlinks report.
In the little corner case user visible details the current state of vfs
support for distributed filesystems looks nuts to me, especially where
we can't apply an appropriate d_move onto a renamed dentry.
The fact that open files, open directories and mount points pin dentries
in memory cause interesting challenges for keeping the local vfs state
in sync with the state of a remote filesystem.
What I would love to be able to do is to replay some kind of journal
that reports what happened to the filesystem outside of the linux vfs
onto the linux vfs so that we can get a more accurate picture of what
really happened to the filesystem. Which should allow *notify and the
like to actually work. Would make the /proc/self/fd/* symlinks more
useful, and make allow files that are mount points to be renamed.
But ultimately the change in semantics bugs me. Using d_move less
often feels user visible and because d_materialise_unique because it
does not handle renames of files feels like a lurking maintenance bomb
for sysfs.
Especially since renames on files with mount points on them should be
treated differently from normal files.
Speaking of I just found a small unhandled case in __d_unalias. We need
to deny renaming of mount points.
Eric
From: "Eric W. Biederman" <ebiederm@xmission.com>
Subject: dcache: Deny renaming via __d_unalias dentries of mountpoints
Make __d_unalias match vfs_rename_dir and vfs_rename_other and don't
allow renaming mount points.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
diff --git a/fs/dcache.c b/fs/dcache.c
index 85c9e2b..d236722 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2380,14 +2380,17 @@ static struct dentry *__d_unalias(struct inode *inode,
struct dentry *dentry, struct dentry *alias)
{
struct mutex *m1 = NULL, *m2 = NULL;
- struct dentry *ret;
+ struct dentry *ret = ERR_PTR(-EBUSY);
+
+ /* Linux does not rename mount points */
+ if (d_mountpoint(alias))
+ goto out_err;
/* If alias and dentry share a parent, then no extra locks required */
if (alias->d_parent == dentry->d_parent)
goto out_unalias;
/* See lock_rename() */
- ret = ERR_PTR(-EBUSY);
if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex))
goto out_err;
m1 = &dentry->d_sb->s_vfs_rename_mutex;
next prev parent reply other threads:[~2012-06-08 7:54 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-03 22:36 processes hung after sys_renameat, and 'missing' processes Dave Jones
2012-06-03 22:51 ` Dave Jones
2012-06-03 23:07 ` Linus Torvalds
2012-06-03 23:17 ` Al Viro
2012-06-03 23:28 ` Al Viro
2012-06-03 23:40 ` Al Viro
2012-06-03 23:59 ` Al Viro
2012-06-04 0:07 ` Dave Jones
2012-06-06 19:42 ` Dave Jones
2012-06-06 22:38 ` Linus Torvalds
2012-06-06 23:00 ` Dave Jones
2012-06-06 23:31 ` Linus Torvalds
2012-06-06 23:54 ` Al Viro
2012-06-07 0:29 ` Dave Jones
2012-06-07 0:40 ` Al Viro
2012-06-07 0:42 ` Linus Torvalds
2012-06-07 1:19 ` Dave Jones
2012-06-07 1:29 ` Al Viro
2012-06-07 1:31 ` Dave Jones
2012-06-07 1:31 ` Al Viro
2012-06-07 1:42 ` Dave Jones
2012-06-07 1:45 ` Linus Torvalds
2012-06-07 1:54 ` Al Viro
2012-06-07 2:08 ` Dave Jones
2012-06-07 19:36 ` Al Viro
2012-06-07 20:43 ` Sage Weil
2012-06-07 23:12 ` Eric W. Biederman
2012-06-07 23:39 ` Al Viro
2012-06-07 23:57 ` Linus Torvalds
2012-06-08 0:36 ` Al Viro
2012-06-08 0:42 ` Linus Torvalds
2012-06-08 0:59 ` Al Viro
2012-06-08 5:25 ` Eric W. Biederman
2012-06-08 5:48 ` Al Viro
2012-06-08 7:54 ` Eric W. Biederman [this message]
2012-06-08 20:20 ` Al Viro
2012-06-08 2:08 ` Eric W. Biederman
2012-06-08 2:37 ` Al Viro
2012-06-08 2:18 ` Al Viro
2012-06-08 16:22 ` J. Bruce Fields
2012-06-08 17:44 ` Linus Torvalds
2012-06-11 12:17 ` J. Bruce Fields
2012-06-07 1:40 ` Linus Torvalds
2012-06-07 0:35 ` Linus Torvalds
2012-06-07 10:26 ` Peter Zijlstra
2012-06-07 15:30 ` Linus Torvalds
2012-06-08 7:31 ` Peter Zijlstra
2012-06-08 14:38 ` Dave Jones
2012-06-08 14:51 ` Peter Zijlstra
2012-06-08 15:01 ` Dave Jones
2012-06-08 15:11 ` Peter Zijlstra
2012-06-08 15:21 ` Dave Jones
2012-06-08 14:46 ` J. Bruce Fields
2012-06-08 15:08 ` Peter Zijlstra
2012-06-11 12:17 ` J. Bruce Fields
2012-06-04 0:00 ` Dave Jones
2012-06-04 0:16 ` Linus Torvalds
2012-06-04 0:20 ` Al Viro
2012-06-04 9:35 ` Peter Zijlstra
2012-06-04 9:29 ` Peter Zijlstra
2012-06-04 10:49 ` Peter Zijlstra
2012-06-07 0:13 ` Dave Jones
-- strict thread matches above, loose matches on Subject: below --
2012-06-07 7:07 Miklos Szeredi
2012-06-07 15:44 ` Linus Torvalds
2012-06-11 16:02 ` Miklos Szeredi
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=87pq9a9r3b.fsf@xmission.com \
--to=ebiederm@xmission.com \
--cc=bfields@redhat.com \
--cc=davej@redhat.com \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mszeredi@suse.cz \
--cc=peterz@infradead.org \
--cc=sage@newdream.net \
--cc=torvalds@linux-foundation.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