After commit "fsnotify: annotate directory entry modification events"
fsnotify_nameremove() requires that dentry->d_name and dentry->d_parent
are stable.
fsnotify_nameremove() is usually called from d_delete() from filesystem
->unlink(), ->rmdir(), ->rename() ops under parent vfs inode lock, so
said commit adds a WARN_ON() if parent inode is not locked.
fsnotify_nameremove() is also called from nfs_complete_sillyrename(),
also under parent vfs inode lock.
While the requirement of stable d_name and d_parent seems to be true
for all in-tree callers, the WARN_ON() was not true is some cases.
The following functions call d_delete() from pseudo filesystems
without parent inode lock, but those filesystems have no rename()
op, so this is not a concern: ffs_epfiles_destroy(),
rpc_gssd_dummy_depopulate(), devpts_pty_kill(), efivarfs_file_write(),
__ns_get_path(). In the last case, the dentry is a pseudo dentry
that should not generate any event.
The following functions call d_delete() from clustered filesystems
without parent inode lock, but those filesystems use clustered locks
to synchronize d_delete() with d_move():
ceph_fill_trace(), ocfs2_dentry_convert_worker().
Relax the WARN_ON() to accommodate for these two special cases:
- fs with no dir rename() op
- fs that does d_move() instead of vfs
Move the WARN_ON() into fsnotify_nameremove() where it matters.
Cc: Miklos Szeredi <miklos@xxxxxxxxxx>
Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
---
Jan,
I have completed my research about d_delete() callers and my conclusions
are summarized in this patch.
Should you decide to accept the result, you would probably want to
squash this with patch 1.
I am guessing you would feel more cozy with dget_parent() and
take_dentry_name_snapshot(), but let's see if we can get an ack on
my conclusions from either Al or Miklos first.
Thanks,
Amir.
include/linux/fsnotify.h | 46 ++++++++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 14e1f43c38c9..d20c92c3cdeb 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -22,23 +22,10 @@
*
* Unlike fsnotify_parent(), the event will be reported regardless of the
* FS_EVENT_ON_CHILD mask on the parent inode.
- *
- * When called with NULL @dir (from fsnotify_nameremove()), the dentry parent
- * inode is used as the inode to report the event to.
*/
static inline int fsnotify_dirent(struct inode *dir, struct dentry *dentry,
__u32 mask)
{
- if (!dir)
- dir = d_inode(dentry->d_parent);
-
- /*
- * This helper assumes d_parent and d_name are stable. It must be true
- * when called from fsnotify_create()/fsnotify_mkdir(). Less sure about
- * all callers that get here from d_delete() => fsnotify_nameremove().
- */
- WARN_ON(!inode_is_locked(dir));
-
return fsnotify(dir, mask, d_inode(dentry), FSNOTIFY_EVENT_INODE,
dentry->d_name.name, 0);
}
@@ -158,15 +145,46 @@ static inline void fsnotify_vfsmount_delete(struct vfsmount *mnt)
/*
* fsnotify_nameremove - a filename was removed from a directory
+ *
+ * Requires that d_parent and d_name are stable.
+ *
+ * Called from d_delete() and nfs_complete_sillyrename().
+ * The latter is called from nfs client ->unlink() ->rmdir() ->rename()
+ * under parent vfs inode lock.
+ *
+ * Most filesystems call d_delete() from ->unlink() ->rmdir() ->rename()
+ * ops under parent vfs inode lock.
+ *
+ * Some pseudo filesystems call d_delete() without parent inode lock.
+ * Those filesystems have no ->rename() op and they do not call
+ * d_move() directly, so d_parent and d_name are stable by definition.
+ * Examples: devpts, efivarfs, rpc_pipefs, functionfs.
+ *
+ * Last, there are the clustered filesystems that call d_delete() on
+ * remote nodes, not under vfs parent inode lock, but they use cluster
+ * distributed locks on local and remote nodes. Those filesystems call
+ * d_delete() under their cluster lock. Examples:
+ * - in ceph_fill_trace() under CEPH_MDS_R_PARENT_LOCKED
+ * - in ocfs2_dentry_convert_worker() under ocfs2_dentry_lock
+ * But those filesystems also call d_move() under the same cluster lock
+ * (i.e. FS_RENAME_DOES_D_MOVE), so d_parent and d_name are also stable.
*/
static inline void fsnotify_nameremove(struct dentry *dentry, int isdir)
{
+ struct inode *dir = d_inode(dentry->d_parent);
__u32 mask = FS_DELETE;
+ /* d_delete() of pseudo inode? (e.g. __ns_get_path() playing tricks) */
+ if (IS_ROOT(dentry))
+ return;
+
+ WARN_ON_ONCE(!inode_is_locked(dir) && dir->i_op->rename &&
+ !(dir->i_sb->s_type->fs_flags & FS_RENAME_DOES_D_MOVE));
+
if (isdir)
mask |= FS_ISDIR;
- fsnotify_dirent(NULL, dentry, mask);
+ fsnotify_dirent(dir, dentry, mask);
}
/*
--
2.17.1
