On Wed, 24 May 2023 16:26:45 +0800
Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
> Hi,
>
> 在 2023/05/24 15:53, Mariusz Tkaczyk 写道:
> > On Tue, 23 May 2023 21:38:56 +0800
> > Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
> >
> >> From: Yu Kuai <yukuai3@xxxxxxxxxx>
> >>
> >> Pluged bio is unlimited means that all submitted bio will be pluged, and
> >> those bio won't be issued to underlaying disks until blk_finish_plug() or
> >> blk_flush_plug(). In this case, a lot memory will be used for
> >> raid10_bio and io latency will be very bad.
> >>
> >> This test do some dirty pages writeback for raid10, where plug is used, and
> >> check if device inflight counter exceed threshold.
> >>
> >> This problem is supposed to be fixed by [1].
> >
> > The test here is for md, mdadm has nothing to do here. I'm not against it
> > but please extract it to separate directory because like "md_tests".
> > We need to start grouping tests.
>
> Sorry I don't understand this, currently the test for md is here, I
> don't see why need a seperate directory, is there a plan to move all
> these tests into new directory?
Yes, that how it was in the past, and I would like to change it because
tests directory is too big and we are still adding new tests.
Ideally, tests like that should stay with kernel, because repository you are
contributing now is "mdadm" - userspace application to manage md devices, not
the driver itself.
That is why I wanted to highlight that placement is controversial but I'm fine
with that. As you said, we did that in the past.
Song, what do you think? Where we should put the test like in the future?
> >> +
> >> +# check if inflight exceed threshold
> >> +while true; do
> >> + tmp=`cat /sys/block/md0/inflight | awk '{printf("%d\n", $1 +
> >> $2);}'`
> >> + if [ $tmp -gt $threshold ]; then
> >> + die "inflight is greater than 4096"
> >
> > The message here is not meaningful, what 4096 is? Please add comment
> > describing why value above 4096 causes an error. We need to understand how
> > the future changes in md may affect this setting (I think that there is a
> > correlation between the value and MAX_PLUG_BIO).
>
> MAX_PLUG_BIO is just limit for one task, I'm not sure if there will only
> be one task issuing io, that why I choose a much larger value 4096.
Please add it in comment above the value.
Thanks,
Mariusz
