Enable lock context analysis
Subsystems (3)
v5 cover Enable lock context analysis
Hi Jens,
Recently the following patch series has been merged: [PATCH v5 00/36] Compiler-Based Context- and Locking-Analysis
(https://lore.kernel.org/lkml/[email protected]/). That patch series drops support for verifying lock context annotations with sparse and introduces support for verifying lock context annotations with Clang. The support in Clang for lock context annotation and verification is better than that in sparse. As an example, __cond_acquires() and __guarded_by() are supported by Clang but not by sparse. Hence this patch series that enables lock context analysis for the block layer core.
Note: although the Linux kernel documentation specifies 22 as minimal version for Clang for context analysis support, this patch series requires Clang 23 because it annotates function pointers. As one can see here, a patch has been queued that fixes the kernel documentation:
https://lore.kernel.org/all/177926568868.711.3058599932884307249.tip-bot2@tip-bot2/
Please consider this patch series for the next merge window.
Thanks,
Bart.
Changes compared to v4:
- Rebased and retested on top of Jens' latest for-next branch.
Changes compared to v3:
- Replaced the "block/bdev: Annotate the blk_holder_ops callback invocations" patch with a patch that adds __releases() annotations to the function pointers in struct blk_holder_ops.
- Dropped the blk-zoned patch since a better patch from Christoph has been merged.
Changes compared to v2:
- Retained the block layer core patches and left out the block driver patches.
- Inlined blkg_conf_open_bdev_frozen() and blkg_conf_close_bdev_frozen().
- In blkg_conf_open_bdev(), added a return statement if the WARN_ON_ONCE() statement triggers.
- Replaced the "block/ioctl: Add lock context annotations" patch with a __release() annotation.
- Replaced the blk-zoned patch with a patch from Christoph.
Changes compared to v1:
- Rebased this patch series on top of Jens' for-next branch.
- Included two patches that split blkg_conf_prep() and blkg_conf_exit().
- Modified how patches are split. Split the block layer core patch into multiple patches and moved the CONTEXT_ANALYSIS := y assignments into the block driver patches.
- Made the new source code comments easier to comprehend.
- Introduced macros in the mq-deadline and Kyber I/O schedulers to make the __acquires() expressions easier to read.
- Removed the changes from this series that are not block layer changes.
Bart Van Assche (12):
block: Annotate the queue limits functions
block/bdev: Annotate the blk_holder_ops callback functions block/cgroup: Split blkg_conf_prep()
block/cgroup: Split blkg_conf_exit()
block/cgroup: Improve lock context annotations block/cgroup: Inline blkg_conf_{open,close}_bdev_frozen() block/crypto: Annotate the crypto functions block/blk-iocost: Add lock context annotations block/blk-mq-debugfs: Improve lock context annotations block/kyber: Make the lock context annotations compatible with Clang block/mq-deadline: Make the lock context annotations compatible with
Clang
block: Enable lock context analysis
block/Makefile | 2 + block/bfq-cgroup.c | 11 ++++- block/blk-cgroup.c | 98 ++++++++++---------------------------- block/blk-cgroup.h | 13 +++-- block/blk-crypto-profile.c | 2 + block/blk-iocost.c | 96 +++++++++++++++++++++++-------------- block/blk-iolatency.c | 19 ++++---- block/blk-mq-debugfs.c | 12 ++--- block/blk-throttle.c | 34 +++++++------ block/blk.h | 4 ++ block/kyber-iosched.c | 7 ++- block/mq-deadline.c | 12 +++-- include/linux/blkdev.h | 21 +++++--- 13 files changed, 173 insertions(+), 158 deletions(-)
PATCH v5 1/12 block: Annotate the queue limits functions
Let the thread-safety checker verify whether every start of a queue limits update is followed by a call to a function that finishes a queue limits update.
--- include/linux/blkdev.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
include/linux/blkdev.h +6 -3
@@ -1092,15 +1092,17 @@ static inline unsigned int blk_boundary_sectors_left(sector_t offset,*/ static inline struct queue_limits queue_limits_start_update (struct request_queue *q) + __acquires(&q->limits_lock) {mutex_lock (&q->limits_lock);return q->limits; } int queue_limits_commit_update_frozen(struct request_queue *q, -struct queue_limits *lim); + struct queue_limits *lim) __releases(&q->limits_lock); int queue_limits_commit_update(struct request_queue *q, -struct queue_limits *lim); -int queue_limits_set (struct request_queue *q,struct queue_limits *lim); + struct queue_limits *lim) __releases(&q->limits_lock); +int queue_limits_set(struct request_queue *q, struct queue_limits *lim) + __must_not_hold(&q->limits_lock);int blk_validate_limits (struct queue_limits *lim);/**
@@ -1112,6 +1114,7 @@ int blk_validate_limits(struct queue_limits *lim);* starting update. */ static inline void queue_limits_cancel_update (struct request_queue *q) + __releases(&q->limits_lock) {mutex_unlock (&q->limits_lock); }
Re: [PATCH v5 01/12] block: Annotate the queue limits functions
Quoted 10 lines · Bart Van Assche
Let the thread-safety checker verify whether every start of a queue limits update is followed by a call to a function that finishes a queue limits update.
Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- include/linux/blkdev.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
PATCH v5 2/12 block/bdev: Annotate the blk_holder_ops callback functions
In reply to Bart Van AsscheThe four callback functions in blk_holder_ops all release the bd_holder_lock. Annotate these functions accordingly.
--- include/linux/blkdev.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
include/linux/blkdev.h +8 -4
@@ -1746,22 +1746,26 @@ void blkdev_show(struct seq_file *seqf, off_t offset);#endif struct blk_holder_ops { -void (*mark_dead)(struct block_device *bdev,bool surprise); + void (*mark_dead)(struct block_device *bdev, bool surprise) + __releases(&bdev->bd_holder_lock);/* * Sync the file system mounted on the block device. */ -void (*sync)(struct block_device *bdev); + void (*sync)(struct block_device *bdev) + __releases(&bdev->bd_holder_lock);/* * Freeze the file system mounted on the block device. */ -int (*freeze)(struct block_device *bdev); + int (*freeze)(struct block_device *bdev) + __releases(&bdev->bd_holder_lock);/* * Thaw the file system mounted on the block device. */ -int (*thaw)(struct block_device *bdev); + int (*thaw)(struct block_device *bdev) + __releases(&bdev->bd_holder_lock); };/*
Re: [PATCH v5 02/12] block/bdev: Annotate the blk_holder_ops callback functions
Quoted 8 lines · Bart Van Assche
The four callback functions in
blk_holder_opsall release thebd_holder_lock. Annotate these functions accordingly.Signed-off-by Bart Van Assche <[email protected]>--- include/linux/blkdev.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 02/12] block/bdev: Annotate the blk_holder_ops callback functions
In reply to Bart Van AsscheLooks good:
PATCH v5 3/12 block/cgroup: Split blkg_conf_prep()
In reply to Bart Van AsscheMove the blkg_conf_open_bdev() call out of blkg_conf_prep() to make it possible to add lock context annotations to blkg_conf_prep(). Change an if-statement in blkg_conf_open_bdev() into a WARN_ON_ONCE() call. Export blkg_conf_open_bdev() because it is called by the BFQ I/O scheduler and the BFQ I/O scheduler may be built as a kernel module.
--- block/bfq-cgroup.c | 4 ++++ block/blk-cgroup.c | 18 ++++++++---------- block/blk-iocost.c | 4 ++++ 3 files changed, 16 insertions(+), 10 deletions(-)
block/bfq-cgroup.c +4 -0
@@ -1051,6 +1051,10 @@ static ssize_t bfq_io_set_device_weight(struct kernfs_open_file *of,blkg_conf_init (&ctx, buf); + ret =blkg_conf_open_bdev (&ctx); +if (ret) + goto out; + ret =blkg_conf_prep (blkcg, &blkcg_policy_bfq, &ctx);if (ret) goto out;
block/blk-cgroup.c +8 -10
@@ -771,10 +771,7 @@ EXPORT_SYMBOL_GPL(blkg_conf_init);* @ctx->input and get and store the matching bdev in @ctx->bdev. @ctx->body is * set to point past the device node prefix. * -* This function may be called multiple times on @ctx and the extra calls become -* NOOPs. blkg_conf_prep() implicitly calls this function. Use this function -* explicitly if bdev access is needed without resolving the blkcg / policy part -* of @ctx->input. Returns -errno on error. +* Returns: -errno on error. */ int blkg_conf_open_bdev (struct blkg_conf_ctx *ctx) {
@@ -783,8 +780,8 @@ int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx)struct block_device *bdev;int key_len; -if (ctx->bdev) -return 0 ; +if (WARN_ON_ONCE (ctx->bdev)) +return -EINVAL ;if (sscanf (input,"%u:%u%n" , &major, &minor, &key_len) !=2 )return -EINVAL ;
@@ -813,6 +810,8 @@ int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx) ctx->bdev = bdev;return 0 ; } +EXPORT_SYMBOL_GPL (blkg_conf_open_bdev); +/* * Similar to blkg_conf_open_bdev, but additionally freezes the queue, * ensures the correct locking order between freeze queue and q->rq_qos_mutex.
@@ -857,7 +856,7 @@ unsigned long __must_check blkg_conf_open_bdev_frozen(struct blkg_conf_ctx *ctx)* following MAJ:MIN, @ctx->bdev points to the target block device and * @ctx->blkg to the blkg being configured. * -* blkg_conf_open_bdev() may be called on @ctx beforehand. On success, this +* 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(). */
@@ -870,9 +869,8 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,struct blkcg_gq *blkg;int ret; - ret =blkg_conf_open_bdev (ctx); -if (ret) -return ret; +if (WARN_ON_ONCE (!ctx->bdev)) +return -EINVAL ; disk = ctx->bdev->bd_disk; q = disk->queue;
block/blk-iocost.c +4 -0
@@ -3140,6 +3140,10 @@ static ssize_t ioc_weight_write(struct kernfs_open_file *of, char *buf,blkg_conf_init (&ctx, buf); + ret =blkg_conf_open_bdev (&ctx); +if (ret) + goto err; + ret =blkg_conf_prep (blkcg, &blkcg_policy_iocost, &ctx);if (ret) goto err;
Re: [PATCH v5 03/12] block/cgroup: Split blkg_conf_prep()
Quoted 6 lines · Bart Van Assche
Move the
blkg_conf_open_bdev()call out ofblkg_conf_prep()to make it possible to add lock context annotations toblkg_conf_prep(). Change an if-statement inblkg_conf_open_bdev()into aWARN_ON_ONCE()call. Exportblkg_conf_open_bdev()because it is called by the BFQ I/O scheduler and the BFQ I/O scheduler may be built as a kernel module.
Quoted 9 lines
Reviewed-by Christoph Hellwig <[email protected]>Cc Tejun Heo <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- block/bfq-cgroup.c | 4 ++++ block/blk-cgroup.c | 18 ++++++++---------- block/blk-iocost.c | 4 ++++ 3 files changed, 16 insertions(+), 10 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
PATCH v5 4/12 block/cgroup: Split blkg_conf_exit()
In reply to Bart Van AsscheSplit 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.
--- 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(-)
block/bfq-cgroup.c +6 -3
@@ -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; }
block/blk-cgroup.c +28 -29
92 lines not on this page · raw message
block/blk-cgroup.h +4 -2
@@ -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
block/blk-iocost.c +32 -35
161 lines not on this page · raw message
block/blk-iolatency.c +11 -8
@@ -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; }
block/blk-throttle.c +20 -14
@@ -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; }
Re: [PATCH v5 04/12] block/cgroup: Split blkg_conf_exit()
Quoted 9 lines · Bart Van Assche
Split
blkg_conf_exit()intoblkg_conf_unprep()andblkg_conf_close_bdev()becauseblkg_conf_exit()is not compatible with the Clang thread-safety annotations. Removeblkg_conf_exit(). Renameblkg_conf_exit_frozen()intoblkg_conf_close_bdev_frozen(). Add thread-safety annotations to the new functions.Reviewed-by Christoph Hellwig <[email protected]>Cc Tejun Heo <[email protected]>Signed-off-by Bart Van Assche <[email protected]>
Quoted 9 lines
--- 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(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
PATCH v5 5/12 block/cgroup: Improve lock context annotations
In reply to Bart Van AsscheAdd lock context annotations where these are missing. Move the blkg_conf_prep() annotation into block/blk-cgroup.h to make it visible to all blkg_conf_prep() callers.
--- block/blk-cgroup.c | 1 - block/blk-cgroup.h | 15 ++++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-)
block/blk-cgroup.c +0 -1
@@ -862,7 +862,6 @@ unsigned long __must_check 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) - __acquires(&bdev->bd_queue->queue_lock) {struct gendisk *disk;struct request_queue *q;
block/blk-cgroup.h +10 -5
@@ -218,14 +218,19 @@ struct blkg_conf_ctx { };void blkg_conf_init (struct blkg_conf_ctx *ctx,char *input); -int blkg_conf_open_bdev (struct blkg_conf_ctx *ctx); +int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx) + __cond_acquires(0, &ctx->bdev->bd_queue->rq_qos_mutex);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_unprep (struct blkg_conf_ctx *ctx); -void blkg_conf_close_bdev (struct blkg_conf_ctx *ctx); + struct blkg_conf_ctx *ctx) + __cond_acquires(0, &ctx->bdev->bd_disk->queue->queue_lock); +void blkg_conf_unprep(struct blkg_conf_ctx *ctx) + __releases(ctx->bdev->bd_disk->queue->queue_lock); +void blkg_conf_close_bdev(struct blkg_conf_ctx *ctx) + __releases(&ctx->bdev->bd_queue->rq_qos_mutex); void blkg_conf_close_bdev_frozen(struct blkg_conf_ctx *ctx, -unsigned long memflags); + unsigned long memflags) + __releases(&ctx->bdev->bd_queue->rq_qos_mutex);/** * bio_issue_as_root_blkg - see if this bio needs to be issued as root blkg
Re: [PATCH v5 05/12] block/cgroup: Improve lock context annotations
Quoted 10 lines · Bart Van Assche
Add lock context annotations where these are missing. Move the
blkg_conf_prep()annotation intoblock/blk-cgroup.hto make it visible to allblkg_conf_prep()callers.Signed-off-by Bart Van Assche <[email protected]>--- block/blk-cgroup.c | 1 - block/blk-cgroup.h | 15 ++++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 05/12] block/cgroup: Improve lock context annotations
In reply to Bart Van AsscheQuoted 3 lines · Bart Van Assche
Add lock context annotations where these are missing. Move the
blkg_conf_prep()annotation intoblock/blk-cgroup.hto make it visible to allblkg_conf_prep()callers.
Looks good:
PATCH v5 6/12 block/cgroup: Inline blkg_conf_{open,close}_bdev_frozen()
In reply to Bart Van AsscheThe blkg_conf_open_bdev_frozen() calling convention is not compatible with lock context annotations. Inline both blkg_conf_open_bdev_frozen() and blkg_conf_close_bdev_frozen() because these functions only have a single caller. This patch prepares for enabling lock context analysis.
The type of 'memflags' has been changed from unsigned long into unsigned int to match the type of current->flags. See also <linux/sched.h>.
--- block/blk-cgroup.c | 46 ---------------------------------------------- block/blk-cgroup.h | 4 ---- block/blk-iocost.c | 29 +++++++++++++++++++++++------ 3 files changed, 23 insertions(+), 56 deletions(-)
block/blk-cgroup.c +0 -46
@@ -812,39 +812,6 @@ int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx) }EXPORT_SYMBOL_GPL (blkg_conf_open_bdev); -/* -* Similar to blkg_conf_open_bdev, but additionally freezes the queue, -* 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_close_bdev_frozen() for restoring the memalloc scope. -*/ -unsigned long __must_check blkg_conf_open_bdev_frozen (struct blkg_conf_ctx *ctx) -{ -int ret; -unsigned long memflags; - -if (ctx->bdev) -return -EINVAL ; - - ret =blkg_conf_open_bdev (ctx); -if (ret <0 ) -return ret; -/* -* At this point, we haven’t started protecting anything related to QoS, -* so we release q->rq_qos_mutex here, which was first acquired in blkg_ -* conf_open_bdev. Later, we re-acquire q->rq_qos_mutex after freezing -* the queue to maintain the correct locking order. -*/ -mutex_unlock (&ctx->bdev->bd_queue->rq_qos_mutex); - - memflags =blk_mq_freeze_queue (ctx->bdev->bd_queue); -mutex_lock (&ctx->bdev->bd_queue->rq_qos_mutex); - -return memflags; -} -/** * blkg_conf_prep - parse and prepare for per-blkg config update * @blkcg: target block cgroup
@@ -991,19 +958,6 @@ void blkg_conf_close_bdev(struct blkg_conf_ctx *ctx) }EXPORT_SYMBOL_GPL (blkg_conf_close_bdev); -/* -* 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_close_bdev_frozen (struct blkg_conf_ctx *ctx, -unsigned long memflags) -{ -struct request_queue *q = ctx->bdev->bd_queue; - -blkg_conf_close_bdev (ctx); -blk_mq_unfreeze_queue (q, memflags); -} -static void blkg_iostat_add (struct blkg_iostat *dst,struct blkg_iostat *src) {int i;
block/blk-cgroup.h +0 -4
@@ -220,7 +220,6 @@ struct blkg_conf_ctx {void blkg_conf_init (struct blkg_conf_ctx *ctx,char *input); int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx) __cond_acquires(0, &ctx->bdev->bd_queue->rq_qos_mutex); -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) __cond_acquires(0, &ctx->bdev->bd_disk->queue->queue_lock);
@@ -228,9 +227,6 @@ void blkg_conf_unprep(struct blkg_conf_ctx *ctx)__releases (ctx->bdev->bd_disk->queue->queue_lock); void blkg_conf_close_bdev(struct blkg_conf_ctx *ctx) __releases(&ctx->bdev->bd_queue->rq_qos_mutex); -void blkg_conf_close_bdev_frozen(struct blkg_conf_ctx *ctx, - unsigned long memflags) - __releases(&ctx->bdev->bd_queue->rq_qos_mutex);/** * bio_issue_as_root_blkg - see if this bio needs to be issued as root blkg
block/blk-iocost.c +23 -6
@@ -3233,19 +3233,30 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input, size_t nbytes, loff_t off) {struct blkg_conf_ctx ctx; +struct request_queue *q;struct gendisk *disk;struct ioc *ioc;u32 qos[NR_QOS_PARAMS ];bool enable, user;char *body, *p; -unsigned long memflags; +unsigned int memflags;int ret;blkg_conf_init (&ctx, input); - memflags =blkg_conf_open_bdev_frozen (&ctx); -if (IS_ERR_VALUE (memflags)) -return memflags; + ret =blkg_conf_open_bdev (&ctx); +if (ret) +return ret; +/* +* At this point, we haven’t started protecting anything related to QoS, +* so we release q->rq_qos_mutex here, which was first acquired in blkg_ +* conf_open_bdev. Later, we re-acquire q->rq_qos_mutex after freezing +* the queue to maintain the correct locking order. +*/ +mutex_unlock (&ctx.bdev->bd_queue->rq_qos_mutex); + + memflags =blk_mq_freeze_queue (ctx.bdev->bd_queue); +mutex_lock (&ctx.bdev->bd_queue->rq_qos_mutex); body = ctx.body; disk = ctx.bdev->bd_disk;
@@ -3362,14 +3373,20 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,blk_mq_unquiesce_queue (disk->queue); -blkg_conf_close_bdev_frozen (&ctx, memflags); + q = ctx.bdev->bd_queue; +blkg_conf_close_bdev (&ctx); +blk_mq_unfreeze_queue (q, memflags); +return nbytes; + einval:spin_unlock_irq (&ioc->lock);blk_mq_unquiesce_queue (disk->queue); ret = -EINVAL ; err: -blkg_conf_close_bdev_frozen (&ctx, memflags); + q = ctx.bdev->bd_queue; +blkg_conf_close_bdev (&ctx); +blk_mq_unfreeze_queue (q, memflags);return ret; }
Re: [PATCH v5 06/12] block/cgroup: Inline blkg_conf_{open,close}_bdev_frozen()
Quoted 8 lines · Bart Van Assche
The
blkg_conf_open_bdev_frozen()calling convention is not compatible with lock context annotations. Inline bothblkg_conf_open_bdev_frozen()andblkg_conf_close_bdev_frozen()because these functions only have a single caller. This patch prepares for enabling lock context analysis.The type of 'memflags' has been changed from unsigned long into unsigned int to match the type of
current->flags. See also <linux/sched.h>.
Quoted 8 lines
Cc Tejun Heo <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- block/blk-cgroup.c | 46 ---------------------------------------------- block/blk-cgroup.h | 4 ---- block/blk-iocost.c | 29 +++++++++++++++++++++++------ 3 files changed, 23 insertions(+), 56 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 06/12] block/cgroup: Inline blkg_conf_{open,close}_bdev_frozen()
In reply to Bart Van AsscheQuoted 2 lines · Bart Van Assche
The
blkg_conf_open_bdev_frozen()calling convention is not compatible with lock context annotations. Inline bothblkg_conf_open_bdev_frozen()
Maybe say fold into the only caller here? Inline to me implies turning it into an inline function.
Quoted 5 lines
block/blk-iocost.c +4
-0+ q = ctx.bdev->bd_queue; +blkg_conf_close_bdev (&ctx); +blk_mq_unfreeze_queue (q, memflags); +return nbytes;
[...]
block/blk-iocost.c static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input, · line 3390
Quoted 4 lines
block/blk-iocost.c +3
-0+ q = ctx.bdev->bd_queue; +blkg_conf_close_bdev (&ctx); +blk_mq_unfreeze_queue (q, memflags);return ret;
It looks like this could be easily shared into a single label if you check for a non-zero ret and return that and else nbytes.
Otherwise this looks good.
PATCH v5 7/12 block/crypto: Annotate the crypto functions
In reply to Bart Van AsscheAdd the lock context annotations required for Clang's thread-safety analysis.
--- block/blk-crypto-profile.c | 2 ++ 1 file changed, 2 insertions(+)
block/blk-crypto-profile.c +2 -0
@@ -43,6 +43,7 @@ struct blk_crypto_keyslot { };static inline void blk_crypto_hw_enter (struct blk_crypto_profile *profile) + __acquires(&profile->lock) {/* * Calling into the driver requires profile->lock held and the device
@@ -55,6 +56,7 @@ static inline void blk_crypto_hw_enter(struct blk_crypto_profile *profile) }static inline void blk_crypto_hw_exit (struct blk_crypto_profile *profile) + __releases(&profile->lock) {up_write (&profile->lock);if (profile->dev)
Re: [PATCH v5 07/12] block/crypto: Annotate the crypto functions
Quoted 10 lines · Bart Van Assche
Add the lock context annotations required for Clang's thread-safety analysis.
Reviewed-by Christoph Hellwig <[email protected]>Cc Eric Biggers <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- block/blk-crypto-profile.c | 2 ++ 1 file changed, 2 insertions(+)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
PATCH v5 8/12 block/blk-iocost: Add lock context annotations
In reply to Bart Van AsscheSince iocg_lock() and iocg_unlock() both use conditional locking, annotate both with __no_context_analysis and use token_context_lock() to introduce a new lock context.
--- block/blk-iocost.c | 6 ++++++ 1 file changed, 6 insertions(+)
block/blk-iocost.c +6 -0
@@ -727,7 +727,11 @@ static void iocg_commit_bio(struct ioc_gq *iocg, struct bio *bio,put_cpu_ptr (gcs); } +token_context_lock (ioc_lock); +static void iocg_lock (struct ioc_gq *iocg,bool lock_ioc,unsigned long *flags) +__acquires (ioc_lock) + __context_unsafe(conditional locking) {if (lock_ioc) {spin_lock_irqsave (&iocg->ioc->lock, *flags);
@@ -738,6 +742,8 @@ static void iocg_lock(struct ioc_gq *iocg, bool lock_ioc, unsigned long *flags) }static void iocg_unlock (struct ioc_gq *iocg,bool unlock_ioc,unsigned long *flags) +__releases (ioc_lock) + __context_unsafe(conditional locking) {if (unlock_ioc) {spin_unlock (&iocg->waitq.lock);
Re: [PATCH v5 08/12] block/blk-iocost: Add lock context annotations
Quoted 9 lines · Bart Van Assche
Since
iocg_lock()andiocg_unlock()both use conditional locking, annotate both with__no_context_analysisand usetoken_context_lock()to introduce a new lock context.Signed-off-by Bart Van Assche <[email protected]>--- block/blk-iocost.c | 6 ++++++ 1 file changed, 6 insertions(+)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 08/12] block/blk-iocost: Add lock context annotations
In reply to Bart Van AsscheQuoted 3 lines · Bart Van Assche
Since
iocg_lock()andiocg_unlock()both use conditional locking, annotate both with__no_context_analysisand usetoken_context_lock()to introduce a new lock context.
Both of these are only called from two funtions. Have you looked into merging the locking logic into these helpers to see if this improves the situation? __no_context_analysis just seems like a big hammer for something relatively simple like this.
Quoted 23 lines
Signed-off-by Bart Van Assche <[email protected]>--- block/blk-iocost.c | 6 ++++++ 1 file changed, 6 insertions(+)block/blk-iocost.c +4
-0@@ -727,7 +727,11 @@ static void iocg_commit_bio(struct ioc_gq *iocg, struct bio *bio,put_cpu_ptr (gcs); } +token_context_lock (ioc_lock); +static void iocg_lock (struct ioc_gq *iocg,bool lock_ioc,unsigned long *flags) +__acquires (ioc_lock) + __context_unsafe(conditional locking) {if (lock_ioc) {spin_lock_irqsave (&iocg->ioc->lock, *flags);
block/blk-iocost.c static void iocg_lock(struct ioc_gq *iocg, bool lock_ioc, unsigned long *flags) · line 749
Quoted 8 lines
block/blk-iocost.c +2
-0@@ -738,6 +742,8 @@ static void iocg_lock(struct ioc_gq *iocg, bool lock_ioc, unsigned long *flags) }static void iocg_unlock (struct ioc_gq *iocg,bool unlock_ioc,unsigned long *flags) +__releases (ioc_lock) + __context_unsafe(conditional locking) {if (unlock_ioc) {spin_unlock (&iocg->waitq.lock);
---end quoted text---
Re: [PATCH v5 08/12] block/blk-iocost: Add lock context annotations
Quoted 9 lines · Christoph Hellwig
Quoted 3 lines · Bart Van Assche
Since
iocg_lock()andiocg_unlock()both use conditional locking, annotate both with__no_context_analysisand usetoken_context_lock()to introduce a new lock context.Both of these are only called from two funtions. Have you looked into merging the locking logic into these helpers to see if this improves the situation?
__no_context_analysisjust seems like a big hammer for something relatively simple like this.
This will require splitting ioc_rqos_throttle(). I will look into this.
Bart.
PATCH v5 9/12 block/blk-mq-debugfs: Improve lock context annotations
In reply to Bart Van AsscheMake the existing lock context annotations compatible with Clang. Add the lock context annotations that are missing.
--- block/blk-mq-debugfs.c | 12 ++++++------ block/blk.h | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-)
block/blk-mq-debugfs.c +6 -6
@@ -20,7 +20,7 @@ static int queue_poll_stat_show(void *data, struct seq_file *m) }static void *queue_requeue_list_start (struct seq_file *m,loff_t *pos) - __acquires(&q->requeue_lock) + __acquires(&((struct request_queue *)m->private)->requeue_lock) {struct request_queue *q = m->private;
@@ -36,7 +36,7 @@ static void *queue_requeue_list_next(struct seq_file *m, void *v, loff_t *pos) }static void queue_requeue_list_stop (struct seq_file *m,void *v) - __releases(&q->requeue_lock) + __releases(&((struct request_queue *)m->private)->requeue_lock) {struct request_queue *q = m->private;
@@ -298,7 +298,7 @@ int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)EXPORT_SYMBOL_GPL (blk_mq_debugfs_rq_show);static void *hctx_dispatch_start (struct seq_file *m,loff_t *pos) - __acquires(&hctx->lock) + __acquires(&((struct blk_mq_hw_ctx *)m->private)->lock) {struct blk_mq_hw_ctx *hctx = m->private;
@@ -314,7 +314,7 @@ static void *hctx_dispatch_next(struct seq_file *m, void *v, loff_t *pos) }static void hctx_dispatch_stop (struct seq_file *m,void *v) - __releases(&hctx->lock) + __releases(&((struct blk_mq_hw_ctx *)m->private)->lock) {struct blk_mq_hw_ctx *hctx = m->private;
@@ -486,7 +486,7 @@ static int hctx_dispatch_busy_show(void *data, struct seq_file *m)#define CTX_RQ_SEQ_OPS (name, type) \ static void *ctx_##name##_rq_list_start(struct seq_file *m, loff_t *pos) \ - __acquires(&ctx->lock) \ + __acquires(&((struct blk_mq_ctx *)m->private)->lock) \ { \ struct blk_mq_ctx *ctx = m->private; \ \
@@ -503,7 +503,7 @@ static void *ctx_##name##_rq_list_next(struct seq_file *m, void *v, \ } \ \ static void ctx_##name##_rq_list_stop(struct seq_file *m, void *v) \ - __releases(&ctx->lock) \ + __releases(&((struct blk_mq_ctx *)m->private)->lock) \ { \ struct blk_mq_ctx *ctx = m->private; \ \
block/blk.h +4 -0
@@ -756,16 +756,19 @@ static inline void blk_unfreeze_release_lock(struct request_queue *q)* reclaim from triggering block I/O. */ static inline void blk_debugfs_lock_nomemsave (struct request_queue *q) + __acquires(&q->debugfs_mutex) {mutex_lock (&q->debugfs_mutex); }static inline void blk_debugfs_unlock_nomemrestore (struct request_queue *q) + __releases(&q->debugfs_mutex) {mutex_unlock (&q->debugfs_mutex); }static inline unsigned int __must_check blk_debugfs_lock (struct request_queue *q) + __acquires(&q->debugfs_mutex) {unsigned int memflags =memalloc_noio_save ();
@@ -775,6 +778,7 @@ static inline unsigned int __must_check blk_debugfs_lock(struct request_queue *qstatic inline void blk_debugfs_unlock (struct request_queue *q,unsigned int memflags) + __releases(&q->debugfs_mutex) {blk_debugfs_unlock_nomemrestore (q);memalloc_noio_restore (memflags);
Re: [PATCH v5 09/12] block/blk-mq-debugfs: Improve lock context annotations
Quoted 9 lines · Bart Van Assche
Make the existing lock context annotations compatible with Clang. Add the lock context annotations that are missing.
Signed-off-by Bart Van Assche <[email protected]>--- block/blk-mq-debugfs.c | 12 ++++++------ block/blk.h | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 09/12] block/blk-mq-debugfs: Improve lock context annotations
In reply to Bart Van AsscheQuoted 14 lines · Bart Van Assche
Make the existing lock context annotations compatible with Clang. Add the lock context annotations that are missing.
Signed-off-by Bart Van Assche <[email protected]>--- block/blk-mq-debugfs.c | 12 ++++++------ block/blk.h | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-)block/blk-mq-debugfs.c +0
-0
block/blk-mq-debugfs.c static int queue_poll_stat_show(void *data, struct seq_file *m) · line 23
Quoted 5 lines
block/blk-mq-debugfs.c +1
-1@@ -20,7 +20,7 @@ static int queue_poll_stat_show(void *data, struct seq_file *m) }static void *queue_requeue_list_start (struct seq_file *m,loff_t *pos) - __acquires(&q->requeue_lock) + __acquires(&((struct request_queue *)m->private)->requeue_lock)
I try to member where we got stuck on this, but isn't there a way to at least have a macro for this dereference that can have a comment?
Also I guess most seq_file users will have a mess like this, but I don't really know a good way around it.
Re: [PATCH v5 09/12] block/blk-mq-debugfs: Improve lock context annotations
Quoted 13 lines · Christoph Hellwig
block/blk-mq-debugfs.c static int queue_poll_stat_show(void *data, struct seq_file *m) · line 23
Quoted 8 lines · Bart Van Assche
block/blk-mq-debugfs.c +1
-1@@ -20,7 +20,7 @@ static int queue_poll_stat_show(void *data, struct seq_file *m) }static void *queue_requeue_list_start (struct seq_file *m,loff_t *pos) - __acquires(&q->requeue_lock) + __acquires(&((struct request_queue *)m->private)->requeue_lock)I try to member where we got stuck on this, but isn't there a way to at least have a macro for this dereference that can have a comment?
Quoted 2 lines
Also I guess most
seq_fileusers will have a mess like this, but I don't really know a good way around it.
What was requested last time, to introduce macros for similar expressions in the I/O schedulers, has been implemented. I will introduce helper macros in blk-mq-debugfs.c too. Inline functions can't be used for this purpose because Clang's alias analyzer does not expand inline functions.
Thanks,
Bart.
PATCH v5 10/12 block/kyber: Make the lock context annotations compatible with Clang
In reply to Bart Van AsscheWhile sparse ignores the __acquires() and __releases() arguments, Clang verifies these. Make the arguments of __acquires() and __releases() acceptable for Clang.
--- block/kyber-iosched.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
block/kyber-iosched.c +5 -2
@@ -882,6 +882,9 @@ static const struct elv_fs_entry kyber_sched_attrs[] = { };#undef KYBER_LAT_ATTR +#define HCTX_FROM_SEQ_FILE (m) ((struct blk_mq_hw_ctx *)(m)->private) +#define KYBER_HCTX_DATA (hctx) ((struct kyber_hctx_data *)(hctx)->sched_data) +#ifdef CONFIG_BLK_DEBUG_FS #define KYBER_DEBUGFS_DOMAIN_ATTRS (domain, name) \ static int kyber_##name##_tokens_show(void *data, struct seq_file *m) \
@@ -894,7 +897,7 @@ static int kyber_##name##_tokens_show(void *data, struct seq_file *m) \ } \ \ static void *kyber_##name##_rqs_start(struct seq_file *m, loff_t *pos) \ - __acquires(&khd->lock) \ + __acquires(&KYBER_HCTX_DATA(HCTX_FROM_SEQ_FILE(m))->lock) \ { \ struct blk_mq_hw_ctx *hctx = m->private; \ struct kyber_hctx_data *khd = hctx->sched_data; \
@@ -913,7 +916,7 @@ static void *kyber_##name##_rqs_next(struct seq_file *m, void *v, \ } \ \ static void kyber_##name##_rqs_stop(struct seq_file *m, void *v) \ - __releases(&khd->lock) \ + __releases(&KYBER_HCTX_DATA(HCTX_FROM_SEQ_FILE(m))->lock) \ { \ struct blk_mq_hw_ctx *hctx = m->private; \ struct kyber_hctx_data *khd = hctx->sched_data; \
Re: [PATCH v5 10/12] block/kyber: Make the lock context annotations compatible with Clang
Quoted 9 lines · Bart Van Assche
While sparse ignores the
__acquires()and__releases()arguments, Clang verifies these. Make the arguments of__acquires()and__releases()acceptable for Clang.Signed-off-by Bart Van Assche <[email protected]>--- block/kyber-iosched.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 10/12] block/kyber: Make the lock context annotations compatible with Clang
In reply to Bart Van AsscheLooks good:
PATCH v5 11/12 block/mq-deadline: Make the lock context annotations compatible with Clang
In reply to Bart Van AsscheWhile sparse ignores the __acquires() and __releases() arguments, Clang verifies these. Make the arguments of __acquires() and __releases() acceptable for Clang.
--- block/mq-deadline.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
block/mq-deadline.c +8 -4
@@ -794,11 +794,15 @@ static const struct elv_fs_entry deadline_attrs[] = {__ATTR_NULL }; +#define RQ_FROM_SEQ_FILE (m) ((struct request_queue *)(m)->private) +#define DD_DATA_FROM_RQ (rq) \ + ((struct deadline_data *)(rq)->elevator->elevator_data) +#ifdef CONFIG_BLK_DEBUG_FS #define DEADLINE_DEBUGFS_DDIR_ATTRS (prio, data_dir, name) \ static void *deadline_##name##_fifo_start(struct seq_file *m, \ loff_t *pos) \ - __acquires(&dd->lock) \ + __acquires(&DD_DATA_FROM_RQ(RQ_FROM_SEQ_FILE(m))->lock) \ { \ struct request_queue *q = m->private; \ struct deadline_data *dd = q->elevator->elevator_data; \
@@ -819,7 +823,7 @@ static void *deadline_##name##_fifo_next(struct seq_file *m, void *v, \ } \ \ static void deadline_##name##_fifo_stop(struct seq_file *m, void *v) \ - __releases(&dd->lock) \ + __releases(&DD_DATA_FROM_RQ(RQ_FROM_SEQ_FILE(m))->lock) \ { \ struct request_queue *q = m->private; \ struct deadline_data *dd = q->elevator->elevator_data; \
@@ -921,7 +925,7 @@ static int dd_owned_by_driver_show(void *data, struct seq_file *m) }static void *deadline_dispatch_start (struct seq_file *m,loff_t *pos) - __acquires(&dd->lock) + __acquires(&DD_DATA_FROM_RQ(RQ_FROM_SEQ_FILE(m))->lock) {struct request_queue *q = m->private;struct deadline_data *dd = q->elevator->elevator_data;
@@ -939,7 +943,7 @@ static void *deadline_dispatch_next(struct seq_file *m, void *v, loff_t *pos) }static void deadline_dispatch_stop (struct seq_file *m,void *v) - __releases(&dd->lock) + __releases(&DD_DATA_FROM_RQ(RQ_FROM_SEQ_FILE(m))->lock) {struct request_queue *q = m->private;struct deadline_data *dd = q->elevator->elevator_data;
Re: [PATCH v5 11/12] block/mq-deadline: Make the lock context annotations compatible with Clang
Quoted 9 lines · Bart Van Assche
While sparse ignores the
__acquires()and__releases()arguments, Clang verifies these. Make the arguments of__acquires()and__releases()acceptable for Clang.Signed-off-by Bart Van Assche <[email protected]>--- block/mq-deadline.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 11/12] block/mq-deadline: Make the lock context annotations compatible with Clang
In reply to Bart Van AsscheLooks good:
PATCH v5 12/12 block: Enable lock context analysis
In reply to Bart Van AsscheNow that all block/*.c files have been annotated, enable lock context analysis for all these source files.
--- block/Makefile | 2 ++ 1 file changed, 2 insertions(+)
block/Makefile +2 -0
@@ -3,6 +3,8 @@ # Makefile for the kernel block layer # +CONTEXT_ANALYSIS := y + obj-y := bdev.o fops.o bio.o elevator.o blk-core.o blk-sysfs.o \ blk-flush.o blk-settings.o blk-ioc.o blk-map.o \ blk-merge.o blk-timeout.o blk-lib.o blk-mq.o \
Re: [PATCH v5 12/12] block: Enable lock context analysis
Quoted 8 lines · Bart Van Assche
Now that all
block/*.cfiles have been annotated, enable lock context analysis for all these source files.Signed-off-by Bart Van Assche <[email protected]>--- block/Makefile | 2 ++ 1 file changed, 2 insertions(+)
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH v5 12/12] block: Enable lock context analysis
In reply to Bart Van AsscheQuoted 2 lines · Bart Van Assche
Now that all
block/*.cfiles have been annotated, enable lock context analysis for all these source files.
Looks good: