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
.
