Generalize the take_name_snapshot()/release_name_snapshot() interface so it is possible to snapshot either a dentry d_name or its snapshot. The order of fields d_name and d_inode in struct dentry is swapped so d_name is adjacent to d_iname. This does not change struct size nor cache lines alignment. Currently, we snapshot the old name in vfs_rename() and we snapshot the snapshot the dentry name in __fsnotify_parent() and then we pass qstr to inotify which allocated a variable length event struct and copied the name. This new interface allows us to snapshot the name directly into an fanotify event struct instead of allocating a variable length struct and copying the name to it. Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> --- Jan, This vfs patch is a pre-requisite to fanotify name event patches [1]. I wanted to get it out in advance for wider audience review and in the hope that you or Al could pick up this patch for v5.6-rc1 in preparation for the fanotify patches. Al, any objections? Thanks, Amir. [1] https://github.com/amir73il/linux/commits/fanotify_name fs/dcache.c | 50 ++++++++++++++++++++++++++++++++---------- fs/debugfs/inode.c | 4 ++-- fs/namei.c | 2 +- fs/notify/fsnotify.c | 2 +- fs/overlayfs/export.c | 2 +- include/linux/dcache.h | 29 ++++++++++++++++++------ 6 files changed, 66 insertions(+), 23 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index b280e07e162b..a2b9ff77db18 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -259,9 +259,15 @@ struct external_name { unsigned char name[]; }; +static inline struct external_name *external_name_snap( + const struct name_snapshot *name) +{ + return container_of(name->name.name, struct external_name, name[0]); +} + static inline struct external_name *external_name(struct dentry *dentry) { - return container_of(dentry->d_name.name, struct external_name, name[0]); + return external_name_snap(&dentry->d_name_snap); } static void __d_free(struct rcu_head *head) @@ -278,27 +284,49 @@ static void __d_free_external(struct rcu_head *head) kmem_cache_free(dentry_cache, dentry); } +static inline int name_snap_is_external(const struct name_snapshot *name) +{ + return name->name.name != name->inline_name; +} + static inline int dname_external(const struct dentry *dentry) { - return dentry->d_name.name != dentry->d_iname; + return name_snap_is_external(&dentry->d_name_snap); } -void take_dentry_name_snapshot(struct name_snapshot *name, struct dentry *dentry) +/* + * Snapshot either a dentry d_name or it's snapshot. When snapshotting a dentry + * d_name, caller is responsible that d_name is stable. + */ +void take_name_snapshot(struct name_snapshot *name, + const struct name_snapshot *orig) { - spin_lock(&dentry->d_lock); - name->name = dentry->d_name; - if (unlikely(dname_external(dentry))) { - atomic_inc(&external_name(dentry)->u.count); + name->name = orig->name; + if (unlikely(name_snap_is_external(orig))) { + atomic_inc(&external_name_snap(orig)->u.count); } else { - memcpy(name->inline_name, dentry->d_iname, - dentry->d_name.len + 1); + memcpy(name->inline_name, orig->inline_name, + orig->name.len + 1); name->name.name = name->inline_name; } +} +EXPORT_SYMBOL(take_name_snapshot); + +void take_dentry_name_snapshot(struct name_snapshot *name, + struct dentry *dentry) +{ + BUILD_BUG_ON(offsetof(struct dentry, d_iname) != + offsetof(struct dentry, d_name_snap.inline_name)); + BUILD_BUG_ON(sizeof_field(struct dentry, d_iname) != + sizeof_field(struct dentry, d_name_snap.inline_name)); + + spin_lock(&dentry->d_lock); + take_name_snapshot(name, &dentry->d_name_snap); spin_unlock(&dentry->d_lock); } EXPORT_SYMBOL(take_dentry_name_snapshot); -void release_dentry_name_snapshot(struct name_snapshot *name) +void release_name_snapshot(struct name_snapshot *name) { if (unlikely(name->name.name != name->inline_name)) { struct external_name *p; @@ -307,7 +335,7 @@ void release_dentry_name_snapshot(struct name_snapshot *name) kfree_rcu(p, u.head); } } -EXPORT_SYMBOL(release_dentry_name_snapshot); +EXPORT_SYMBOL(release_name_snapshot); static inline void __d_set_inode_and_type(struct dentry *dentry, struct inode *inode, diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index f4d8df5e4714..149128da7c4c 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -859,14 +859,14 @@ struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, error = simple_rename(d_inode(old_dir), old_dentry, d_inode(new_dir), dentry, 0); if (error) { - release_dentry_name_snapshot(&old_name); + release_name_snapshot(&old_name); goto exit; } d_move(old_dentry, dentry); fsnotify_move(d_inode(old_dir), d_inode(new_dir), &old_name.name, d_is_dir(old_dentry), NULL, old_dentry); - release_dentry_name_snapshot(&old_name); + release_name_snapshot(&old_name); unlock_rename(new_dir, old_dir); dput(dentry); return old_dentry; diff --git a/fs/namei.c b/fs/namei.c index d6c91d1e88cb..de9cb22a95b0 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -4511,7 +4511,7 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry, new_is_dir, NULL, new_dentry); } } - release_dentry_name_snapshot(&old_name); + release_name_snapshot(&old_name); return error; } diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index a8b281569bbf..b7665b0e7edd 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -173,7 +173,7 @@ int fsnotify_parent(__u32 mask, const void *data, int data_type) take_dentry_name_snapshot(&name, dentry); ret = fsnotify(p_inode, mask, data, data_type, &name.name, 0); - release_dentry_name_snapshot(&name); + release_name_snapshot(&name); } dput(parent); diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c index 70e55588aedc..fd2856075373 100644 --- a/fs/overlayfs/export.c +++ b/fs/overlayfs/export.c @@ -400,7 +400,7 @@ static struct dentry *ovl_lookup_real_one(struct dentry *connected, } out: - release_dentry_name_snapshot(&name); + release_name_snapshot(&name); dput(parent); inode_unlock(dir); return this; diff --git a/include/linux/dcache.h b/include/linux/dcache.h index c1488cc84fd9..8aebca3830a5 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -86,16 +86,34 @@ extern struct dentry_stat_t dentry_stat; #define d_lock d_lockref.lock +struct name_snapshot { + struct qstr name; + unsigned char inline_name[DNAME_INLINE_LEN]; +}; + struct dentry { /* RCU lookup touched fields */ unsigned int d_flags; /* protected by d_lock */ seqcount_t d_seq; /* per dentry seqlock */ struct hlist_bl_node d_hash; /* lookup hash list */ struct dentry *d_parent; /* parent directory */ - struct qstr d_name; struct inode *d_inode; /* Where the name belongs to - NULL is * negative */ - unsigned char d_iname[DNAME_INLINE_LEN]; /* small names */ + union { + struct name_snapshot d_name_snap; + /* + * Unfolded replica of the above struct instead of: + * + * #define d_name d_name_snap.name + * + * which isn't popssible anyway because d_name keyword is also + * in use by coda and msdos structs in uapi. + */ + struct { + struct qstr d_name; + unsigned char d_iname[DNAME_INLINE_LEN]; + }; + }; /* Ref lookup also touches following */ struct lockref d_lockref; /* per-dentry lock and refcount */ @@ -596,11 +614,8 @@ static inline struct inode *d_real_inode(const struct dentry *dentry) return d_backing_inode(d_real((struct dentry *) dentry, NULL)); } -struct name_snapshot { - struct qstr name; - unsigned char inline_name[DNAME_INLINE_LEN]; -}; void take_dentry_name_snapshot(struct name_snapshot *, struct dentry *); -void release_dentry_name_snapshot(struct name_snapshot *); +void take_name_snapshot(struct name_snapshot *, const struct name_snapshot *); +void release_name_snapshot(struct name_snapshot *); #endif /* __LINUX_DCACHE_H */ -- 2.17.1