2022-08-16 07:28:04

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 0/3] bfq simple cleanups

From: Yu Kuai <[email protected]>

Yu Kuai (3):
block: bfq: remove unused functions
block, bfq: remove useless checking in bfq_put_queue()
block, bfq: remove useless parameter for bfq_add/del_bfqq_busy()

block/bfq-cgroup.c | 5 -----
block/bfq-iosched.c | 14 ++++++--------
block/bfq-iosched.h | 18 ++++++++++--------
block/bfq-wf2q.c | 9 ++++++---
4 files changed, 22 insertions(+), 24 deletions(-)

--
2.31.1


2022-08-16 07:32:01

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 2/3] block, bfq: remove useless checking in bfq_put_queue()

From: Yu Kuai <[email protected]>

'bfqq->bfqd' is ensured to set in bfq_init_queue(), and it will never
change afterwards.

Signed-off-by: Yu Kuai <[email protected]>
---
block/bfq-iosched.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index c740b41fe0a4..f39067389b2b 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5255,9 +5255,7 @@ void bfq_put_queue(struct bfq_queue *bfqq)
struct hlist_node *n;
struct bfq_group *bfqg = bfqq_group(bfqq);

- if (bfqq->bfqd)
- bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p %d",
- bfqq, bfqq->ref);
+ bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p %d", bfqq, bfqq->ref);

bfqq->ref--;
if (bfqq->ref)
@@ -5321,7 +5319,7 @@ void bfq_put_queue(struct bfq_queue *bfqq)
hlist_del_init(&item->woken_list_node);
}

- if (bfqq->bfqd && bfqq->bfqd->last_completed_rq_bfqq == bfqq)
+ if (bfqq->bfqd->last_completed_rq_bfqq == bfqq)
bfqq->bfqd->last_completed_rq_bfqq = NULL;

kmem_cache_free(bfq_pool, bfqq);
--
2.31.1

2022-08-16 07:34:46

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 1/3] block, bfq: remove unused functions

From: Yu Kuai <[email protected]>

While doing code coverage testing(CONFIG_BFQ_CGROUP_DEBUG is disabled), we
found that some functions doesn't have caller, thus remove them.

Signed-off-by: Yu Kuai <[email protected]>
---
block/bfq-cgroup.c | 5 -----
block/bfq-iosched.h | 13 ++++++++-----
2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 30b15a9a47c4..144bca006463 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -254,17 +254,12 @@ void bfqg_stats_update_completion(struct bfq_group *bfqg, u64 start_time_ns,

#else /* CONFIG_BFQ_CGROUP_DEBUG */

-void bfqg_stats_update_io_add(struct bfq_group *bfqg, struct bfq_queue *bfqq,
- blk_opf_t opf) { }
void bfqg_stats_update_io_remove(struct bfq_group *bfqg, blk_opf_t opf) { }
void bfqg_stats_update_io_merged(struct bfq_group *bfqg, blk_opf_t opf) { }
void bfqg_stats_update_completion(struct bfq_group *bfqg, u64 start_time_ns,
u64 io_start_time_ns, blk_opf_t opf) { }
void bfqg_stats_update_dequeue(struct bfq_group *bfqg) { }
-void bfqg_stats_set_start_empty_time(struct bfq_group *bfqg) { }
-void bfqg_stats_update_idle_time(struct bfq_group *bfqg) { }
void bfqg_stats_set_start_idle_time(struct bfq_group *bfqg) { }
-void bfqg_stats_update_avg_queue_size(struct bfq_group *bfqg) { }

#endif /* CONFIG_BFQ_CGROUP_DEBUG */

diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
index ad8e513d7e87..f81ab3c8fa3c 100644
--- a/block/bfq-iosched.h
+++ b/block/bfq-iosched.h
@@ -993,20 +993,23 @@ void bfq_put_async_queues(struct bfq_data *bfqd, struct bfq_group *bfqg);
/* ---------------- cgroups-support interface ---------------- */

void bfqg_stats_update_legacy_io(struct request_queue *q, struct request *rq);
-void bfqg_stats_update_io_add(struct bfq_group *bfqg, struct bfq_queue *bfqq,
- blk_opf_t opf);
void bfqg_stats_update_io_remove(struct bfq_group *bfqg, blk_opf_t opf);
void bfqg_stats_update_io_merged(struct bfq_group *bfqg, blk_opf_t opf);
void bfqg_stats_update_completion(struct bfq_group *bfqg, u64 start_time_ns,
u64 io_start_time_ns, blk_opf_t opf);
void bfqg_stats_update_dequeue(struct bfq_group *bfqg);
-void bfqg_stats_set_start_empty_time(struct bfq_group *bfqg);
-void bfqg_stats_update_idle_time(struct bfq_group *bfqg);
void bfqg_stats_set_start_idle_time(struct bfq_group *bfqg);
-void bfqg_stats_update_avg_queue_size(struct bfq_group *bfqg);
void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq,
struct bfq_group *bfqg);

+#ifdef CONFIG_BFQ_CGROUP_DEBUG
+void bfqg_stats_update_io_add(struct bfq_group *bfqg, struct bfq_queue *bfqq,
+ blk_opf_t opf);
+void bfqg_stats_set_start_empty_time(struct bfq_group *bfqg);
+void bfqg_stats_update_idle_time(struct bfq_group *bfqg);
+void bfqg_stats_update_avg_queue_size(struct bfq_group *bfqg);
+#endif
+
void bfq_init_entity(struct bfq_entity *entity, struct bfq_group *bfqg);
void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio);
void bfq_end_wr_async(struct bfq_data *bfqd);
--
2.31.1

2022-08-16 07:36:08

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 3/3] block, bfq: remove useless parameter for bfq_add/del_bfqq_busy()

From: Yu Kuai <[email protected]>

'bfqd' can be accessed through 'bfqq->bfqd', there is no need to pass
it as a parameter separately.

Signed-off-by: Yu Kuai <[email protected]>
---
block/bfq-iosched.c | 8 ++++----
block/bfq-iosched.h | 5 ++---
block/bfq-wf2q.c | 9 ++++++---
3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index f39067389b2b..7ea427817f7f 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -1925,7 +1925,7 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
bfqq->service_from_backlogged = 0;
bfq_clear_bfqq_softrt_update(bfqq);

- bfq_add_bfqq_busy(bfqd, bfqq);
+ bfq_add_bfqq_busy(bfqq);

/*
* Expire in-service queue if preemption may be needed for
@@ -2419,7 +2419,7 @@ static void bfq_remove_request(struct request_queue *q,
bfqq->next_rq = NULL;

if (bfq_bfqq_busy(bfqq) && bfqq != bfqd->in_service_queue) {
- bfq_del_bfqq_busy(bfqd, bfqq, false);
+ bfq_del_bfqq_busy(bfqq, false);
/*
* bfqq emptied. In normal operation, when
* bfqq is empty, bfqq->entity.service and
@@ -3098,7 +3098,7 @@ void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq)
*/
if (bfq_bfqq_busy(bfqq) && RB_EMPTY_ROOT(&bfqq->sort_list) &&
bfqq != bfqd->in_service_queue)
- bfq_del_bfqq_busy(bfqd, bfqq, false);
+ bfq_del_bfqq_busy(bfqq, false);

bfq_reassign_last_bfqq(bfqq, NULL);

@@ -3908,7 +3908,7 @@ static bool __bfq_bfqq_expire(struct bfq_data *bfqd, struct bfq_queue *bfqq,
*/
bfqq->budget_timeout = jiffies;

- bfq_del_bfqq_busy(bfqd, bfqq, true);
+ bfq_del_bfqq_busy(bfqq, true);
} else {
bfq_requeue_bfqq(bfqd, bfqq, true);
/*
diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
index f81ab3c8fa3c..64ee618064ba 100644
--- a/block/bfq-iosched.h
+++ b/block/bfq-iosched.h
@@ -1080,9 +1080,8 @@ void bfq_deactivate_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
void bfq_activate_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq);
void bfq_requeue_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
bool expiration);
-void bfq_del_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq,
- bool expiration);
-void bfq_add_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq);
+void bfq_del_bfqq_busy(struct bfq_queue *bfqq, bool expiration);
+void bfq_add_bfqq_busy(struct bfq_queue *bfqq);

/* --------------- end of interface of B-WF2Q+ ---------------- */

diff --git a/block/bfq-wf2q.c b/block/bfq-wf2q.c
index 983413cdefad..8fc3da4c23bb 100644
--- a/block/bfq-wf2q.c
+++ b/block/bfq-wf2q.c
@@ -1651,9 +1651,10 @@ void bfq_requeue_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
* the service tree. As a special case, it can be invoked during an
* expiration.
*/
-void bfq_del_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq,
- bool expiration)
+void bfq_del_bfqq_busy(struct bfq_queue *bfqq, bool expiration)
{
+ struct bfq_data *bfqd = bfqq->bfqd;
+
bfq_log_bfqq(bfqd, bfqq, "del from busy");

bfq_clear_bfqq_busy(bfqq);
@@ -1674,8 +1675,10 @@ void bfq_del_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq,
/*
* Called when an inactive queue receives a new request.
*/
-void bfq_add_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq)
+void bfq_add_bfqq_busy(struct bfq_queue *bfqq)
{
+ struct bfq_data *bfqd = bfqq->bfqd;
+
bfq_log_bfqq(bfqd, bfqq, "add to busy");

bfq_activate_bfqq(bfqd, bfqq);
--
2.31.1

2022-08-17 12:03:53

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH -next 1/3] block, bfq: remove unused functions

On Tue 16-08-22 09:56:29, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> While doing code coverage testing(CONFIG_BFQ_CGROUP_DEBUG is disabled), we
> found that some functions doesn't have caller, thus remove them.
>
> Signed-off-by: Yu Kuai <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> block/bfq-cgroup.c | 5 -----
> block/bfq-iosched.h | 13 ++++++++-----
> 2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index 30b15a9a47c4..144bca006463 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -254,17 +254,12 @@ void bfqg_stats_update_completion(struct bfq_group *bfqg, u64 start_time_ns,
>
> #else /* CONFIG_BFQ_CGROUP_DEBUG */
>
> -void bfqg_stats_update_io_add(struct bfq_group *bfqg, struct bfq_queue *bfqq,
> - blk_opf_t opf) { }
> void bfqg_stats_update_io_remove(struct bfq_group *bfqg, blk_opf_t opf) { }
> void bfqg_stats_update_io_merged(struct bfq_group *bfqg, blk_opf_t opf) { }
> void bfqg_stats_update_completion(struct bfq_group *bfqg, u64 start_time_ns,
> u64 io_start_time_ns, blk_opf_t opf) { }
> void bfqg_stats_update_dequeue(struct bfq_group *bfqg) { }
> -void bfqg_stats_set_start_empty_time(struct bfq_group *bfqg) { }
> -void bfqg_stats_update_idle_time(struct bfq_group *bfqg) { }
> void bfqg_stats_set_start_idle_time(struct bfq_group *bfqg) { }
> -void bfqg_stats_update_avg_queue_size(struct bfq_group *bfqg) { }
>
> #endif /* CONFIG_BFQ_CGROUP_DEBUG */
>
> diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
> index ad8e513d7e87..f81ab3c8fa3c 100644
> --- a/block/bfq-iosched.h
> +++ b/block/bfq-iosched.h
> @@ -993,20 +993,23 @@ void bfq_put_async_queues(struct bfq_data *bfqd, struct bfq_group *bfqg);
> /* ---------------- cgroups-support interface ---------------- */
>
> void bfqg_stats_update_legacy_io(struct request_queue *q, struct request *rq);
> -void bfqg_stats_update_io_add(struct bfq_group *bfqg, struct bfq_queue *bfqq,
> - blk_opf_t opf);
> void bfqg_stats_update_io_remove(struct bfq_group *bfqg, blk_opf_t opf);
> void bfqg_stats_update_io_merged(struct bfq_group *bfqg, blk_opf_t opf);
> void bfqg_stats_update_completion(struct bfq_group *bfqg, u64 start_time_ns,
> u64 io_start_time_ns, blk_opf_t opf);
> void bfqg_stats_update_dequeue(struct bfq_group *bfqg);
> -void bfqg_stats_set_start_empty_time(struct bfq_group *bfqg);
> -void bfqg_stats_update_idle_time(struct bfq_group *bfqg);
> void bfqg_stats_set_start_idle_time(struct bfq_group *bfqg);
> -void bfqg_stats_update_avg_queue_size(struct bfq_group *bfqg);
> void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> struct bfq_group *bfqg);
>
> +#ifdef CONFIG_BFQ_CGROUP_DEBUG
> +void bfqg_stats_update_io_add(struct bfq_group *bfqg, struct bfq_queue *bfqq,
> + blk_opf_t opf);
> +void bfqg_stats_set_start_empty_time(struct bfq_group *bfqg);
> +void bfqg_stats_update_idle_time(struct bfq_group *bfqg);
> +void bfqg_stats_update_avg_queue_size(struct bfq_group *bfqg);
> +#endif
> +
> void bfq_init_entity(struct bfq_entity *entity, struct bfq_group *bfqg);
> void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio);
> void bfq_end_wr_async(struct bfq_data *bfqd);
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2022-08-17 12:03:59

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH -next 3/3] block, bfq: remove useless parameter for bfq_add/del_bfqq_busy()

On Tue 16-08-22 09:56:31, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> 'bfqd' can be accessed through 'bfqq->bfqd', there is no need to pass
> it as a parameter separately.
>
> Signed-off-by: Yu Kuai <[email protected]>

I agree. That also seemed a bit strange to me in bfq code. Feel free to
add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> block/bfq-iosched.c | 8 ++++----
> block/bfq-iosched.h | 5 ++---
> block/bfq-wf2q.c | 9 ++++++---
> 3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index f39067389b2b..7ea427817f7f 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -1925,7 +1925,7 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
> bfqq->service_from_backlogged = 0;
> bfq_clear_bfqq_softrt_update(bfqq);
>
> - bfq_add_bfqq_busy(bfqd, bfqq);
> + bfq_add_bfqq_busy(bfqq);
>
> /*
> * Expire in-service queue if preemption may be needed for
> @@ -2419,7 +2419,7 @@ static void bfq_remove_request(struct request_queue *q,
> bfqq->next_rq = NULL;
>
> if (bfq_bfqq_busy(bfqq) && bfqq != bfqd->in_service_queue) {
> - bfq_del_bfqq_busy(bfqd, bfqq, false);
> + bfq_del_bfqq_busy(bfqq, false);
> /*
> * bfqq emptied. In normal operation, when
> * bfqq is empty, bfqq->entity.service and
> @@ -3098,7 +3098,7 @@ void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq)
> */
> if (bfq_bfqq_busy(bfqq) && RB_EMPTY_ROOT(&bfqq->sort_list) &&
> bfqq != bfqd->in_service_queue)
> - bfq_del_bfqq_busy(bfqd, bfqq, false);
> + bfq_del_bfqq_busy(bfqq, false);
>
> bfq_reassign_last_bfqq(bfqq, NULL);
>
> @@ -3908,7 +3908,7 @@ static bool __bfq_bfqq_expire(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> */
> bfqq->budget_timeout = jiffies;
>
> - bfq_del_bfqq_busy(bfqd, bfqq, true);
> + bfq_del_bfqq_busy(bfqq, true);
> } else {
> bfq_requeue_bfqq(bfqd, bfqq, true);
> /*
> diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
> index f81ab3c8fa3c..64ee618064ba 100644
> --- a/block/bfq-iosched.h
> +++ b/block/bfq-iosched.h
> @@ -1080,9 +1080,8 @@ void bfq_deactivate_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> void bfq_activate_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq);
> void bfq_requeue_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> bool expiration);
> -void bfq_del_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> - bool expiration);
> -void bfq_add_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq);
> +void bfq_del_bfqq_busy(struct bfq_queue *bfqq, bool expiration);
> +void bfq_add_bfqq_busy(struct bfq_queue *bfqq);
>
> /* --------------- end of interface of B-WF2Q+ ---------------- */
>
> diff --git a/block/bfq-wf2q.c b/block/bfq-wf2q.c
> index 983413cdefad..8fc3da4c23bb 100644
> --- a/block/bfq-wf2q.c
> +++ b/block/bfq-wf2q.c
> @@ -1651,9 +1651,10 @@ void bfq_requeue_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> * the service tree. As a special case, it can be invoked during an
> * expiration.
> */
> -void bfq_del_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> - bool expiration)
> +void bfq_del_bfqq_busy(struct bfq_queue *bfqq, bool expiration)
> {
> + struct bfq_data *bfqd = bfqq->bfqd;
> +
> bfq_log_bfqq(bfqd, bfqq, "del from busy");
>
> bfq_clear_bfqq_busy(bfqq);
> @@ -1674,8 +1675,10 @@ void bfq_del_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> /*
> * Called when an inactive queue receives a new request.
> */
> -void bfq_add_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq)
> +void bfq_add_bfqq_busy(struct bfq_queue *bfqq)
> {
> + struct bfq_data *bfqd = bfqq->bfqd;
> +
> bfq_log_bfqq(bfqd, bfqq, "add to busy");
>
> bfq_activate_bfqq(bfqd, bfqq);
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2022-08-17 12:26:56

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH -next 2/3] block, bfq: remove useless checking in bfq_put_queue()

On Tue 16-08-22 09:56:30, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> 'bfqq->bfqd' is ensured to set in bfq_init_queue(), and it will never
> change afterwards.
>
> Signed-off-by: Yu Kuai <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> block/bfq-iosched.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index c740b41fe0a4..f39067389b2b 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -5255,9 +5255,7 @@ void bfq_put_queue(struct bfq_queue *bfqq)
> struct hlist_node *n;
> struct bfq_group *bfqg = bfqq_group(bfqq);
>
> - if (bfqq->bfqd)
> - bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p %d",
> - bfqq, bfqq->ref);
> + bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p %d", bfqq, bfqq->ref);
>
> bfqq->ref--;
> if (bfqq->ref)
> @@ -5321,7 +5319,7 @@ void bfq_put_queue(struct bfq_queue *bfqq)
> hlist_del_init(&item->woken_list_node);
> }
>
> - if (bfqq->bfqd && bfqq->bfqd->last_completed_rq_bfqq == bfqq)
> + if (bfqq->bfqd->last_completed_rq_bfqq == bfqq)
> bfqq->bfqd->last_completed_rq_bfqq = NULL;
>
> kmem_cache_free(bfq_pool, bfqq);
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2022-08-19 21:15:20

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH -next 0/3] bfq simple cleanups

On Tue, 16 Aug 2022 09:56:28 +0800, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> Yu Kuai (3):
> block: bfq: remove unused functions
> block, bfq: remove useless checking in bfq_put_queue()
> block, bfq: remove useless parameter for bfq_add/del_bfqq_busy()
>
> [...]

Applied, thanks!

[1/3] block, bfq: remove unused functions
commit: 83501be67dbabc4a19ca7cf0f7406cc200272880
[2/3] block, bfq: remove useless checking in bfq_put_queue()
commit: bf2c5a1d2aa31faf3cae0158da2a301acfa0c9fd
[3/3] block, bfq: remove useless parameter for bfq_add/del_bfqq_busy()
commit: f5f9d71bc15eaa72f70cd49aa507b0809a0325ae

Best regards,
--
Jens Axboe