Hi Eric
Eric Sandeen [sandeen@xxxxxxx] wrote:
> Ok, this isn't quite right. For a file with holes, the "best-case"
> scenario is > 1 extent. I think your patch will bail out
> if any particular extent on one side of a hole does not defrag,
> rather than looking at all extents in a hole-y file.
No. I have tested that case. cur_nextents and getnextents() are always for
the whole file and not for a group of non hole extents that will preallocated
at once.
Sometimes there will be unnessary coping of extents when the file is hole-y.
But the final result is the same.
>
> I think the right way to do it is to do 2 loops - first loop, go
> through allocating extents (accounting for holes if necessary).
> THEN check how many extents the temp file has, and proceed with
> copying in a 2nd loop if things look better.
I thought about that too. It's the better way. But this require to spilt the
loop. I dont know if it's worth.
utz
>
> I have a patch that does this, I'll probably get it checked in tomorrow
> if it looks good to a reviewer or two.
>
> Thanks,
> -Eric
>
> On Sun, 2 Mar 2003, utz lehmann wrote:
>
> > Hi
> >
> > This patch moves the check for improved extent count directly after the
> > preallocation of the extents. So it can avoid copying of the extent data in
> > the case that no improvement is made. The patch is against current CVS.
> >
> > On my testsystem with 3 files which cant improved the elapsed time went down
> > from 3min to 6s and saved about 3GB disk io.
> >
> >
> > utz
> >
> >
> > --- xfs_fsr.c.orig Sat Mar 1 13:53:36 2003
> > +++ xfs_fsr.c Sun Mar 2 23:02:25 2003
> > @@ -967,7 +967,8 @@
> > {
> > int tfd;
> > int srval;
> > - int nextents, extent, cur_nextents, new_nextents;
> > + int nextents, extent, cur_nextents;
> > + int new_nextents = 0;
> > unsigned blksz_dio;
> > unsigned dio_min;
> > struct dioattr dio;
> > @@ -1111,6 +1112,15 @@
> > return -1;
> > }
> > }
> > + /* Check if the extent count improved */
> > + new_nextents = getnextents(tfd);
> > + if (cur_nextents <= new_nextents) {
> > + if (vflag)
> > + fsrprintf(_("No improvement made: %s\n"),
> > fname);
> > + close(tfd);
> > + free(fbuf);
> > + return 1; /* no change/no error */
> > + }
> > for (cnt = outmap[extent].bmv_length; cnt > 0;
> > cnt -= ct, pos += ct) {
> > if (nfrags && --nfrags) {
> > @@ -1195,15 +1205,6 @@
> > sx.sx_offset = 0;
> > sx.sx_length = statp->bs_size;
> >
> > - /* Check if the extent count improved */
> > - new_nextents = getnextents(tfd);
> > - if (cur_nextents <= new_nextents) {
> > - if (vflag)
> > - fsrprintf(_("No improvement made: %s\n"), fname);
> > - close(tfd);
> > - return 1; /* no change/no error */
> > - }
> > -
> > /* switch to the owner's id, to keep quota in line */
> > if (fchown(tfd, statp->bs_uid, statp->bs_gid) < 0) {
> > if (vflag)
> >
> >
> >
> >
>
|