xfs_spaceman: Accounting for AGFL blocks
PATCH 2/2 xfs_spaceman: Accounting for AGFL blocks
Here is the original discussion that Dave wrote for xfs_spaceman:
http://oss.sgi.com/archives/xfs/2012-10/msg00363.html
These patches were not posted and were just forward ported to a current 3.18-rc7+ for-next XFS tree and were pushed to the fiemapfs branch in Dave's kernel tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git
The original xfs_spaceman tool that Dave wrote is here:
http://oss.sgi.com/archives/xfs/2012-10/msg00366.html
Dave just updated it to the 3.2.2 code base and pushed it to the spaceman branch in this tree:
git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
This xfs_spaceman utility previously failed to account for AGFL blocks. Old output (Before changes).
$ sudo xfs_spaceman -c "freesp" /media/xfs from to extents blocks pct 1024 2047 1 1262 0.04 4096 8191 1 5126 0.15 8192 16383 3 35344 1.05 32768 65535 1 43989 1.31 262144 524287 3 1334894 39.78 524288 967428 2 1934840 57.66
As you can see the AGFL blocks were unaccounted (4 per AG, and there were 4 AGs in this filesystem).
This patch is concerned with adding a new function which will walk the free extents in AGFL and account for these AGFL blocks, while file system scanning.
New output (After implementing this patch).
$ uname -a
Linux dhruv-MacBookAir 3.18.0-rc7+ #3 SMP Thu Dec 25 15:29:59 IST 2014 x86_64 x86_64 x86_64 GNU/Linux
$ sudo xfs_spaceman -V
xfs_spaceman version 3.2.2
$ sudo xfs_spaceman -c "freesp" /media/xfs
from to extents blocks pct
1 1 16 16 0.00
1024 2047 1 1262 0.04
4096 8191 1 5126 0.15
8192 16383 3 35344 1.05
32768 65535 1 43989 1.31
262144 524287 3 1334894 39.78
524288 967428 2 1934840 57.66Please do comment.
Signed-off-by:
Dhruvesh Rathore <[email protected]>
Amey Ruikar <[email protected]>
Somdeep Dey <[email protected]>
---linux-xfs/fs/xfs/libxfs/xfs_alloc.c | 81 +++
1 file changed, 81 insertions(+)
-------------------------------------------------------------------------------------------
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +81 -0
@@ -2697,6 +2697,83 @@ } + +/* +* Walk the free extents in the AGFL (AG Free List) of each AG, and dump +* them all into the fieinfo. +* +* With a freshly made filesystem, 4 blocks are reserved immediately after +* the free space B+tree root blocks (blocks 4 to 7). As they are used up +* additional blocks are reserved from the AG and added to the free list +* array. A typical device will have space for 128 elements in the array. +* The actual size can be determined using XFS_AGFL_SIZE macro. The array +* is maintained as a circular list and active elements are pointed by +* AGF's agf_flfirst, agf_fllast and agf_flcount values. +*/ +static int +xfs_alloc_agfl_freespace_map ( +struct xfs_buf **agbp,/* buffer for a.g. freelist header */ +struct xfs_btree_cur *cur, +struct fiemap_extent_info *fieinfo, +xfs_agblock_t sagbno, +xfs_agblock_t eagbno) +{ +xfs_agf_t *agf;/* a.g. freespace structure */ +xfs_buf_t *agflbp;/* buffer for a.g. freelist structure */ +__be32 *agfl_bno;/* Reference to freelist block */ +int j; +int index; +int error =0 ; + + agf =XFS_BUF_TO_AGF (*agbp); + error =xfs_alloc_read_agfl (cur->bc_mp,NULL ,be32_to_cpu (agf->agf_seqno), + &agflbp); +if (error) +return error; + + agfl_bno =XFS_BUF_TO_AGFL_BNO (cur->bc_mp, agflbp); + + index =be32_to_cpu (agf->agf_flfirst); + +for (j=1 ; j<=be32_to_cpu (agf->agf_flcount); j++) + { +xfs_agblock_t fbno; +xfs_extlen_t flen; +xfs_daddr_t dbno; +xfs_fileoff_t dlen; +int flags =0 ; + +/* Relative AG block number */ + fbno =be32_to_cpu (agfl_bno[index]); +/* Each entry in the AGFL is a single entry i.e length is 1 block */ + flen =1 ; + +/* +* AGFL is maintained as a circular list. Following is needed +* to handle array wrapping correctly. +*/ +if ( index == ( (XFS_AGFL_SIZE (cur->bc_mp))-1 ) ) + index =0 ;/* First index of AGFL */ +else + index++;/* Next index in AGFL */ + +/* +* Use daddr format for all range/len calculations as that is +* the format the range/len variables are supplied in by +* userspace. +*/ + dbno =XFS_AGB_TO_DADDR (cur->bc_mp, cur->bc_private.a.agno, fbno); + dlen =XFS_FSB_TO_BB (cur->bc_mp, flen); + error = -fiemap_fill_next_extent (fieinfo,BBTOB (dbno), +BBTOB (dbno),BBTOB (dlen), flags); +if (error) +break ; + } +xfs_buf_relse (agflbp);/* Release the buffer */ +return error; + +} +/* * Map the freespace from the requested range in the requested order. *
@@ -2802,6 +2879,10 @@ }XFS_WANT_CORRUPTED_GOTO (i ==1 , del_cursor); +/* Account for the free blocks in AGFL */ + error =xfs_alloc_agfl_freespace_map (&agbp, cur, fieinfo, sagno, + agno == eagno ? eagbno :NULLAGBLOCK ); +if (!bycnt) {/* * if we are doing a bno ordered lookup, we can just
------------------------------------------------------------------------------------------------
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 10 lines · Dhruvesh Rathore
Here is the original discussion that Dave wrote for
xfs_spaceman:http://oss.sgi.com/archives/xfs/2012-10/msg00363.html
These patches were not posted and were just forward ported to a current 3.18-rc7+ for-next XFS tree and were pushed to the fiemapfs branch in Dave's kernel tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git
Quoted 8 lines
The original
xfs_spacemantool that Dave wrote is here:http://oss.sgi.com/archives/xfs/2012-10/msg00366.html
Dave just updated it to the 3.2.2 code base and pushed it to the spaceman branch in this tree:
git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
As per last patch, this is series description so it doesn't need to be repeated in every patch ;)
Quoted 22 lines
From here:
This
xfs_spacemanutility previously failed to account for AGFL blocks. Old output (Before changes).$ sudo xfs_spaceman -c "freesp" /media/xfs from to extents blocks pct 1024 2047 1 1262 0.04 4096 8191 1 5126 0.15 8192 16383 3 35344 1.05 32768 65535 1 43989 1.31 262144 524287 3 1334894 39.78 524288 967428 2 1934840 57.66As you can see the AGFL blocks were unaccounted (4 per AG, and there were 4 AGs in this filesystem).
This patch is concerned with adding a new function which will walk the free extents in AGFL and account for these AGFL blocks, while file system scanning.
New output (After implementing this patch).
Quoted 13 lines
$ uname -a
Linux dhruv-MacBookAir 3.18.0-rc7+ #3 SMP Thu Dec 25 15:29:59 IST 2014 x86_64 x86_64 x86_64 GNU/Linux
$ sudo xfs_spaceman -V
xfs_spaceman version 3.2.2
$ sudo xfs_spaceman -c "freesp" /media/xfs
from to extents blocks pct
1 1 16 16 0.00
1024 2047 1 1262 0.04
4096 8191 1 5126 0.15
8192 16383 3 35344 1.05
32768 65535 1 43989 1.31
262144 524287 3 1334894 39.78
524288 967428 2 1934840 57.66to here, this is a good patch description. :)
Quoted 1 line
Please do comment.
No need to ask in each patch description - the series description if the place for that....
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2702
Quoted 3 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +3
-0+ +/* +* Walk the free extents in the AGFL (AG Free List) of each AG, and dump
No need to explain what AGFL means - anyone reading the code already knows....
Quoted 4 lines
+ * them all into the fieinfo.
+ *
+ * With a freshly made filesystem, 4 blocks are reserved immediately after
+ * the free space B+tree root blocks (blocks 4 to 7). As they are used up
The number of blocks is actually dependent on filesystem geometry, but again there isn't a need to describe the exact workings of the AGFL here.
Quoted 5 lines
+ * additional blocks are reserved from the AG and added to the free list
+ * array. A typical device will have space for 128 elements in the array.
+ * The actual size can be determined usingXFS_AGFL_SIZEmacro. The array
+ * is maintained as a circular list and active elements are pointed by
+ * AGF'sagf_flfirst,agf_fllastandagf_flcountvalues.
Yup, you're describing the code, not /why/ the function exists.
/*
* When we map free space we need to take into account the blocks
* that are indexed by the AGFL. There aren't found by walking the
* free space btrees, so we have to walk each AGFL to find them. */
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2719
Quoted 8 lines
+ */
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +7
-0+static int +xfs_alloc_agfl_freespace_map ( +struct xfs_buf **agbp,/* buffer for a.g. freelist header */ +struct xfs_btree_cur *cur, +struct fiemap_extent_info *fieinfo, +xfs_agblock_t sagbno, +xfs_agblock_t eagbno)
No need to pass the freespace btree cursor into this function, nor the agf buffer. What is required is the struct xfs_mount, the current agno and the agf structure. i.e.
xfs_alloc_agfl_freespace_map(
struct xfs_mount *mp,
struct xfs_agf *agf,
struct fiemap_extent_info *fieinfo,
xfs_agnumber_t agno,
xfs_agblock_t sagbno,
xfs_agblock_t eagbno)
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2724
Quoted 5 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +5
-0+{ +xfs_agf_t *agf;/* a.g. freespace structure */ +xfs_buf_t *agflbp;/* buffer for a.g. freelist structure */ +__be32 *agfl_bno;/* Reference to freelist block */ +int j;
^
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2725
Quoted 1 line
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +1
-0+int index;
^
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2726
Quoted 1 line
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +1
-0+int error =0 ;
^
Stray whitespace.
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2730
Quoted 4 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +4
-0+ + agf =XFS_BUF_TO_AGF (*agbp); + error =xfs_alloc_read_agfl (cur->bc_mp,NULL ,be32_to_cpu (agf->agf_seqno), + &agflbp);
error = xfs_alloc_read_agfl(mp, NULL, agno, &agflbp);
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2739
Quoted 9 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +9
-0+if (error) +return error; + + agfl_bno =XFS_BUF_TO_AGFL_BNO (cur->bc_mp, agflbp); + + index =be32_to_cpu (agf->agf_flfirst); + +for (j=1 ; j<=be32_to_cpu (agf->agf_flcount); j++) + {
formatting, and no need for j and index parameters.
for (i = be32_to_cpu(agf->agf_flfirst);
i < be32_to_cpu(agf->agf_fllast);) {
....
if (++i == XFS_AGFL_SIZE(mp))
i = 0;
}linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2744
Quoted 5 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +5
-0+xfs_agblock_t fbno; +xfs_extlen_t flen; +xfs_daddr_t dbno; +xfs_fileoff_t dlen; +int flags =0 ;
all the indenting in the function needs fixing.
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2749
Quoted 5 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +5
-0+ +/* Relative AG block number */ + fbno =be32_to_cpu (agfl_bno[index]); +/* Each entry in the AGFL is a single entry i.e length is 1 block */ + flen =1 ;
These can all be folded into the code below - no need for intermediate variables...
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2758
Quoted 9 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +9
-0+ +/* +* AGFL is maintained as a circular list. Following is needed +* to handle array wrapping correctly. +*/ +if ( index == ( (XFS_AGFL_SIZE (cur->bc_mp))-1 ) ) + index =0 ;/* First index of AGFL */ +else + index++;/* Next index in AGFL */
This is handled by the above loop construct so can be ignored.
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2768
Quoted 10 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +10
-0+ +/* +* Use daddr format for all range/len calculations as that is +* the format the range/len variables are supplied in by +* userspace. +*/ + dbno =XFS_AGB_TO_DADDR (cur->bc_mp, cur->bc_private.a.agno, fbno); + dlen =XFS_FSB_TO_BB (cur->bc_mp, flen); + error = -fiemap_fill_next_extent (fieinfo,BBTOB (dbno), +BBTOB (dbno),BBTOB (dlen), flags);
This takes bytes as it's values, and we have AG block numbers and filesystem block lengths. so:
and the entire loop becomes:
for (i = be32_to_cpu(agf->agf_flfirst);
i < be32_to_cpu(agf->agf_fllast);) {
/*
* Use daddr format for all range/len calculations as that is
* the format the range/len variables are supplied in by
* userspace.
*/
dbno = XFS_AGB_TO_DADDR(mp, agno, be32_to_cpu(agfl_bno[i]));
error = fiemap_fill_next_extent(fieinfo,
BBTOB(dbno), BBTOB(dbno),
XFS_FSB_TO_B(mp, 1), flags);
if (error)
break;
if (++i == XFS_AGFL_SIZE(mp))
i = 0;
}
xfs_buf_relse(agflbp);
return error;
}Hmmm - I think something is still missing - what are the sagbno and eagbno parameters supposed to do?
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Dave Chinner
Quoted 6 lines · Dhruvesh Rathore
Here is the original discussion that Dave wrote for
xfs_spaceman:http://oss.sgi.com/archives/xfs/2012-10/msg00363.html
These patches were not posted and were just forward ported to a current 3.18-rc7+ for-next XFS tree and were pushed to the fiemapfs branch in
Dave's
Quoted 8 lines
Quoted 8 lines
kernel tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.gitThe original
xfs_spacemantool that Dave wrote is here:
Quoted 8 lines
Quoted 4 lines
Dave just updated it to the 3.2.2 code base and pushed it to the spaceman branch in this tree:
git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.gitAs per last patch, this is series description so it doesn't need to be repeated in every patch ;)
We will keep that in mind, while sending the updated patches. :)
Quoted 18 lines
From here:
Quoted 16 lines
This
xfs_spacemanutility previously failed to account for AGFL blocks. Old output (Before changes).$ sudo xfs_spaceman -c "freesp" /media/xfs from to extents blocks pct 1024 2047 1 1262 0.04 4096 8191 1 5126 0.15 8192 16383 3 35344 1.05 32768 65535 1 43989 1.31 262144 524287 3 1334894 39.78 524288 967428 2 1934840 57.66As you can see the AGFL blocks were unaccounted (4 per AG, and there were 4 AGs in this filesystem).
This patch is concerned with adding a new function which will walk the
free
Quoted 1 line
Quoted 1 line
extents in AGFL and account for these AGFL blocks, while file system
scanning.
Quoted 5 lines
Quoted 5 lines
New output (After implementing this patch).
$ uname -a Linux dhruv-MacBookAir 3.18.0-rc7+ #3 SMP Thu Dec 25 15:29:59 IST 2014
x86_64 x86_64 x86_64 GNU/Linux
Quoted 12 lines
Quoted 11 lines
$ sudo xfs_spaceman -V xfs_spaceman version 3.2.2 $ sudo xfs_spaceman -c "freesp" /media/xfs from to extents blocks pct 1 1 16 16 0.00 1024 2047 1 1262 0.04 4096 8191 1 5126 0.15 8192 16383 3 35344 1.05 32768 65535 1 43989 1.31 262144 524287 3 1334894 39.78 524288 967428 2 1934840 57.66
Quoted 7 lines
to here, this is a good patch description. :)
Quoted 1 line
Please do comment.
No need to ask in each patch description - the series description if the place for that....
We will make a not of this too.
Quoted 10 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2702
Quoted 3 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +3
-0+ +/* +* Walk the free extents in the AGFL (AG Free List) of each AG, and dump No need to explain what AGFL means - anyone reading the code already knows....
Quoted 3 lines
+ * them all into the fieinfo.
+ *
+ * With a freshly made filesystem, 4 blocks are reserved immediately
after
Quoted 1 line
Quoted 1 line
+ * the free space B+tree root blocks (blocks 4 to 7). As they are used
up
Quoted 7 lines
The number of blocks is actually dependent on filesystem geometry, but again there isn't a need to describe the exact workings of the AGFL here.
Quoted 2 lines
+ * additional blocks are reserved from the AG and added to the free list
+ * array. A typical device will have space for 128 elements in the
array.
Quoted 1 line
Quoted 1 line
+ * The actual size can be determined using
XFS_AGFL_SIZEmacro. The
array
Quoted 4 lines
Quoted 2 lines
+ * is maintained as a circular list and active elements are pointed by
+ * AGF'sagf_flfirst,agf_fllastandagf_flcountvalues.Yup, you're describing the code, not /why/ the function exists.
Our bad, we will ensure that the comment, explains why the function is needed.
Quoted 11 lines
/*
* When we map free space we need to take into account the blocks
* that are indexed by the AGFL. There aren't found by walking the
* free space btrees, so we have to walk each AGFL to find them. */linux-xfs/fs/xfs/libxfs/xfs_alloc.c
Quoted 4 lines
+ */
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +3
-0+static int +xfs_alloc_agfl_freespace_map( + struct xfs_buf **agbp, /* buffer for a.g. freelist header
*/
Quoted 5 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2719
Quoted 4 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +4
-0+struct xfs_btree_cur *cur, + struct fiemap_extent_info *fieinfo, + xfs_agblock_t sagbno, + xfs_agblock_t eagbno)
Quoted 14 lines
No need to pass the freespace btree cursor into this function, nor the agf buffer. What is required is the struct
xfs_mount, the current agno and the agf structure. i.e. xfs_alloc_agfl_freespace_map(
structxfs_mount*mp,
structxfs_agf*agf,
structfiemap_extent_info*fieinfo,
xfs_agnumber_tagno,
xfs_agblock_tsagbno,
xfs_agblock_teagbno)Quoted 3 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +3
-0+{ +xfs_agf_t *agf;/* a.g. freespace structure */ +xfs_buf_t *agflbp;/* buffer for a.g. freelist
structure */
Quoted 13 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2724
Quoted 2 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +2
-0+__be32 *agfl_bno;/* Reference to freelist block */ +int j;^Quoted 1 line
+ int index;
^Quoted 1 line
+ int error = 0;
^Stray whitespace.
Quoted 3 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +3
-0+ + agf =XFS_BUF_TO_AGF (*agbp); + error = xfs_alloc_read_agfl(cur->bc_mp, NULL,
be32_to_cpu(agf->agf_seqno),
Quoted 23 lines
Quoted 1 line
+ &agflbp);
error = xfs_alloc_read_agfl(mp, NULL, agno, &agflbp);linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2739
Quoted 9 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +9
-0+if (error) +return error; + + agfl_bno =XFS_BUF_TO_AGFL_BNO (cur->bc_mp, agflbp); + + index =be32_to_cpu (agf->agf_flfirst); + +for (j=1 ; j<=be32_to_cpu (agf->agf_flcount); j++) + {formatting, and no need for j and index parameters.
for (i = be32_to_cpu(agf->agf_flfirst); i < be32_to_cpu(agf->agf_fllast);) { .... if (++i == XFS_AGFL_SIZE(mp)) i = 0; }
Quoted 12 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2744
Quoted 5 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +5
-0+xfs_agblock_t fbno; +xfs_extlen_t flen; +xfs_daddr_t dbno; +xfs_fileoff_t dlen; +int flags =0 ;all the indenting in the function needs fixing.
Quoted 4 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +4
-0+ +/* Relative AG block number */ + fbno =be32_to_cpu (agfl_bno[index]); +/* Each entry in the AGFL is a single entry i.e length is 1 block
*/
Quoted 31 lines
Quoted 1 line
+ flen = 1;
These can all be folded into the code below - no need for intermediate variables...
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2758
Quoted 9 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +9
-0+ +/* +* AGFL is maintained as a circular list. Following is needed +* to handle array wrapping correctly. +*/ +if ( index == ( (XFS_AGFL_SIZE (cur->bc_mp))-1 ) ) + index =0 ;/* First index of AGFL */ +else + index++;/* Next index in AGFL */ This is handled by the above loop construct so can be ignored.
linux-xfs/fs/xfs/libxfs/xfs_alloc.c line 2768
Quoted 10 lines
linux-xfs/fs/xfs/libxfs/xfs_alloc.c +10
-0+ +/* +* Use daddr format for all range/len calculations as that is +* the format the range/len variables are supplied in by +* userspace. +*/ + dbno =XFS_AGB_TO_DADDR (cur->bc_mp, cur->bc_private.a.agno, fbno); + dlen =XFS_FSB_TO_BB (cur->bc_mp, flen); + error = -fiemap_fill_next_extent (fieinfo,BBTOB (dbno), +BBTOB (dbno),BBTOB (dlen), flags);This takes bytes as it's values, and we have AG block numbers and filesystem block lengths. so:
Quoted 6 lines
and the entire loop becomes:
for (i = be32_to_cpu(agf->agf_flfirst); i < be32_to_cpu(agf->agf_fllast);) { /* * Use daddr format for all range/len calculations as
that is
Quoted 4 lines
* the format the range/len variables are supplied in by
* userspace.
*/
dbno = XFS_AGB_TO_DADDR(mp, agno,
be32_to_cpu(agfl_bno[i]));
Quoted 3 lines
error = fiemap_fill_next_extent(fieinfo,
BBTOB(dbno), BBTOB(dbno),XFS_FSB_TO_B(mp, 1),
flags);
Quoted 8 lines
if (error)
break;
if (++i == XFS_AGFL_SIZE(mp))
i = 0;
}
xfs_buf_relse(agflbp);
return error;
}We will incorporate the above mentioned changes, and send the updated patch at the earliest.
Quoted 2 lines
Hmmm - I think something is still missing - what are the sagbno and eagbno parameters supposed to do?
Actually the parameters sagbno and eagbno are not needed in this function and can be excluded.
Quoted 6 lines
Cheers,
Dave.
--
Dave Chinner
[email protected]
Thank you for the detailed feedback. We will be sending the updated patches at the earliest.
Regards,
A-DRS
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 6 lines · ADRS PICT
Quoted 2 lines · Dave Chinner
Hmmm - I think something is still missing - what are the sagbno and eagbno parameters supposed to do?
Actually the parameters sagbno and eagbno are not needed in this function and can be excluded.
Why? Don't we still have to check the blocks found on the AGFL fll within the range requested by the user, like we do for every extent found in the btree?
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
A message that is not in the archive (sent off-list or not yet received): CALJmpHOFVZer7YL8Kek6jk2U8f7JyBrJknYzVhyowht_EGm7DQ@mail.gmail.com
PATCH 2/2 xfs_spaceman: Accounting for AGFL blocks
Quoted 11 lines · Dave Chinner
Quoted 6 lines · ADRS PICT
Quoted 2 lines · Dave Chinner
Hmmm - I think something is still missing - what are the sagbno and eagbno parameters supposed to do?
Actually the parameters sagbno and eagbno are not needed in this function and can be excluded.
Why? Don't we still have to check the blocks found on the AGFL fll within the range requested by the user, like we do for every extent found in the btree?
We had felt that the range check is not needed as we are fetching block numbers from the allocation group free block array by the function xfs_alloc_read_agfl().
And in xfs_alloc_read_agfl(), the error checking is done implicitly.
However, after you have raised this point, it is clear to us that performing a range check will be a good way to catch and display a warning if the blocks are out of range.
Regards,
A-DRS
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 13 lines · Dhruvesh Rathore
Quoted 11 lines · Dave Chinner
Quoted 6 lines · ADRS PICT
Quoted 2 lines · Dave Chinner
Hmmm - I think something is still missing - what are the sagbno and eagbno parameters supposed to do?
Actually the parameters sagbno and eagbno are not needed in this function and can be excluded.
Why? Don't we still have to check the blocks found on the AGFL fll within the range requested by the user, like we do for every extent found in the btree?
Quoted 9 lines
We had felt that the range check is not needed as we are fetching block numbers from the allocation group free block array by the function
xfs_alloc_read_agfl().
And inxfs_alloc_read_agfl(), the error checking is done implicitly.However, after you have raised this point, it is clear to us that performing a range check will be a good way to catch and display a warning if the blocks are out of range.
We have assimilated the changes you pointed out and have sent the updated patches
in a new thread.
Regards,
A-DRS
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
A message that is not in the archive (sent off-list or not yet received): 20150130013456.GA4251@dastard
Quoted 1 line · Dave Chinner
[ please reply to the list! ]
We apologize it was an honest mistake.
We had hit reply instead of typing in the 'to field'.
Quoted 24 lines · Dhruvesh Rathore
Quoted 13 lines · Dave Chinner
Quoted 11 lines · ADRS PICT
Quoted 6 lines · Dave Chinner
Quoted 2 lines
Hmmm - I think something is still missing - what are the sagbno and eagbno parameters supposed to do?
Actually the parameters sagbno and eagbno are not needed in this function and can be excluded.
Why? Don't we still have to check the blocks found on the AGFL fll within the range requested by the user, like we do for every extent found in the btree?
Quoted 8 lines
We had felt that the range check is not needed as we are fetching block numbers from the allocation group free block array by the function
xfs_alloc_read_agfl().
And inxfs_alloc_read_agfl(), the error checking is done implicitly.However, after you have raised this point, it is clear to us that performing a range check will be a good way to catch and display a warning if the blocks are out of range.
Quoted 7 lines
The range checking is not for validity of the blocks themselves - the ranges passed in come from the user. i.e. the user is asking for free space blocks within a certain range on disk. The blocks in the AGFL may or may not lie within that the user specified range, and so need to be checked against the range (similar to btree extents) to determine if they shoul dbe added to the FIEMAPFS output...
We have incorporated the above changes, and sent the updated patches on the mailing list. Here are the links:
http://oss.sgi.com/archives/xfs/2015-01/msg00484.html
http://oss.sgi.com/archives/xfs/2015-01/msg00486.html
http://oss.sgi.com/archives/xfs/2015-01/msg00487.html
http://oss.sgi.com/archives/xfs/2015-01/msg00488.html
http://oss.sgi.com/archives/xfs/2015-01/msg00489.html
Regards,
A-DRS
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Hi Dave,
Quoted 5 lines · Dhruvesh Rathore
http://oss.sgi.com/archives/xfs/2015-01/msg00484.html
http://oss.sgi.com/archives/xfs/2015-01/msg00486.html
http://oss.sgi.com/archives/xfs/2015-01/msg00487.html
http://oss.sgi.com/archives/xfs/2015-01/msg00488.html
http://oss.sgi.com/archives/xfs/2015-01/msg00489.html
We were wondering if you had a chance to review the above sent patches. :)
Regards,
A-DRS
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs