Re: [PATCH] block: check more requests for multiple_queues in blk_attempt_plug_merge

From: Wilson Jonathan <i400sjon@gmail.com>
To: Song Liu <song@kernel.org>, Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org,
	linux-raid <linux-raid@vger.kernel.org>,
	stable@vger.kernel.org, Larkin Lowrey <llowrey@nuclearwinter.com>,
	Roger Heflin <rogerheflin@gmail.com>
Subject: Re: [PATCH] block: check more requests for multiple_queues in blk_attempt_plug_merge
Date: Fri, 11 Mar 2022 11:30:42 +0000	[thread overview]
Message-ID: <0d4088b987437788846b7d69879189f4870b90c6.camel@gmail.com> (raw)
In-Reply-To: <CAPhsuW5zX96VaBMu-o=JUqDz2KLRBcNFM_gEsT=tHjeYqrngSQ@mail.gmail.com>

On Thu, 2022-03-10 at 14:37 -0800, Song Liu wrote:
> On Thu, Mar 10, 2022 at 2:15 PM Jens Axboe <axboe@kernel.dk> wrote:
> > 
> > On 3/8/22 11:42 PM, Song Liu wrote:
> > > RAID arrays check/repair operations benefit a lot from merging
> > > requests.
> > > If we only check the previous entry for merge attempt, many merge
> > > will be
> > > missed. As a result, significant regression is observed for RAID
> > > check
> > > and repair.
> > > 
> > > Fix this by checking more than just the previous entry when
> > > plug->multiple_queues == true.
> > > 
> > > This improves the check/repair speed of a 20-HDD raid6 from 19
> > > MB/s to
> > > 103 MB/s.
> > 
> > Do the underlying disks not have an IO scheduler attached? Curious
> > why
> > the merges aren't being done there, would be trivial when the list
> > is
> > flushed out. Because if the perf difference is that big, then other
> > workloads would be suffering they are that sensitive to being
> > within a
> > plug worth of IO.
> 
> The disks have mq-deadline by default. I also tried kyber, the result
> is the
> same. Raid repair work sends IOs to all the HDDs in a round-robin
> manner.
> If we only check the previous request, there isn't much opportunity
> for
> merge. I guess other workloads may have different behavior?
> 
> > Between your two approaches, I do greatly prefer the first one
> > though.
> 
> I also like the first one better. But I am not sure whether it will
> slow down
> other workloads. We can probably also make the second one cleaner
> with a new variation of blk_start_plug.

As a matter of note and purely anecdotal: Before the raid "check" slow
down/regression my system would be responsive but delayed (opening a
program or opening the xface application menu or switching a file in
VLC would take longer than normal, fractions of seconds to a second but
slugish and notacable) and with the regression that slow down went from
annoying to unbearable. 

The slowdowns (in programs and menus and file changes) also *seems* to
get worse (in both pre & post regression) the longer the check has been
running and the slower a run naturally gets (I assume as the check
moves from the outer portion of the disk to the inner portion?) and the
lower the KB's reported in cat /proc/mdstat/.

In the post regression situation it wasn't just that the check was
taking much longer and was much slower it was also that it slowed down
everything else to the point that it was painful to try and use the
computer as it was so much less responsive (multiple seconds for
anything to load/run/swtch; even web pages). A laggy annoyance had
become an actual hindrance. 

I have no idea why the speed of the "check" would seemingly affect the
apparent responsiveness of the computer and why it would appear that
the slower the check the slower the responsiveness. 

> 
> Thanks,
> Song