---- 在 星期一, 2019-10-28 20:29:03 Amir Goldstein <amir73il@xxxxxxxxx> 撰写 ----
> On Mon, Oct 28, 2019 at 2:09 PM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote:
> >
> > ---- 在 星期日, 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?
> >
>
> Not sure. I don't think we should target the test by what we know your
> patch does, but by maximizing test coverage in a cost effective way.
>
> Creating a 10M file with so many small holes doesn't add much to test
> coverage IMO. If you feel those are needed, you should use a C helper
> to create those files more efficiently.
>
> BTW I think what is missing from test coverage is small holes
> that are not aligned to 1M boundary.
>
Agreed.
So how about change test pattern to below, it will cover most of the
cases that we want. I haven't done test for the performance(test time)
but I think it will be fast enough.
One 4K empty file.
One 4M empty file.
One 10M file with random small holes (4K~512K)
One 100M file with random big holes (1M~5M)
>
> >
> > >
> > > 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 ?
>
> it's the formula to the sum of the series:
> 2^0+2^2+...2^11 = (2^0 + 2^11)*12/2
oh, sorry, I misread the formula.
>
> >
> > > 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?
>
> leftover from my debugging patch to figure out what takes so much time
> You don't need it
>
> Thanks,
> Amir.
>
