Re: [ogfs-dev]RFC: Reclaim the excess freemeta blocks

[Dominik Vogt <opengfs-devel@lists.sourceforge.net>]

Okay, I finally got around looking at the patch.  See my comments
to the ogfs_reclaim_freemeta function below.


> int ogfs_reclaim_freemeta(ogfs_sbd_t *sdp, void *arg)
> {

>   int error, rnum, excess, s_excess;

Is an int "excess" guaranteed to be large enough?

>   unsigned int buf;
>   uint32 length,block;
>         ogfs_bitmap_t *bits = NULL;
>   ogfs_rgrpd_t *rgd;
>   
>   error = ogfs_rindex_hold(sdp);
>   if (error) {
>     return error;
>   }
> 
>   for ( rnum = 0 ; rnum < sdp->sd_rgcount; rnum++) {
>   
>     rgd = ogfs_rgrpd_get_by_num(sdp, rnum);
>     if (rgd == NULL)
>       continue;
>   
>     error = ogfs_glock_rg(rgd, 0);
>     if (error) {
>       ogfs_rgrpd_put(rgd);
>       continue;
>     }
>     

>     if (test_and_clear_bit(RDF_SYNC_B4_ALLOC, &rgd->rd_flags))
>       ogfs_log_flush(sdp, rgd->rd_gl);

What does this do?

> 
>     length = rgd->rd_ri.ri_length;

>     excess = rgd->rd_rg.rg_freemeta - OGFS_META_CLUMP;
>     if (excess > 0) {

To be safe with integer lengths, better:

      unsigned int excess; /* at top of function */
      if (rgd->rd_rg.rg_freemeta > OGFS_META_CLUMP) {
        excess = rgd->rd_rg.rg_freemeta - OGFS_META_CLUMP;

But wouldn't it be better to purge all free meta blocks, or at
least prefer to release the meta blocks that are not flanked by
other meta blocks to reduce fragmentation?

>       error = ogfs_trans_begin(sdp, TRANS_BMAP, length + 1, 0);
>       if (error) {
>         ogfs_gunlock_rg(rgd, 0);
>         ogfs_rgrpd_put(rgd);
>         ogfs_rindex_release(sdp);
>         return error;
>       }
>     } else {
>       ogfs_gunlock_rg(rgd, 0);
>       ogfs_rgrpd_put(rgd);
>       continue;
>     }
>     
>     s_excess = excess;
>     
>     for (buf = 0; buf < length; buf++) {
>       bits = &rgd->rd_bits[buf];
>       while (excess) {

>         block = ogfs_bitfit(BH_DATA(rgd->rd_bh[buf]) + bits->bi_offset,
>               bits->bi_len, 0, OGFS_BLKST_FREEMETA);

Hm, does this search the bitmap from the start?  If so, this loop
requires O(excess^2 * number_of_rgs) passes through the loop.
This might be too slow on large file systems with lots of free
meta blocks.

>         if (block != BFITNOENT) {
>           excess--;
>           ogfs_setbit(BH_DATA(rgd->rd_bh[buf]) + bits->bi_offset,
>               bits->bi_len, block, OGFS_BLKST_FREE);
>         } else {
>           break;
>         }
>       }
>       ogfs_trans_add_bh(sdp, rgd->rd_bh[buf]);
>     }
>     rgd->rd_rg.rg_freemeta -= (s_excess - excess);
>     rgd->rd_rg.rg_free += (s_excess - excess);
>     ogfs_trans_add_bh(sdp, rgd->rd_bh[0]);
>     ogfs_rgrp_out(&rgd->rd_rg, BH_DATA(rgd->rd_bh[0]));

>     rgrp_lvb_own(rgd);

What is the purpose of this call?

>     set_bit(RDF_SYNC_B4_ALLOC, &rgd->rd_flags);
>     ogfs_trans_end(sdp);
>     ogfs_gunlock_rg(rgd, 0);
>     ogfs_rgrpd_put(rgd);
>   }    
> 
>   ogfs_rindex_release(sdp);
>   return error;
> 
> }

Bye

Dominik ^_^  ^_^


-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100006ave/direct;at.asp_061203_01/01
_______________________________________________
Opengfs-devel mailing list
Opengfs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opengfs-devel