Re: [PATCH] block/mq-deadline: Fix WARN when set async_depth by sysfs

From: Zhiguo Niu <niuzhiguo84@gmail.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Zhiguo Niu <zhiguo.niu@unisoc.com>,
	axboe@kernel.dk, linux-kernel@vger.kernel.org,
	 linux-block@vger.kernel.org, ke.wang@unisoc.com,
	hongyu.jin@unisoc.com,  Damien Le Moal <dlemoal@kernel.org>
Subject: Re: [PATCH] block/mq-deadline: Fix WARN when set async_depth by sysfs
Date: Mon, 1 Apr 2024 08:58:04 +0800	[thread overview]
Message-ID: <CAHJ8P3K9OL6MHNrSrqmf0esbr2h1HJ3mVRmxDNVpf95ZMHQcqg@mail.gmail.com> (raw)
In-Reply-To: <890bd06d-2a94-4138-9854-4a7ed74e0e51@acm.org>

On Sat, Mar 30, 2024 at 2:08 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 3/28/24 7:44 PM, Zhiguo Niu wrote:
> > diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> > index 02a916b..89c516e 100644
> > --- a/block/mq-deadline.c
> > +++ b/block/mq-deadline.c
> > @@ -646,10 +646,12 @@ static void dd_depth_updated(struct blk_mq_hw_ctx *hctx)
> >       struct request_queue *q = hctx->queue;
> >       struct deadline_data *dd = q->elevator->elevator_data;
> >       struct blk_mq_tags *tags = hctx->sched_tags;
> > +     unsigned int shift = tags->bitmap_tags.sb.shift;
> > +     unsigned int dd_min_depth = max(1U, 3 * (1U << shift)  / 4);
> >
> >       dd->async_depth = max(1UL, 3 * q->nr_requests / 4);
> >
> > -     sbitmap_queue_min_shallow_depth(&tags->bitmap_tags, dd->async_depth);
> > +     sbitmap_queue_min_shallow_depth(&tags->bitmap_tags, dd_min_depth);
> >   }
>
> The above patch sets min_shallow_depth to the same value as commit
> d47f9717e5cf ("block/mq-deadline: use correct way to throttling write
> requests"). That commit got reverted because it was causing performance
> problems. So the above patch reintroduces the performance problem that
> has been fixed by commit 256aab46e316 ("Revert "block/mq-deadline: use
> correct way to throttling write requests"").
Hi Bart Van Assche,

This  patch only modifies the initial minimum value of
min_shallow_depth and does not change "dd->async_depth",
so it will not cause performance problems like the previous patch
(d47f9717e5cf ("block/mq-deadline: use correct way to throttling write
> requests")).
>
> Thank you for attempting to reintroduce a problem that just got fixed
> without even mentioning that this is an attempt to reintroduce a
> performance problem.

So what are your suggestions for fixing the warning shown in commit
msg if dd->async_depth is set by the user from sysfs?
thanks
>
> Bart.
>
>