Changes in v2:
- add reviewed-by tag for patch 1
- use WRITE_ONCE() for updating of 'bfqd->queued' in patch 2
This patchset try to make bfq_has_work() more accurate, patch 1 is a
small problem found by code review.
BTW, I not sure why blk_mq_run_hw_queues() is called with 'bfqd->lock'
held, I think this is not necessary. And bfq_has_work() can be more
accurate by reading 'bfqd->queued' with 'bfqd->lock' held after patch 2.
Previous versions:
v1: https://lore.kernel.org/all/[email protected]/
Yu Kuai (2):
block, bfq: protect 'bfqd->queued' by 'bfqd->lock'
block, bfq: make bfq_has_work() more accurate
block/bfq-iosched.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
--
2.31.1
If bfq_schedule_dispatch() is called from bfq_idle_slice_timer_body(),
then 'bfqd->queued' is read without holding 'bfqd->lock'. This is
wrong since it can be wrote concurrently.
Fix the problem by holding 'bfqd->lock' in such case.
Signed-off-by: Yu Kuai <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
block/bfq-iosched.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 272d48d8f326..61750696e87f 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -456,6 +456,8 @@ static struct bfq_io_cq *bfq_bic_lookup(struct request_queue *q)
*/
void bfq_schedule_dispatch(struct bfq_data *bfqd)
{
+ lockdep_assert_held(&bfqd->lock);
+
if (bfqd->queued != 0) {
bfq_log(bfqd, "schedule dispatch");
blk_mq_run_hw_queues(bfqd->queue, true);
@@ -6898,8 +6900,8 @@ bfq_idle_slice_timer_body(struct bfq_data *bfqd, struct bfq_queue *bfqq)
bfq_bfqq_expire(bfqd, bfqq, true, reason);
schedule_dispatch:
- spin_unlock_irqrestore(&bfqd->lock, flags);
bfq_schedule_dispatch(bfqd);
+ spin_unlock_irqrestore(&bfqd->lock, flags);
}
/*
--
2.31.1
On 5/12/2022 7:35 PM, Yu Kuai wrote:
> If bfq_schedule_dispatch() is called from bfq_idle_slice_timer_body(),
> then 'bfqd->queued' is read without holding 'bfqd->lock'. This is
> wrong since it can be wrote concurrently.
>
> Fix the problem by holding 'bfqd->lock' in such case.
>
> Signed-off-by: Yu Kuai <[email protected]>
> Reviewed-by: Jan Kara <[email protected]>
> ---
Reviewed-by: Chaitanya Kulkarni <[email protected]>
-ck
On Fri, 13 May 2022 10:35:05 +0800, Yu Kuai wrote:
> Changes in v2:
> - add reviewed-by tag for patch 1
> - use WRITE_ONCE() for updating of 'bfqd->queued' in patch 2
>
> This patchset try to make bfq_has_work() more accurate, patch 1 is a
> small problem found by code review.
>
> [...]
Applied, thanks!
[1/2] block, bfq: protect 'bfqd->queued' by 'bfqd->lock'
commit: 181490d5321806e537dc5386db5ea640b826bf78
[2/2] block, bfq: make bfq_has_work() more accurate
commit: ddc25c86b466d2359b57bc7798f167baa1735a44
Best regards,
--
Jens Axboe