Hi!
Thanks for the patch but I think you missed Amir's comments to your v2 [1]?
Also ...
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index 42e5a766d33c..714e6f9b74f5 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -617,12 +617,17 @@ void *devpts_get_priv(struct dentry *dentry)
> */
> void devpts_pty_kill(struct dentry *dentry)
> {
> + struct inode *dir = d_inode(dentry->d_parent);
> + struct inode *inode = d_inode(dentry);
> +
> WARN_ON_ONCE(dentry->d_sb->s_magic != DEVPTS_SUPER_MAGIC);
>
> + ihold(inode);
> dentry->d_fsdata = NULL;
> drop_nlink(dentry->d_inode);
> - fsnotify_unlink(d_inode(dentry->d_parent), dentry);
> d_drop(dentry);
> + fsnotify_delete(dir, dentry, inode);
> + iput(inode);
> dput(dentry); /* d_alloc_name() in devpts_pty_new() */
> }
AFAICT d_drop() actually doesn't make the dentry negative so there's no
need for this inode reference game? And similarly for d_invalidate() below?
Or am I missing something?
Honza
[1] https://lore.kernel.org/linux-btrfs/CAOQ4uxhWz_J4fir9ft5XpRVHoNCdk_bP1y-a=MhBqRYSf3N8gA@xxxxxxxxxxxxxx
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index e9b29c6ffccb..189e12dc5d9b 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -271,7 +271,7 @@ void simple_recursive_removal(struct dentry *dentry,
> struct dentry *this = dget(dentry);
> while (true) {
> struct dentry *victim = NULL, *child;
> - struct inode *inode = this->d_inode;
> + struct inode *inode = this->d_inode, *victim_inode;
>
> inode_lock(inode);
> if (d_is_dir(this))
> @@ -283,19 +283,19 @@ void simple_recursive_removal(struct dentry *dentry,
> clear_nlink(inode);
> inode_unlock(inode);
> victim = this;
> + victim_inode = d_inode(victim);
> + ihold(victim_inode);
> this = this->d_parent;
> inode = this->d_inode;
> inode_lock(inode);
> if (simple_positive(victim)) {
> d_invalidate(victim); // avoid lost mounts
> - if (d_is_dir(victim))
> - fsnotify_rmdir(inode, victim);
> - else
> - fsnotify_unlink(inode, victim);
> + fsnotify_delete(inode, victim, victim_inode);
> if (callback)
> callback(victim);
> dput(victim); // unpin it
> }
> + iput(victim_inode);
> if (victim == dentry) {
> inode->i_ctime = inode->i_mtime =
> current_time(inode);
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
