From: Ming Lei <ming.lei@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org
Subject: Re: [PATCH 5/8] ublk: cleanup ublk_ctrl_uring_cmd
Date: Thu, 21 Jul 2022 20:12:09 +0800 [thread overview]
Message-ID: <YtlCmV4YuHegwiGX@T590> (raw)
In-Reply-To: <20220721051632.1676890-6-hch@lst.de>
On Thu, Jul 21, 2022 at 07:16:29AM +0200, Christoph Hellwig wrote:
> Move all per-command work into the per-command ublk_ctrl_* helpers
> instead of being split over those, ublk_ctrl_cmd_validate, and the main
> ublk_ctrl_uring_cmd handler. To facilitate that, the old
> ublk_ctrl_stop_dev function that just contained two function calls is
> folded into both callers.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/block/ublk_drv.c | 234 +++++++++++++++++++--------------------
> 1 file changed, 116 insertions(+), 118 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 1f7bbbc3276a2..af70c18796e70 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -813,13 +813,6 @@ static void ublk_stop_dev(struct ublk_device *ub)
> cancel_delayed_work_sync(&ub->monitor_work);
> }
>
> -static int ublk_ctrl_stop_dev(struct ublk_device *ub)
> -{
> - ublk_stop_dev(ub);
> - cancel_work_sync(&ub->stop_work);
> - return 0;
> -}
> -
> static inline bool ublk_queue_ready(struct ublk_queue *ubq)
> {
> return ubq->nr_io_ready == ubq->q_depth;
> @@ -1205,8 +1198,8 @@ static int ublk_add_dev(struct ublk_device *ub)
>
> static void ublk_remove(struct ublk_device *ub)
> {
> - ublk_ctrl_stop_dev(ub);
> -
> + ublk_stop_dev(ub);
> + cancel_work_sync(&ub->stop_work);
> cdev_device_del(&ub->cdev, &ub->cdev_dev);
> put_device(&ub->cdev_dev);
> }
> @@ -1227,36 +1220,45 @@ static struct ublk_device *ublk_get_device_from_id(int idx)
> return ub;
> }
>
> -static int ublk_ctrl_start_dev(struct ublk_device *ub, struct io_uring_cmd *cmd)
> +static int ublk_ctrl_start_dev(struct io_uring_cmd *cmd)
> {
> struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd;
> - int ret = -EINVAL;
> int ublksrv_pid = (int)header->data[0];
> unsigned long dev_blocks = header->data[1];
> + struct ublk_device *ub;
> + int ret = -EINVAL;
>
> if (ublksrv_pid <= 0)
> - return ret;
> + return -EINVAL;
> +
> + ub = ublk_get_device_from_id(header->dev_id);
> + if (!ub)
> + return -EINVAL;
>
> wait_for_completion_interruptible(&ub->completion);
>
> schedule_delayed_work(&ub->monitor_work, UBLK_DAEMON_MONITOR_PERIOD);
>
> mutex_lock(&ub->mutex);
> - if (!disk_live(ub->ub_disk)) {
> - /* We may get disk size updated */
> - if (dev_blocks) {
> - ub->dev_info.dev_blocks = dev_blocks;
> - ublk_update_capacity(ub);
> - }
> - ub->dev_info.ublksrv_pid = ublksrv_pid;
> - ret = add_disk(ub->ub_disk);
> - if (!ret)
> - ub->dev_info.state = UBLK_S_DEV_LIVE;
> - } else {
> + if (disk_live(ub->ub_disk)) {
> ret = -EEXIST;
> + goto out_unlock;
> }
> - mutex_unlock(&ub->mutex);
>
> + /* We may get disk size updated */
> + if (dev_blocks) {
> + ub->dev_info.dev_blocks = dev_blocks;
> + ublk_update_capacity(ub);
> + }
> + ub->dev_info.ublksrv_pid = ublksrv_pid;
> + ret = add_disk(ub->ub_disk);
> + if (ret)
> + goto out_unlock;
> +
> + ub->dev_info.state = UBLK_S_DEV_LIVE;
> +out_unlock:
> + mutex_unlock(&ub->mutex);
> + ublk_put_device(ub);
> return ret;
> }
>
> @@ -1281,6 +1283,13 @@ static int ublk_ctrl_get_queue_affinity(struct io_uring_cmd *cmd)
> unsigned long queue;
> unsigned int retlen;
> int ret = -EINVAL;
> +
> + if (header->len * BITS_PER_BYTE < nr_cpu_ids)
> + return -EINVAL;
> + if (header->len & (sizeof(unsigned long)-1))
> + return -EINVAL;
> + if (!header->addr)
> + return -EINVAL;
>
> ub = ublk_get_device_from_id(header->dev_id);
> if (!ub)
> @@ -1311,38 +1320,64 @@ static int ublk_ctrl_get_queue_affinity(struct io_uring_cmd *cmd)
> return ret;
> }
>
> -static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_dev_info *info,
> - void __user *argp, int idx)
> +static inline void ublk_dump_dev_info(struct ublksrv_ctrl_dev_info *info)
> {
> + pr_devel("%s: dev id %d flags %llx\n", __func__,
> + info->dev_id, info->flags[0]);
> + pr_devel("\t nr_hw_queues %d queue_depth %d block size %d dev_capacity %lld\n",
> + info->nr_hw_queues, info->queue_depth,
> + info->block_size, info->dev_blocks);
> +}
> +
> +static int ublk_ctrl_add_dev(struct io_uring_cmd *cmd)
> +{
> + struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd;
> + void __user *argp = (void __user *)(unsigned long)header->addr;
> + struct ublksrv_ctrl_dev_info info;
> struct ublk_device *ub;
> - int ret;
> + int ret = -EINVAL;
> +
> + if (header->len < sizeof(info) || !header->addr)
> + return -EINVAL;
> + if (header->queue_id != (u16)-1) {
> + pr_warn("%s: queue_id is wrong %x\n",
> + __func__, header->queue_id);
> + return -EINVAL;
> + }
> + if (copy_from_user(&info, argp, sizeof(info)))
> + return -EFAULT;
> + ublk_dump_dev_info(&ub->dev_info);
The above line should have been ublk_dump_dev_info(&info), otherwise
looks fine:
Reviewed-by: Ming Lei <ming.lei@rehdat.com>
Thanks,
Ming
next prev parent reply other threads:[~2022-07-21 12:12 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-21 5:16 ublk fixups Christoph Hellwig
2022-07-21 5:16 ` [PATCH 1/8] ublk: add a MAINTAINERS entry Christoph Hellwig
2022-07-21 7:11 ` Ming Lei
2022-07-21 5:16 ` [PATCH 2/8] ublk: remove UBLK_IO_F_PREFLUSH Christoph Hellwig
2022-07-21 7:13 ` Ming Lei
2022-07-21 5:16 ` [PATCH 3/8] ublk: remove the empty open and release block device operations Christoph Hellwig
2022-07-21 7:13 ` Ming Lei
2022-07-21 5:16 ` [PATCH 4/8] ublk: simplify ublk_ch_open and ublk_ch_release Christoph Hellwig
2022-07-21 7:15 ` Ming Lei
2022-07-21 5:16 ` [PATCH 5/8] ublk: cleanup ublk_ctrl_uring_cmd Christoph Hellwig
2022-07-21 7:19 ` Ziyang Zhang
2022-07-21 12:12 ` Ming Lei [this message]
2022-07-21 5:16 ` [PATCH 6/8] ublk: fold __ublk_create_dev into ublk_ctrl_add_dev Christoph Hellwig
2022-07-21 7:33 ` Ziyang Zhang
2022-07-21 7:35 ` Ming Lei
2022-07-21 5:16 ` [PATCH 7/8] ublk: rewrite ublk_ctrl_get_queue_affinity to not rely on hctx->cpumask Christoph Hellwig
2022-07-21 7:38 ` Ming Lei
2022-07-21 5:16 ` [PATCH 8/8] ublk: defer disk allocation Christoph Hellwig
2022-07-21 7:55 ` Ming Lei
-- strict thread matches above, loose matches on Subject: below --
2022-07-21 13:09 ublk fixups v2 Christoph Hellwig
2022-07-21 13:09 ` [PATCH 5/8] ublk: cleanup ublk_ctrl_uring_cmd Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YtlCmV4YuHegwiGX@T590 \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox