Re: [PATCH v3] overlay/066: copy-up test for variant sparse files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 ---- 在 星期一, 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.
 >





[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux