All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: yukuai3@huawei.com
Cc: linux-block@vger.kernel.org
Subject: [bug report] blk-iocost: don't release 'ioc->lock' while updating params
Date: Wed, 26 Oct 2022 17:14:13 +0300	[thread overview]
Message-ID: <Y1lAtdAkRp8JYJ+c@kili> (raw)

Hello Yu Kuai,

The patch 2c0647988433: "blk-iocost: don't release 'ioc->lock' while
updating params" from Oct 12, 2022, leads to the following Smatch
static checker warnings:

block/blk-iocost.c:3211 ioc_qos_write() warn: sleeping in atomic context
block/blk-iocost.c:3240 ioc_qos_write() warn: sleeping in atomic context
block/blk-iocost.c:3407 ioc_cost_model_write() warn: sleeping in atomic context

block/blk-iocost.c
    3168 static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
    3169                              size_t nbytes, loff_t off)
    3170 {
    3171         struct block_device *bdev;
    3172         struct gendisk *disk;
    3173         struct ioc *ioc;
    3174         u32 qos[NR_QOS_PARAMS];
    3175         bool enable, user;
    3176         char *p;
    3177         int ret;
    3178 
    3179         bdev = blkcg_conf_open_bdev(&input);
    3180         if (IS_ERR(bdev))
    3181                 return PTR_ERR(bdev);
    3182 
    3183         disk = bdev->bd_disk;
    3184         ioc = q_to_ioc(disk->queue);
    3185         if (!ioc) {
    3186                 ret = blk_iocost_init(disk);
    3187                 if (ret)
    3188                         goto err;
    3189                 ioc = q_to_ioc(disk->queue);
    3190         }
    3191 
    3192         blk_mq_freeze_queue(disk->queue);
    3193         blk_mq_quiesce_queue(disk->queue);
    3194 
    3195         spin_lock_irq(&ioc->lock);

Preempt disabled.

    3196         memcpy(qos, ioc->params.qos, sizeof(qos));
    3197         enable = ioc->enabled;
    3198         user = ioc->user_qos_params;
    3199 
    3200         while ((p = strsep(&input, " \t\n"))) {
    3201                 substring_t args[MAX_OPT_ARGS];
    3202                 char buf[32];
    3203                 int tok;
    3204                 s64 v;
    3205 
    3206                 if (!*p)
    3207                         continue;
    3208 
    3209                 switch (match_token(p, qos_ctrl_tokens, args)) {
    3210                 case QOS_ENABLE:
--> 3211                         match_u64(&args[0], &v);
                                 ^^^^^^^^^^^^^^^^^^^^^^^^
match functions do sleeping allocations.

    3212                         enable = v;
    3213                         continue;
    3214                 case QOS_CTRL:
    3215                         match_strlcpy(buf, &args[0], sizeof(buf));
    3216                         if (!strcmp(buf, "auto"))
    3217                                 user = false;
    3218                         else if (!strcmp(buf, "user"))
    3219                                 user = true;
    3220                         else
    3221                                 goto einval;
    3222                         continue;
    3223                 }
    3224 
    3225                 tok = match_token(p, qos_tokens, args);
    3226                 switch (tok) {
    3227                 case QOS_RPPM:
    3228                 case QOS_WPPM:
    3229                         if (match_strlcpy(buf, &args[0], sizeof(buf)) >=
    3230                             sizeof(buf))
    3231                                 goto einval;
    3232                         if (cgroup_parse_float(buf, 2, &v))
    3233                                 goto einval;
    3234                         if (v < 0 || v > 10000)
    3235                                 goto einval;
    3236                         qos[tok] = v * 100;
    3237                         break;
    3238                 case QOS_RLAT:
    3239                 case QOS_WLAT:
    3240                         if (match_u64(&args[0], &v))
    3241                                 goto einval;
    3242                         qos[tok] = v;
    3243                         break;
    3244                 case QOS_MIN:
    3245                 case QOS_MAX:
    3246                         if (match_strlcpy(buf, &args[0], sizeof(buf)) >=
    3247                             sizeof(buf))
    3248                                 goto einval;
    3249                         if (cgroup_parse_float(buf, 2, &v))
    3250                                 goto einval;
    3251                         if (v < 0)
    3252                                 goto einval;
    3253                         qos[tok] = clamp_t(s64, v * 100,
    3254                                            VRATE_MIN_PPM, VRATE_MAX_PPM);
    3255                         break;
    3256                 default:
    3257                         goto einval;
    3258                 }
    3259                 user = true;
    3260         }
    3261 
    3262         if (qos[QOS_MIN] > qos[QOS_MAX])
    3263                 goto einval;
    3264 
    3265         if (enable) {
    3266                 blk_stat_enable_accounting(disk->queue);
    3267                 blk_queue_flag_set(QUEUE_FLAG_RQ_ALLOC_TIME, disk->queue);
    3268                 ioc->enabled = true;
    3269                 wbt_disable_default(disk->queue);
    3270         } else {
    3271                 blk_queue_flag_clear(QUEUE_FLAG_RQ_ALLOC_TIME, disk->queue);
    3272                 ioc->enabled = false;
    3273                 wbt_enable_default(disk->queue);
    3274         }
    3275 
    3276         if (user) {
    3277                 memcpy(ioc->params.qos, qos, sizeof(qos));
    3278                 ioc->user_qos_params = true;
    3279         } else {
    3280                 ioc->user_qos_params = false;
    3281         }
    3282 
    3283         ioc_refresh_params(ioc, true);
    3284         spin_unlock_irq(&ioc->lock);
    3285 
    3286         blk_mq_unquiesce_queue(disk->queue);
    3287         blk_mq_unfreeze_queue(disk->queue);
    3288 
    3289         blkdev_put_no_open(bdev);
    3290         return nbytes;
    3291 einval:
    3292         spin_unlock_irq(&ioc->lock);
    3293 
    3294         blk_mq_unquiesce_queue(disk->queue);
    3295         blk_mq_unfreeze_queue(disk->queue);
    3296 
    3297         ret = -EINVAL;
    3298 err:
    3299         blkdev_put_no_open(bdev);
    3300         return ret;
    3301 }

regards,
dan carpenter

             reply	other threads:[~2022-10-26 14:14 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-26 14:14 Dan Carpenter [this message]
2022-10-27  1:20 ` [bug report] blk-iocost: don't release 'ioc->lock' while updating params Yu Kuai

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=Y1lAtdAkRp8JYJ+c@kili \
    --to=dan.carpenter@oracle.com \
    --cc=linux-block@vger.kernel.org \
    --cc=yukuai3@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.