Split blkg_conf_exit() into blkg_conf_unprep() and blkg_conf_close_bdev() because blkg_conf_exit() is not compatible with the Clang thread-safety annotations. Remove blkg_conf_exit(). Rename blkg_conf_exit_frozen() into blkg_conf_close_bdev_frozen(). Add thread-safety annotations to the new functions. Reviewed-by: Christoph Hellwig Cc: Tejun Heo Signed-off-by: Bart Van Assche --- block/bfq-cgroup.c | 9 ++++-- block/blk-cgroup.c | 57 ++++++++++++++++++------------------ block/blk-cgroup.h | 6 ++-- block/blk-iocost.c | 67 +++++++++++++++++++++---------------------- block/blk-iolatency.c | 19 ++++++------ block/blk-throttle.c | 34 +++++++++++++--------- 6 files changed, 101 insertions(+), 91 deletions(-) diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index 38396df9dce7..5d40279d6c9d 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -1053,11 +1053,11 @@ static ssize_t bfq_io_set_device_weight(struct kernfs_open_file *of, ret = blkg_conf_open_bdev(&ctx); if (ret) - goto out; + return ret; ret = blkg_conf_prep(blkcg, &blkcg_policy_bfq, &ctx); if (ret) - goto out; + goto close_bdev; if (sscanf(ctx.body, "%llu", &v) == 1) { /* require "default" on dfl */ @@ -1078,8 +1078,11 @@ static ssize_t bfq_io_set_device_weight(struct kernfs_open_file *of, bfq_group_set_weight(bfqg, bfqg->entity.weight, v); ret = 0; } + out: - blkg_conf_exit(&ctx); + blkg_conf_unprep(&ctx); +close_bdev: + blkg_conf_close_bdev(&ctx); return ret ?: nbytes; } diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index a8d95d51b866..38d7bcfcbbe8 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -755,7 +755,7 @@ EXPORT_SYMBOL_GPL(__blkg_prfill_u64); * * Initialize @ctx which can be used to parse blkg config input string @input. * Once initialized, @ctx can be used with blkg_conf_open_bdev() and - * blkg_conf_prep(), and must be cleaned up with blkg_conf_exit(). + * blkg_conf_prep(). */ void blkg_conf_init(struct blkg_conf_ctx *ctx, char *input) { @@ -817,8 +817,8 @@ EXPORT_SYMBOL_GPL(blkg_conf_open_bdev); * ensures the correct locking order between freeze queue and q->rq_qos_mutex. * * This function returns negative error on failure. On success it returns - * memflags which must be saved and later passed to blkg_conf_exit_frozen - * for restoring the memalloc scope. + * memflags which must be saved and later passed to + * blkg_conf_close_bdev_frozen() for restoring the memalloc scope. */ unsigned long __must_check blkg_conf_open_bdev_frozen(struct blkg_conf_ctx *ctx) { @@ -858,7 +858,7 @@ unsigned long __must_check blkg_conf_open_bdev_frozen(struct blkg_conf_ctx *ctx) * * blkg_conf_open_bdev() must be called on @ctx beforehand. On success, this * function returns with queue lock held and must be followed by - * blkg_conf_exit(). + * blkg_conf_close_bdev(). */ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, struct blkg_conf_ctx *ctx) @@ -968,42 +968,41 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, EXPORT_SYMBOL_GPL(blkg_conf_prep); /** - * blkg_conf_exit - clean up per-blkg config update + * blkg_conf_unprep - counterpart of blkg_conf_prep() * @ctx: blkg_conf_ctx initialized with blkg_conf_init() - * - * Clean up after per-blkg config update. This function must be called on all - * blkg_conf_ctx's initialized with blkg_conf_init(). */ -void blkg_conf_exit(struct blkg_conf_ctx *ctx) - __releases(&ctx->bdev->bd_queue->queue_lock) - __releases(&ctx->bdev->bd_queue->rq_qos_mutex) +void blkg_conf_unprep(struct blkg_conf_ctx *ctx) { - if (ctx->blkg) { - spin_unlock_irq(&bdev_get_queue(ctx->bdev)->queue_lock); - ctx->blkg = NULL; - } + WARN_ON_ONCE(!ctx->blkg); + spin_unlock_irq(&ctx->bdev->bd_disk->queue->queue_lock); + ctx->blkg = NULL; +} +EXPORT_SYMBOL_GPL(blkg_conf_unprep); - if (ctx->bdev) { - mutex_unlock(&ctx->bdev->bd_queue->rq_qos_mutex); - blkdev_put_no_open(ctx->bdev); - ctx->body = NULL; - ctx->bdev = NULL; - } +/** + * blkg_conf_close_bdev - counterpart of blkg_conf_open_bdev() + * @ctx: blkg_conf_ctx initialized with blkg_conf_init() + */ +void blkg_conf_close_bdev(struct blkg_conf_ctx *ctx) +{ + mutex_unlock(&ctx->bdev->bd_queue->rq_qos_mutex); + blkdev_put_no_open(ctx->bdev); + ctx->body = NULL; + ctx->bdev = NULL; } -EXPORT_SYMBOL_GPL(blkg_conf_exit); +EXPORT_SYMBOL_GPL(blkg_conf_close_bdev); /* - * Similar to blkg_conf_exit, but also unfreezes the queue. Should be used + * Similar to blkg_close_bdev, but also unfreezes the queue. Should be used * when blkg_conf_open_bdev_frozen is used to open the bdev. */ -void blkg_conf_exit_frozen(struct blkg_conf_ctx *ctx, unsigned long memflags) +void blkg_conf_close_bdev_frozen(struct blkg_conf_ctx *ctx, + unsigned long memflags) { - if (ctx->bdev) { - struct request_queue *q = ctx->bdev->bd_queue; + struct request_queue *q = ctx->bdev->bd_queue; - blkg_conf_exit(ctx); - blk_mq_unfreeze_queue(q, memflags); - } + blkg_conf_close_bdev(ctx); + blk_mq_unfreeze_queue(q, memflags); } static void blkg_iostat_add(struct blkg_iostat *dst, struct blkg_iostat *src) diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 1cce3294634d..ce90f5b60d52 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -222,8 +222,10 @@ int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx); unsigned long blkg_conf_open_bdev_frozen(struct blkg_conf_ctx *ctx); int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, struct blkg_conf_ctx *ctx); -void blkg_conf_exit(struct blkg_conf_ctx *ctx); -void blkg_conf_exit_frozen(struct blkg_conf_ctx *ctx, unsigned long memflags); +void blkg_conf_unprep(struct blkg_conf_ctx *ctx); +void blkg_conf_close_bdev(struct blkg_conf_ctx *ctx); +void blkg_conf_close_bdev_frozen(struct blkg_conf_ctx *ctx, + unsigned long memflags); /** * bio_issue_as_root_blkg - see if this bio needs to be issued as root blkg diff --git a/block/blk-iocost.c b/block/blk-iocost.c index b34f820dedcc..e611dd63d712 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -3142,21 +3142,23 @@ static ssize_t ioc_weight_write(struct kernfs_open_file *of, char *buf, ret = blkg_conf_open_bdev(&ctx); if (ret) - goto err; + return ret; ret = blkg_conf_prep(blkcg, &blkcg_policy_iocost, &ctx); if (ret) - goto err; + goto close_bdev; iocg = blkg_to_iocg(ctx.blkg); + ret = -EINVAL; + if (!strncmp(ctx.body, "default", 7)) { v = 0; } else { if (!sscanf(ctx.body, "%u", &v)) - goto einval; + goto unprep; if (v < CGROUP_WEIGHT_MIN || v > CGROUP_WEIGHT_MAX) - goto einval; + goto unprep; } spin_lock(&iocg->ioc->lock); @@ -3165,14 +3167,15 @@ static ssize_t ioc_weight_write(struct kernfs_open_file *of, char *buf, weight_updated(iocg, &now); spin_unlock(&iocg->ioc->lock); - blkg_conf_exit(&ctx); - return nbytes; + ret = 0; -einval: - ret = -EINVAL; -err: - blkg_conf_exit(&ctx); - return ret; +unprep: + blkg_conf_unprep(&ctx); + +close_bdev: + blkg_conf_close_bdev(&ctx); + + return ret ? ret : nbytes; } static u64 ioc_qos_prfill(struct seq_file *sf, struct blkg_policy_data *pd, @@ -3241,10 +3244,8 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input, blkg_conf_init(&ctx, input); memflags = blkg_conf_open_bdev_frozen(&ctx); - if (IS_ERR_VALUE(memflags)) { - ret = memflags; - goto err; - } + if (IS_ERR_VALUE(memflags)) + return memflags; body = ctx.body; disk = ctx.bdev->bd_disk; @@ -3361,14 +3362,14 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input, blk_mq_unquiesce_queue(disk->queue); - blkg_conf_exit_frozen(&ctx, memflags); + blkg_conf_close_bdev_frozen(&ctx, memflags); return nbytes; einval: spin_unlock_irq(&ioc->lock); blk_mq_unquiesce_queue(disk->queue); ret = -EINVAL; err: - blkg_conf_exit_frozen(&ctx, memflags); + blkg_conf_close_bdev_frozen(&ctx, memflags); return ret; } @@ -3434,20 +3435,20 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input, ret = blkg_conf_open_bdev(&ctx); if (ret) - goto err; + return ret; body = ctx.body; q = bdev_get_queue(ctx.bdev); if (!queue_is_mq(q)) { ret = -EOPNOTSUPP; - goto err; + goto close_bdev; } ioc = q_to_ioc(q); if (!ioc) { ret = blk_iocost_init(ctx.bdev->bd_disk); if (ret) - goto err; + goto close_bdev; ioc = q_to_ioc(q); } @@ -3458,6 +3459,8 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input, memcpy(u, ioc->params.i_lcoefs, sizeof(u)); user = ioc->user_cost_model; + ret = -EINVAL; + while ((p = strsep(&body, " \t\n"))) { substring_t args[MAX_OPT_ARGS]; char buf[32]; @@ -3475,20 +3478,20 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input, else if (!strcmp(buf, "user")) user = true; else - goto einval; + goto unlock; continue; case COST_MODEL: match_strlcpy(buf, &args[0], sizeof(buf)); if (strcmp(buf, "linear")) - goto einval; + goto unlock; continue; } tok = match_token(p, i_lcoef_tokens, args); if (tok == NR_I_LCOEFS) - goto einval; + goto unlock; if (match_u64(&args[0], &v)) - goto einval; + goto unlock; u[tok] = v; user = true; } @@ -3500,24 +3503,18 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input, ioc->user_cost_model = false; } ioc_refresh_params(ioc, true); - spin_unlock_irq(&ioc->lock); - blk_mq_unquiesce_queue(q); - blk_mq_unfreeze_queue(q, memflags); - - blkg_conf_exit(&ctx); - return nbytes; + ret = 0; -einval: +unlock: spin_unlock_irq(&ioc->lock); blk_mq_unquiesce_queue(q); blk_mq_unfreeze_queue(q, memflags); - ret = -EINVAL; -err: - blkg_conf_exit(&ctx); - return ret; +close_bdev: + blkg_conf_close_bdev(&ctx); + return ret ? ret : nbytes; } static struct cftype ioc_files[] = { diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c index 53e8dd2dfa8a..1aaee6fb0f59 100644 --- a/block/blk-iolatency.c +++ b/block/blk-iolatency.c @@ -840,7 +840,7 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf, ret = blkg_conf_open_bdev(&ctx); if (ret) - goto out; + return ret; /* * blk_iolatency_init() may fail after rq_qos_add() succeeds which can @@ -850,11 +850,11 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf, if (!iolat_rq_qos(ctx.bdev->bd_queue)) ret = blk_iolatency_init(ctx.bdev->bd_disk); if (ret) - goto out; + goto close_bdev; ret = blkg_conf_prep(blkcg, &blkcg_policy_iolatency, &ctx); if (ret) - goto out; + goto close_bdev; iolat = blkg_to_lat(ctx.blkg); p = ctx.body; @@ -865,7 +865,7 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf, char val[21]; /* 18446744073709551616 */ if (sscanf(tok, "%15[^=]=%20s", key, val) != 2) - goto out; + goto unprep; if (!strcmp(key, "target")) { u64 v; @@ -875,9 +875,9 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf, else if (sscanf(val, "%llu", &v) == 1) lat_val = v * NSEC_PER_USEC; else - goto out; + goto unprep; } else { - goto out; + goto unprep; } } @@ -889,8 +889,11 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf, if (oldval != iolat->min_lat_nsec) iolatency_clear_scaling(blkg); ret = 0; -out: - blkg_conf_exit(&ctx); + +unprep: + blkg_conf_unprep(&ctx); +close_bdev: + blkg_conf_close_bdev(&ctx); return ret ?: nbytes; } diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 88986dde1e18..47052ba21d1b 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1353,21 +1353,21 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of, ret = blkg_conf_open_bdev(&ctx); if (ret) - goto out_finish; + return ret; if (!blk_throtl_activated(ctx.bdev->bd_queue)) { ret = blk_throtl_init(ctx.bdev->bd_disk); if (ret) - goto out_finish; + goto close_bdev; } ret = blkg_conf_prep(blkcg, &blkcg_policy_throtl, &ctx); if (ret) - goto out_finish; + goto close_bdev; ret = -EINVAL; if (sscanf(ctx.body, "%llu", &v) != 1) - goto out_finish; + goto unprep; if (!v) v = U64_MAX; @@ -1381,8 +1381,12 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of, tg_conf_updated(tg, false); ret = 0; -out_finish: - blkg_conf_exit(&ctx); + +unprep: + blkg_conf_unprep(&ctx); + +close_bdev: + blkg_conf_close_bdev(&ctx); return ret ?: nbytes; } @@ -1537,17 +1541,17 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, ret = blkg_conf_open_bdev(&ctx); if (ret) - goto out_finish; + return ret; if (!blk_throtl_activated(ctx.bdev->bd_queue)) { ret = blk_throtl_init(ctx.bdev->bd_disk); if (ret) - goto out_finish; + goto close_bdev; } ret = blkg_conf_prep(blkcg, &blkcg_policy_throtl, &ctx); if (ret) - goto out_finish; + goto close_bdev; tg = blkg_to_tg(ctx.blkg); tg_update_carryover(tg); @@ -1573,11 +1577,11 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, p = tok; strsep(&p, "="); if (!p || (sscanf(p, "%llu", &val) != 1 && strcmp(p, "max"))) - goto out_finish; + goto unprep; ret = -ERANGE; if (!val) - goto out_finish; + goto unprep; ret = -EINVAL; if (!strcmp(tok, "rbps")) @@ -1589,7 +1593,7 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, else if (!strcmp(tok, "wiops")) v[3] = min_t(u64, val, UINT_MAX); else - goto out_finish; + goto unprep; } tg->bps[READ] = v[0]; @@ -1599,8 +1603,10 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, tg_conf_updated(tg, false); ret = 0; -out_finish: - blkg_conf_exit(&ctx); +unprep: + blkg_conf_unprep(&ctx); +close_bdev: + blkg_conf_close_bdev(&ctx); return ret ?: nbytes; }