Re: [PATCH v6 2/5] Revert "libfs: Add simple_offset_empty()"

[yangerkun <yangerkun@xxxxxxxxxxxxxxx>]

LGTM

Reviewed-by: Yang Erkun <yangerkun@xxxxxxxxxx>

在 2024/12/20 23:33, cel@xxxxxxxxxx 写道:
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> simple_empty() and simple_offset_empty() perform the same task.
> The latter's use as a canary to find bugs has not found any new
> issues. A subsequent patch will remove the use of the mtree for
> iterating directory contents, so revert back to using a similar
> mechanism for determining whether a directory is indeed empty.
>
> Only one such mechanism is ever needed.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>   fs/libfs.c         | 32 --------------------------------
>   include/linux/fs.h |  1 -
>   mm/shmem.c         |  4 ++--
>   3 files changed, 2 insertions(+), 35 deletions(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 3da58a92f48f..8380d9314ebd 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -329,38 +329,6 @@ void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry)
>   	offset_set(dentry, 0);
>   }
>   
> -/**
> - * simple_offset_empty - Check if a dentry can be unlinked
> - * @dentry: dentry to be tested
> - *
> - * Returns 0 if @dentry is a non-empty directory; otherwise returns 1.
> - */
> -int simple_offset_empty(struct dentry *dentry)
> -{
> -	struct inode *inode = d_inode(dentry);
> -	struct offset_ctx *octx;
> -	struct dentry *child;
> -	unsigned long index;
> -	int ret = 1;
> -
> -	if (!inode || !S_ISDIR(inode->i_mode))
> -		return ret;
> -
> -	index = DIR_OFFSET_MIN;
> -	octx = inode->i_op->get_offset_ctx(inode);
> -	mt_for_each(&octx->mt, child, index, LONG_MAX) {
> -		spin_lock(&child->d_lock);
> -		if (simple_positive(child)) {
> -			spin_unlock(&child->d_lock);
> -			ret = 0;
> -			break;
> -		}
> -		spin_unlock(&child->d_lock);
> -	}
> -
> -	return ret;
> -}
> -
>   /**
>    * simple_offset_rename - handle directory offsets for rename
>    * @old_dir: parent directory of source entry
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 7e29433c5ecc..f7efc6866ebc 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3468,7 +3468,6 @@ struct offset_ctx {
>   void simple_offset_init(struct offset_ctx *octx);
>   int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry);
>   void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry);
> -int simple_offset_empty(struct dentry *dentry);
>   int simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry,
>   			 struct inode *new_dir, struct dentry *new_dentry);
>   int simple_offset_rename_exchange(struct inode *old_dir,
> diff --git a/mm/shmem.c b/mm/shmem.c
> index ccb9629a0f70..274c2666f457 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3818,7 +3818,7 @@ static int shmem_unlink(struct inode *dir, struct dentry *dentry)
>   
>   static int shmem_rmdir(struct inode *dir, struct dentry *dentry)
>   {
> -	if (!simple_offset_empty(dentry))
> +	if (!simple_empty(dentry))
>   		return -ENOTEMPTY;
>   
>   	drop_nlink(d_inode(dentry));
> @@ -3875,7 +3875,7 @@ static int shmem_rename2(struct mnt_idmap *idmap,
>   		return simple_offset_rename_exchange(old_dir, old_dentry,
>   						     new_dir, new_dentry);
>   
> -	if (!simple_offset_empty(new_dentry))
> +	if (!simple_empty(new_dentry))
>   		return -ENOTEMPTY;
>   
>   	if (flags & RENAME_WHITEOUT) {