---- 在 星期日, 2019-10-27 21:59:36 Amir Goldstein <amir73il@xxxxxxxxx> 撰写 ----
> On Fri, Oct 25, 2019 at 4:19 AM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote:
> >
> > ---- 在 星期五, 2019-10-25 05:02:07 Amir Goldstein <amir73il@xxxxxxxxx> 撰写 ----
> > > On Thu, Oct 24, 2019 at 3:29 PM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote:
> > > >
> > > > This is intensive copy-up test for sparse files,
> > > > these cases will be mainly used for regression test
> > > > of copy-up improvement for sparse files.
> > > >
> > > > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > > Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxxxx>
> > > >
> > > > ---
> > > > v1->v2:
> > > > - Call _get_block_size to get fs block size.
> > > > - Add comment for test space requirement.
> > > > - Print meaningful error message when copy-up fail.
> > > > - Adjust random hole range to 1M~5M.
> > > > - Fix typo.
> > > >
> > > > v2->v3:
> > > > - Fix space requiremnt for test.
> > > > - Add more descriptions for test files and hole patterns.
> > > > - Define well named variables to replace unexplained numbers.
> > > > - Fix random hole algorithm to what Amir suggested.
> > > > - Adjust iosize to start from 1K.
>
> Chengguang,
>
> Sorry, I did't notice that you did that. Why?
> As you can see below, this change has a very bad impact on test run time.
> Any reason not to use _get_block_size?
Use _get_block_size cannot mitigate the effect perfectly,
in the worst case that we formatted fs with blocksize=1K,
the test will take long time and also test time is not fixed.
>
>
> > > > - Remove from quick test group.
> > >
> > > Why? you said it takes 7s without the kernel patch.
> > > The test overlay/001 is in quick group and it copies up 2*4GB
> > > sparse files.
> >
> > I noticed that after changed to start from 1K iosize the test took about 23s.
> > I'm afraid maybe it will take more time on low performance VM env.
> >
> > The test overlay/001 took 8s/1s with/without kernel patch, so mainly test time
> > wasted on creating test files on test overlay/066.
>
> You are correct about the time spent on creating the files, but...
>
> On my low perf VM, the test runs 95s with overlay over xfs+reflink
>
> But if I set start iosize=4 (which what my fs block size is) the test
> runs only 30s.
>
> IOW, most of the test time is spent on creating the files with small iosize
> below fs block size, which doesn't test copy up of holes at all.
>
> If I further change file size to be a multiply of iosize (x10),
> test run time drops to 6s!
> I don't think we loose too much test coverage if we do that?
> If anything we gain testing different file sizes.
hmm, for small iosize the file size is even smaller than
copy-up CHUNK SIZE(1M), so that all contents(data+hole)
will be passed at once, I'm not very sure is it helpful for
hole copy-up logic in kernel patch. What do you think?
>
> The disk space requirement formula for ${iosize}K_holefiles becomes:
> 10*(2^0 + 2^11)K*12/2 =~ 10 * 1024 * 12
That's the mean of 12/2 ?
> same as before, just needs explaining.
> (the formula assumes the worst case of min_iosize=1)
>
> -------------
> #
> # |-- hole --|-- data --| ... |-- data --|-- hole --|
>
> -iosize=1
> +min_iosize=$(($(_get_block_size "${lowerdir}") / 1024 ))
> +iosize=$min_iosize
> max_iosize=2048
> -file_size=10240
> -max_pos=`expr $file_size - $max_iosize`
>
> while [ $iosize -le $max_iosize ]; do
> + file_size=$((10*$iosize))
> + max_pos=`expr $file_size - $iosize`
> + date >>$seqres.full
That's the purpose for putting data info here?
> + echo "Creating ${testfile}_iosize${iosize}K_holefile..." >>$seqres.full
> pos=$iosize
> $XFS_IO_PROG -fc "truncate ${file_size}K" \
> "${lowerdir}/${testfile}_iosize${iosize}K_holefile"
> >>$seqres.full
> -----------
>
>
> >
> > >
> > > Tests that are not in quick group are far less likely to be run
> > > regularly by developers.
> >
> > hmm...well, lets add 'quick' group again and remove it if anyone complains later.
> >
>
> I am now complaining ;-), but after fixes above, test is really quick
>
> Please send a fix patch (to already merged test) to fir test runtime
> and possibly use _get_block_size.
>
> Thanks,
> Amir.
>
