[Devel] [PATCH VZ9 v2 5/5] block/blk-cbt: add BLKCBTRENAME instead of BLKCBTSET flag

[Pavel Tikhomirov]

On 1/28/25 08:39, Andrey Zhadchenko wrote:
> Users could rename cbts with CI_FLAG_NEW_NAME flag during
> BLKCBTSET ioctl. Set command now uses ci_name to identify
> target, so we need a new field to present updated name.
> Add new BLKCBTRENAME ioctl instead of adding new fields to
> blk_user_cbt_info.
> 
> Note that we do not need to extra protect cbt->name: it is only
> used by
>   - concurrent userspace commands, which are also taking cbt_mutex
>   - cbt_page_alloc + CBT_DEAD bit. Fortunately CBT_DEAD bit can
> only occur during another user space command, which again is
> helding cbt_mutex.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-96269
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
> ---
>   block/blk-cbt.c         | 47 ++++++++++++++++++++++++++++++++++++-----
>   block/ioctl.c           |  1 +
>   include/uapi/linux/fs.h |  2 +-
>   3 files changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/block/blk-cbt.c b/block/blk-cbt.c
> index 3fd28ec8c0d2..dcda59240878 100644
> --- a/block/blk-cbt.c
> +++ b/block/blk-cbt.c
> @@ -946,11 +946,6 @@ static int cbt_ioc_set(struct block_device *bdev, struct blk_user_cbt_info __use
>   	if (!cbt)
>   		goto ioc_set_failed;
>   
> -	if (ci.ci_flags & CI_FLAG_NEW_NAME)
> -		memcpy(cbt->name, &ci.ci_name, sizeof(ci.ci_name));
> -	else if (memcmp(cbt->name, &ci.ci_name, sizeof(ci.ci_name)))
> -		goto ioc_set_failed;
> -
>   	ret = -EIO;
>   	if (test_bit(CBT_ERROR, &cbt->flags))
>   		goto ioc_set_failed;
> @@ -1020,6 +1015,46 @@ static int cbt_ioc_list(struct block_device *bdev, void __user *arg)
>   	return 0;
>   }
>   
> +static int cbt_ioc_rename(struct block_device *bdev, void __user *arg)
> +{
> +	struct request_queue *q = bdev_get_queue(bdev);
> +	struct blk_user_cbt_list lst;
> +	char name[CBT_NAME_LENGTH];
> +	struct cbt_info *cbt;
> +
> +	if (copy_from_user(&lst, arg, sizeof(lst)))
> +		return -EFAULT;
> +
> +	if (lst.count != 2)
> +		return -EINVAL;
> +
> +	if (copy_from_user(name, (char *)arg + sizeof(lst), CBT_NAME_LENGTH))
> +		return -EFAULT;
> +
> +	mutex_lock(&cbt_mutex);
> +
> +	cbt = blk_cbt_find(q, name);
> +	if (!cbt) {
> +		mutex_unlock(&cbt_mutex);
> +		return -ENOENT;
> +	}
> +
> +	if (copy_from_user(name, (char *)arg + sizeof(lst) + CBT_NAME_LENGTH, CBT_NAME_LENGTH)) {
> +		mutex_unlock(&cbt_mutex);
> +		return -EFAULT;
> +	}

Moving the above user-copy to before taking mutex_lock, would 1) 
decrease mutex held time, 2) save us one line as we would not need 
mutex_unlock there and 3) generally make the code easier to read as 
user-copies are brought together in code.

> +
> +	if (blk_cbt_find(q, name)) {
> +		mutex_unlock(&cbt_mutex);
> +		return -EINVAL;
> +	}
> +
> +	memcpy(cbt->name, name, CBT_NAME_LENGTH);
> +	mutex_unlock(&cbt_mutex);
> +
> +	return 0;
> +}
> +
>   static int cbt_ioc_misc(struct block_device *bdev, void __user *arg)
>   {
>   	struct request_queue *q = bdev_get_queue(bdev);
> @@ -1077,6 +1112,8 @@ int blk_cbt_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
>   		return cbt_ioc_misc(bdev, arg);
>   	case BLKCBTLIST:
>   		return cbt_ioc_list(bdev, arg);
> +	case BLKCBTRENAME:
> +		return cbt_ioc_rename(bdev, arg);
>   	default:
>   		BUG();
>   	}
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 6e76d55790ea..05d79953b2c0 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -570,6 +570,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, blk_mode_t mode,
>   	case BLKCBTCLR:
>   	case BLKCBTMISC:
>   	case BLKCBTLIST:
> +	case BLKCBTRENAME:
>   		return blk_cbt_ioctl(bdev, cmd, (char __user *)arg);
>   	default:
>   		return -ENOIOCTLCMD;
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 3122e2783eda..4ab115091bab 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -151,7 +151,6 @@ struct blk_user_cbt_info {
>   enum CI_FLAGS
>   {
>   	CI_FLAG_ONCE = 1, /* BLKCBTGET will clear bits */
> -	CI_FLAG_NEW_NAME = 2 /* BLKCBTSET update name */
>   };
>   
>   /* Extension of cbt ioctls:  */
> @@ -189,6 +188,7 @@ struct blk_user_cbt_list {
>   #define BLKCBTCLR _IOR(0x12,204,struct blk_user_cbt_info)
>   #define BLKCBTMISC _IOWR(0x12,205,struct blk_user_cbt_misc_info)
>   #define BLKCBTLIST _IOWR(0x12, 206, struct blk_user_cbt_list)
> +#define BLKCBTRENAME _IOWR(0x12, 207, struct blk_user_cbt_list)
>   
>   /*
>    * Flags for the fsx_xflags field

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.