* [bug report] blk-iocost: don't release 'ioc->lock' while updating params
@ 2022-10-26 14:14 Dan Carpenter
2022-10-27 1:20 ` Yu Kuai
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-10-26 14:14 UTC (permalink / raw)
To: yukuai3; +Cc: linux-block
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] blk-iocost: don't release 'ioc->lock' while updating params
2022-10-26 14:14 [bug report] blk-iocost: don't release 'ioc->lock' while updating params Dan Carpenter
@ 2022-10-27 1:20 ` Yu Kuai
0 siblings, 0 replies; 2+ messages in thread
From: Yu Kuai @ 2022-10-27 1:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-block, yukuai (C)
Hi, Dan!
在 2022/10/26 22:14, Dan Carpenter 写道:
> 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.
Thanks for the report, I'll try to fix this soon.
Kuai
>
> 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
>
> .
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-10-27 1:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-26 14:14 [bug report] blk-iocost: don't release 'ioc->lock' while updating params Dan Carpenter
2022-10-27 1:20 ` Yu Kuai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox