| Message ID | 20190514221901.29125-2-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Sort out fsnotify_nameremove() mess | expand |
On Wed 15-05-19 01:18:58, Amir Goldstein wrote: > There is a common pattern among pseudo filesystems for removing a dentry > from code paths that are NOT coming from vfs_{unlink,rmdir}, using a > combination of simple_{unlink,rmdir} and d_delete(). > > Create an helper to perform this common operation. This helper is going > to be used as a place holder for the new fsnotify_remove() hook. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> Looks like a good idea. One comment below: > +/* > + * Unlike simple_unlink/rmdir, this helper is NOT called from vfs_unlink/rmdir. > + * Caller must guaranty that d_parent and d_name are stable. > + */ > +int simple_remove(struct inode *dir, struct dentry *dentry) > +{ > + int ret; > + > + dget(dentry); > + if (d_is_dir(dentry)) > + ret = simple_rmdir(dir, dentry); > + else > + ret = simple_unlink(dir, dentry); > + > + if (!ret) > + d_delete(dentry); > + dput(dentry); This dget() - dput() pair looks fishy. After some research I understand why it's needed but I think it deserves a comment like: /* * 'simple_' operations get dentry reference on create/mkdir and drop * it on unlink/rmdir. So we have to get dentry reference here to * protect d_delete() from accessing freed dentry. */ Honza
diff --git a/fs/libfs.c b/fs/libfs.c index 4b59b1816efb..030e67c52b5f 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -353,6 +353,28 @@ int simple_rmdir(struct inode *dir, struct dentry *dentry) } EXPORT_SYMBOL(simple_rmdir); +/* + * Unlike simple_unlink/rmdir, this helper is NOT called from vfs_unlink/rmdir. + * Caller must guaranty that d_parent and d_name are stable. + */ +int simple_remove(struct inode *dir, struct dentry *dentry) +{ + int ret; + + dget(dentry); + if (d_is_dir(dentry)) + ret = simple_rmdir(dir, dentry); + else + ret = simple_unlink(dir, dentry); + + if (!ret) + d_delete(dentry); + dput(dentry); + + return ret; +} +EXPORT_SYMBOL(simple_remove); + int simple_rename(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, struct dentry *new_dentry, unsigned int flags) diff --git a/include/linux/fs.h b/include/linux/fs.h index f7fdfe93e25d..74ea5f0b3b9d 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3245,6 +3245,7 @@ extern int simple_open(struct inode *inode, struct file *file); extern int simple_link(struct dentry *, struct inode *, struct dentry *); extern int simple_unlink(struct inode *, struct dentry *); extern int simple_rmdir(struct inode *, struct dentry *); +extern int simple_remove(struct inode *, struct dentry *); extern int simple_rename(struct inode *, struct dentry *, struct inode *, struct dentry *, unsigned int); extern int noop_fsync(struct file *, loff_t, loff_t, int);
There is a common pattern among pseudo filesystems for removing a dentry from code paths that are NOT coming from vfs_{unlink,rmdir}, using a combination of simple_{unlink,rmdir} and d_delete(). Create an helper to perform this common operation. This helper is going to be used as a place holder for the new fsnotify_remove() hook. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/libfs.c | 22 ++++++++++++++++++++++ include/linux/fs.h | 1 + 2 files changed, 23 insertions(+)