Branches for-6.11/block and block-6.10 both have the following sparse
build issues:
block/bdev.c:388:17: warning: symbol 'blockdev_mnt' was not declared.
Should it be static?
block/blk-settings.c:266:9: warning: context imbalance in
'queue_limits_commit_update' - wrong count at exit
block/blk-cgroup.c:834:5: warning: context imbalance in
'blkg_conf_prep' - wrong count at exit
block/blk-cgroup.c:965:9: warning: context imbalance in
'blkg_conf_exit' - wrong count at exit
block/blk-iocost.c:732:9: warning: context imbalance in 'iocg_lock' -
wrong count at exit
block/blk-iocost.c:743:28: warning: context imbalance in 'iocg_unlock'
- unexpected unlock
block/blk-zoned.c:595:30: warning: context imbalance in
'disk_get_and_lock_zone_wplug' - wrong count at exit
block/blk-zoned.c: note: in included file (through include/linux/blkdev.h):
./include/linux/bio.h:592:9: warning: context imbalance in
'blk_zone_wplug_handle_write' - unexpected unlock
block/blk-zoned.c:1747:31: warning: context imbalance in
'blk_revalidate_seq_zone' - unexpected unlock
block/bfq-iosched.c:5498:9: warning: context imbalance in
'bfq_exit_icq' - different lock contexts for basic block
This series contains changes to resolve some of those issues. Some issues
are not addressed, as follows:
- for zoned stuff, as mentioned by Damien at [0], they are not worth
solving
- for blk-iocost.c issues, at [1] Tejun says that lockdep does a better
job (than sparse)
- for blk-cgroup.c issues, at [2] again Tejun says that lockdep does a
better job
I am reposting the patch from Jiapeng Chong, as it was never picked up.
[0] https://lore.kernel.org/linux-block/[email protected]/
[1] https://lore.kernel.org/linux-block/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
Jiapeng Chong (1):
bdev: make blockdev_mnt static
John Garry (2):
block: Drop locking annotation for limits_lock
block: BFQ: Refactor bfq_exit_icq() to silence sparse warning
block/bdev.c | 2 +-
block/bfq-iosched.c | 38 ++++++++++++++++++++------------------
block/blk-settings.c | 1 -
include/linux/blkdev.h | 1 -
4 files changed, 21 insertions(+), 21 deletions(-)
--
2.31.1
Currently compiling block/blk-settings.c with C=1 gives the following
warning:
block/blk-settings.c:262:9: warning: context imbalance in 'queue_limits_commit_update' - wrong count at exit
request_queue.limits_lock is a mutex. Sparse locking annotation for
mutexes are currently not supported - see [0] - so drop that locking
annotation.
[0] https://lore.kernel.org/lkml/[email protected]/T/#mbb8bda6c0a7ca7ce19f46df976a8e3b489745488
Fixes: d690cb8ae14bd ("block: add an API to atomically update queue limits")
Signed-off-by: John Garry <[email protected]>
---
block/blk-settings.c | 1 -
include/linux/blkdev.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index effeb9a639bb..cdc7845260b9 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -254,7 +254,6 @@ int blk_set_default_limits(struct queue_limits *lim)
*/
int queue_limits_commit_update(struct request_queue *q,
struct queue_limits *lim)
- __releases(q->limits_lock)
{
int error = blk_validate_limits(lim);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 24c36929920b..f798c9c6eb30 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -904,7 +904,6 @@ static inline unsigned int blk_chunk_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;
--
2.31.1
Currently building for C=1 generates the following warning:
block/bfq-iosched.c:5498:9: warning: context imbalance in 'bfq_exit_icq' - different lock contexts for basic block
Refactor bfq_exit_icq() into a core part which loops for the actuators,
and only lock calling this routine when necessary.
Signed-off-by: John Garry <[email protected]>
---
block/bfq-iosched.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 4b88a54a9b76..36a4998c4b37 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5463,40 +5463,42 @@ static void bfq_exit_icq_bfqq(struct bfq_io_cq *bic, bool is_sync,
}
}
+static void _bfq_exit_icq(struct bfq_io_cq *bic, unsigned int num_actuators)
+{
+ struct bfq_iocq_bfqq_data *bfqq_data = bic->bfqq_data;
+ unsigned int act_idx;
+
+ for (act_idx = 0; act_idx < num_actuators; act_idx++) {
+ if (bfqq_data[act_idx].stable_merge_bfqq)
+ bfq_put_stable_ref(bfqq_data[act_idx].stable_merge_bfqq);
+
+ bfq_exit_icq_bfqq(bic, true, act_idx);
+ bfq_exit_icq_bfqq(bic, false, act_idx);
+ }
+}
+
static void bfq_exit_icq(struct io_cq *icq)
{
struct bfq_io_cq *bic = icq_to_bic(icq);
struct bfq_data *bfqd = bic_to_bfqd(bic);
unsigned long flags;
- unsigned int act_idx;
+
/*
* If bfqd and thus bfqd->num_actuators is not available any
* longer, then cycle over all possible per-actuator bfqqs in
* next loop. We rely on bic being zeroed on creation, and
* therefore on its unused per-actuator fields being NULL.
- */
- unsigned int num_actuators = BFQ_MAX_ACTUATORS;
- struct bfq_iocq_bfqq_data *bfqq_data = bic->bfqq_data;
-
- /*
+ *
* bfqd is NULL if scheduler already exited, and in that case
* this is the last time these queues are accessed.
*/
if (bfqd) {
spin_lock_irqsave(&bfqd->lock, flags);
- num_actuators = bfqd->num_actuators;
- }
-
- for (act_idx = 0; act_idx < num_actuators; act_idx++) {
- if (bfqq_data[act_idx].stable_merge_bfqq)
- bfq_put_stable_ref(bfqq_data[act_idx].stable_merge_bfqq);
-
- bfq_exit_icq_bfqq(bic, true, act_idx);
- bfq_exit_icq_bfqq(bic, false, act_idx);
- }
-
- if (bfqd)
+ _bfq_exit_icq(bic, bfqd->num_actuators);
spin_unlock_irqrestore(&bfqd->lock, flags);
+ } else {
+ _bfq_exit_icq(bic, BFQ_MAX_ACTUATORS);
+ }
}
/*
--
2.31.1
On 6/14/24 2:03 AM, John Garry wrote:
> Currently compiling block/blk-settings.c with C=1 gives the following
> warning:
> block/blk-settings.c:262:9: warning: context imbalance in 'queue_limits_commit_update' - wrong count at exit
>
> request_queue.limits_lock is a mutex. Sparse locking annotation for
> mutexes are currently not supported - see [0] - so drop that locking
> annotation.
Reviewed-by: Bart Van Assche <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On 6/14/24 2:03 AM, John Garry wrote:
> Currently building for C=1 generates the following warning:
> block/bfq-iosched.c:5498:9: warning: context imbalance in 'bfq_exit_icq' - different lock contexts for basic block
>
> Refactor bfq_exit_icq() into a core part which loops for the actuators,
> and only lock calling this routine when necessary.
I like this patch because it makes the code easier to read.
Reviewed-by: Bart Van Assche <[email protected]>