Re: [PATCH] fs: move the bdex_statx call to vfs_getattr_nosec

["Jain, Ayush" <ayushjai@amd.com>]

On 4/22/2025 7:45 PM, hch wrote:
> Turns out this doesn't work.  We used to have the request_mask, but it
> got removed in 25fbcd62d2e1 ("bdev: use bdev_io_min() for statx block
> size") so that stat can expose the block device min I/O size in
> st_blkdev, and as the blksize doesn't have it's own request_mask flag
> is hard to special case.
> 
> So maybe the better question is why devtmpfs even calls into
> vfs_getattr?  As far as I can tell handle_remove is only ever called on
> the actual devtmpfs file system, so we don't need to go through the
> VFS to query i_mode.  i.e. the patch should also fix the issue.  The
> modify_change is probably not needed either, but for now I'm aiming
> for the minimal fix.
> 
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index 6dd1a8860f1c..53fb0829eb7b 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -296,7 +296,7 @@ static int delete_path(const char *nodepath)
>  	return err;
>  }
>  
> -static int dev_mynode(struct device *dev, struct inode *inode, struct kstat *stat)
> +static int dev_mynode(struct device *dev, struct inode *inode)
>  {
>  	/* did we create it */
>  	if (inode->i_private != &thread)
> @@ -304,13 +304,13 @@ static int dev_mynode(struct device *dev, struct inode *inode, struct kstat *sta
>  
>  	/* does the dev_t match */
>  	if (is_blockdev(dev)) {
> -		if (!S_ISBLK(stat->mode))
> +		if (!S_ISBLK(inode->i_mode))
>  			return 0;
>  	} else {
> -		if (!S_ISCHR(stat->mode))
> +		if (!S_ISCHR(inode->i_mode))
>  			return 0;
>  	}
> -	if (stat->rdev != dev->devt)
> +	if (inode->i_rdev != dev->devt)
>  		return 0;
>  
>  	/* ours */
> @@ -321,8 +321,7 @@ static int handle_remove(const char *nodename, struct device *dev)
>  {
>  	struct path parent;
>  	struct dentry *dentry;
> -	struct kstat stat;
> -	struct path p;
> +	struct inode *inode;
>  	int deleted = 0;
>  	int err;
>  
> @@ -330,11 +329,8 @@ static int handle_remove(const char *nodename, struct device *dev)
>  	if (IS_ERR(dentry))
>  		return PTR_ERR(dentry);
>  
> -	p.mnt = parent.mnt;
> -	p.dentry = dentry;
> -	err = vfs_getattr(&p, &stat, STATX_TYPE | STATX_MODE,
> -			  AT_STATX_SYNC_AS_STAT);
> -	if (!err && dev_mynode(dev, d_inode(dentry), &stat)) {
> +	inode = d_inode(dentry);
> +	if (dev_mynode(dev, inode)) {
>  		struct iattr newattrs;
>  		/*
>  		 * before unlinking this node, reset permissions
> @@ -342,7 +338,7 @@ static int handle_remove(const char *nodename, struct device *dev)
>  		 */
>  		newattrs.ia_uid = GLOBAL_ROOT_UID;
>  		newattrs.ia_gid = GLOBAL_ROOT_GID;
> -		newattrs.ia_mode = stat.mode & ~0777;
> +		newattrs.ia_mode = inode->i_mode & ~0777;
>  		newattrs.ia_valid =
>  			ATTR_UID|ATTR_GID|ATTR_MODE;
>  		inode_lock(d_inode(dentry));


Thank you for the quick fix, this fixes blk devices hang issues on AMD
EPYC systems as well.

Tested-by: Ayush Jain <Ayush.jain3@amd.com>

Thanks,
Ayush