* [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c
@ 2012-03-27 20:40 Eric Sandeen
2012-03-27 20:50 ` Eric Sandeen
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Eric Sandeen @ 2012-03-27 20:40 UTC (permalink / raw)
To: xfs-oss; +Cc: Gabriel VLASIU
xfs_swap_extents_check_format() contains checks to make sure that
original and the temporary files during defrag are compatible;
Gabriel VLASIU ran into a case where xfs_fsr returned EINVAL
because the tests found the btree root to be of size 120,
while the fork offset was only 104; IOW, they overlapped.
However, this is just due to an error in the
xfs_swap_extents_check_format() tests, because it is checking
the in-memory btree root size against the on-disk fork offset.
We should be checking the on-disk sizes in both cases.
This patch adds a new macro to calculate this size, and uses
it in the tests.
With this change, the filesystem image provided by Gabriel
allows for proper file degragmentation.
Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/fs/xfs/xfs_bmap_btree.h b/fs/xfs/xfs_bmap_btree.h
index 0e66c4e..d9eeb64 100644
--- a/fs/xfs/xfs_bmap_btree.h
+++ b/fs/xfs/xfs_bmap_btree.h
@@ -189,12 +189,14 @@ typedef __be64 xfs_bmbt_ptr_t, xfs_bmdr_ptr_t;
#define XFS_BMAP_BROOT_SPACE_CALC(nrecs) \
(int)(XFS_BTREE_LBLOCK_LEN + \
((nrecs) * (sizeof(xfs_bmbt_key_t) + sizeof(xfs_bmbt_ptr_t))))
-
#define XFS_BMAP_BROOT_SPACE(bb) \
(XFS_BMAP_BROOT_SPACE_CALC(be16_to_cpu((bb)->bb_numrecs)))
+
#define XFS_BMDR_SPACE_CALC(nrecs) \
(int)(sizeof(xfs_bmdr_block_t) + \
((nrecs) * (sizeof(xfs_bmbt_key_t) + sizeof(xfs_bmbt_ptr_t))))
+#define XFS_BMAP_BMDR_SPACE(bb) \
+ (XFS_BMDR_SPACE_CALC(be16_to_cpu((bb)->bb_numrecs)))
/*
* Maximum number of bmap btree levels.
diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
index dd974a5..2707d86 100644
--- a/fs/xfs/xfs_dfrag.c
+++ b/fs/xfs/xfs_dfrag.c
@@ -26,6 +26,9 @@
#include "xfs_ag.h"
#include "xfs_mount.h"
#include "xfs_bmap_btree.h"
+#include "xfs_alloc_btree.h"
+#include "xfs_ialloc_btree.h"
+#include "xfs_btree.h"
#include "xfs_dinode.h"
#include "xfs_inode.h"
#include "xfs_inode_item.h"
@@ -184,7 +187,7 @@ xfs_swap_extents_check_format(
*/
if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) {
if (XFS_IFORK_BOFF(ip) &&
- tip->i_df.if_broot_bytes > XFS_IFORK_BOFF(ip))
+ XFS_BMAP_BMDR_SPACE(tip->i_df.if_broot) > XFS_IFORK_BOFF(ip))
return EINVAL;
if (XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK) <=
XFS_IFORK_MAXEXT(ip, XFS_DATA_FORK))
@@ -194,9 +197,8 @@ xfs_swap_extents_check_format(
/* Reciprocal target->temp btree format checks */
if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE) {
if (XFS_IFORK_BOFF(tip) &&
- ip->i_df.if_broot_bytes > XFS_IFORK_BOFF(tip))
+ XFS_BMAP_BMDR_SPACE(ip->i_df.if_broot) > XFS_IFORK_BOFF(tip))
return EINVAL;
-
if (XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) <=
XFS_IFORK_MAXEXT(tip, XFS_DATA_FORK))
return EINVAL;
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c
2012-03-27 20:40 [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
@ 2012-03-27 20:50 ` Eric Sandeen
2012-03-27 21:16 ` Dave Chinner
2012-03-28 17:21 ` [PATCH 2/1] xfs: use XFS_BMAP_BMDR_SPACE vs. XFS_BROOT_SIZE_ADJ Eric Sandeen
2013-06-19 21:51 ` [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
2 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2012-03-27 20:50 UTC (permalink / raw)
To: xfs-oss; +Cc: Gabriel VLASIU
On 3/27/12 3:40 PM, Eric Sandeen wrote:
> xfs_swap_extents_check_format() contains checks to make sure that
> original and the temporary files during defrag are compatible;
> Gabriel VLASIU ran into a case where xfs_fsr returned EINVAL
> because the tests found the btree root to be of size 120,
> while the fork offset was only 104; IOW, they overlapped.
>
> However, this is just due to an error in the
> xfs_swap_extents_check_format() tests, because it is checking
> the in-memory btree root size against the on-disk fork offset.
> We should be checking the on-disk sizes in both cases.
>
> This patch adds a new macro to calculate this size, and uses
> it in the tests.
>
> With this change, the filesystem image provided by Gabriel
> allows for proper file degragmentation.
Hm, as usually happens right after finalizing this I stumbled
on something else. xfs_iroot_realloc() does essentially the same
test, but uses a funky macro to resolve the incore/ondisk size
difference:
ASSERT(ifp->if_broot_bytes <=
XFS_IFORK_SIZE(ip, whichfork) + XFS_BROOT_SIZE_ADJ);
so dfrag.c could be fixed up the same way, I suppose, using
XFS_BROOT_SIZE_ADJ if desired (though I have no real love for that
undocumented macro!)
-Eric
> Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/fs/xfs/xfs_bmap_btree.h b/fs/xfs/xfs_bmap_btree.h
> index 0e66c4e..d9eeb64 100644
> --- a/fs/xfs/xfs_bmap_btree.h
> +++ b/fs/xfs/xfs_bmap_btree.h
> @@ -189,12 +189,14 @@ typedef __be64 xfs_bmbt_ptr_t, xfs_bmdr_ptr_t;
> #define XFS_BMAP_BROOT_SPACE_CALC(nrecs) \
> (int)(XFS_BTREE_LBLOCK_LEN + \
> ((nrecs) * (sizeof(xfs_bmbt_key_t) + sizeof(xfs_bmbt_ptr_t))))
> -
> #define XFS_BMAP_BROOT_SPACE(bb) \
> (XFS_BMAP_BROOT_SPACE_CALC(be16_to_cpu((bb)->bb_numrecs)))
> +
> #define XFS_BMDR_SPACE_CALC(nrecs) \
> (int)(sizeof(xfs_bmdr_block_t) + \
> ((nrecs) * (sizeof(xfs_bmbt_key_t) + sizeof(xfs_bmbt_ptr_t))))
> +#define XFS_BMAP_BMDR_SPACE(bb) \
> + (XFS_BMDR_SPACE_CALC(be16_to_cpu((bb)->bb_numrecs)))
>
> /*
> * Maximum number of bmap btree levels.
> diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
> index dd974a5..2707d86 100644
> --- a/fs/xfs/xfs_dfrag.c
> +++ b/fs/xfs/xfs_dfrag.c
> @@ -26,6 +26,9 @@
> #include "xfs_ag.h"
> #include "xfs_mount.h"
> #include "xfs_bmap_btree.h"
> +#include "xfs_alloc_btree.h"
> +#include "xfs_ialloc_btree.h"
> +#include "xfs_btree.h"
> #include "xfs_dinode.h"
> #include "xfs_inode.h"
> #include "xfs_inode_item.h"
> @@ -184,7 +187,7 @@ xfs_swap_extents_check_format(
> */
> if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) {
> if (XFS_IFORK_BOFF(ip) &&
> - tip->i_df.if_broot_bytes > XFS_IFORK_BOFF(ip))
> + XFS_BMAP_BMDR_SPACE(tip->i_df.if_broot) > XFS_IFORK_BOFF(ip))
> return EINVAL;
> if (XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK) <=
> XFS_IFORK_MAXEXT(ip, XFS_DATA_FORK))
> @@ -194,9 +197,8 @@ xfs_swap_extents_check_format(
> /* Reciprocal target->temp btree format checks */
> if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE) {
> if (XFS_IFORK_BOFF(tip) &&
> - ip->i_df.if_broot_bytes > XFS_IFORK_BOFF(tip))
> + XFS_BMAP_BMDR_SPACE(ip->i_df.if_broot) > XFS_IFORK_BOFF(tip))
> return EINVAL;
> -
> if (XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) <=
> XFS_IFORK_MAXEXT(tip, XFS_DATA_FORK))
> return EINVAL;
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c
2012-03-27 20:50 ` Eric Sandeen
@ 2012-03-27 21:16 ` Dave Chinner
0 siblings, 0 replies; 9+ messages in thread
From: Dave Chinner @ 2012-03-27 21:16 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Gabriel VLASIU, xfs-oss
On Tue, Mar 27, 2012 at 03:50:48PM -0500, Eric Sandeen wrote:
> On 3/27/12 3:40 PM, Eric Sandeen wrote:
> > xfs_swap_extents_check_format() contains checks to make sure that
> > original and the temporary files during defrag are compatible;
> > Gabriel VLASIU ran into a case where xfs_fsr returned EINVAL
> > because the tests found the btree root to be of size 120,
> > while the fork offset was only 104; IOW, they overlapped.
> >
> > However, this is just due to an error in the
> > xfs_swap_extents_check_format() tests, because it is checking
> > the in-memory btree root size against the on-disk fork offset.
> > We should be checking the on-disk sizes in both cases.
> >
> > This patch adds a new macro to calculate this size, and uses
> > it in the tests.
> >
> > With this change, the filesystem image provided by Gabriel
> > allows for proper file degragmentation.
>
> Hm, as usually happens right after finalizing this I stumbled
> on something else. xfs_iroot_realloc() does essentially the same
> test, but uses a funky macro to resolve the incore/ondisk size
> difference:
>
> ASSERT(ifp->if_broot_bytes <=
> XFS_IFORK_SIZE(ip, whichfork) + XFS_BROOT_SIZE_ADJ);
>
> so dfrag.c could be fixed up the same way, I suppose, using
> XFS_BROOT_SIZE_ADJ if desired (though I have no real love for that
> undocumented macro!)
I much prefer the addition of a XFS_BMAP_BMDR_SPACE() macro. Perhaps
it might be worthwhile to convert those uses of XFS_BROOT_SIZE_ADJ
to use your new macro, and get rid of the XFS_BROOT_SIZE_ADJ grot
altogether?
Anyway, consider your patch:
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/1] xfs: use XFS_BMAP_BMDR_SPACE vs. XFS_BROOT_SIZE_ADJ
2012-03-27 20:40 [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
2012-03-27 20:50 ` Eric Sandeen
@ 2012-03-28 17:21 ` Eric Sandeen
2012-03-29 0:55 ` Dave Chinner
2013-06-19 21:51 ` [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
2 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2012-03-28 17:21 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
XFS_BROOT_SIZE_ADJ is an undocumented macro which accounts for
the difference in size between the on-disk and in-core btree
root. It's much clearer to just use the newly-added
XFS_BMAP_BMDR_SPACE macro which gives us the on-disk size
directly.
In one case, we must test that the if_broot exists before
applying the macro, however.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/fs/xfs/xfs_dinode.h b/fs/xfs/xfs_dinode.h
index a372163..be76047 100644
--- a/fs/xfs/xfs_dinode.h
+++ b/fs/xfs/xfs_dinode.h
@@ -107,9 +107,6 @@ typedef enum xfs_dinode_fmt {
#define XFS_LITINO(mp) \
((int)(((mp)->m_sb.sb_inodesize) - sizeof(struct xfs_dinode)))
-#define XFS_BROOT_SIZE_ADJ \
- (XFS_BTREE_LBLOCK_LEN - sizeof(xfs_bmdr_block_t))
-
/*
* Inode data & attribute fork sizes, per inode.
*/
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index b210224..a1a6337 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1833,8 +1833,8 @@ xfs_iroot_realloc(
np = (char *)XFS_BMAP_BROOT_PTR_ADDR(mp, ifp->if_broot, 1,
(int)new_size);
ifp->if_broot_bytes = (int)new_size;
- ASSERT(ifp->if_broot_bytes <=
- XFS_IFORK_SIZE(ip, whichfork) + XFS_BROOT_SIZE_ADJ);
+ ASSERT(XFS_BMAP_BMDR_SPACE(ifp->if_broot) <=
+ XFS_IFORK_SIZE(ip, whichfork));
memmove(np, op, cur_max * (uint)sizeof(xfs_dfsbno_t));
return;
}
@@ -1886,8 +1886,9 @@ xfs_iroot_realloc(
kmem_free(ifp->if_broot);
ifp->if_broot = new_broot;
ifp->if_broot_bytes = (int)new_size;
- ASSERT(ifp->if_broot_bytes <=
- XFS_IFORK_SIZE(ip, whichfork) + XFS_BROOT_SIZE_ADJ);
+ if (ifp->if_broot)
+ ASSERT(XFS_BMAP_BMDR_SPACE(ifp->if_broot) <=
+ XFS_IFORK_SIZE(ip, whichfork));
return;
}
@@ -2201,9 +2202,8 @@ xfs_iflush_fork(
if ((iip->ili_format.ilf_fields & brootflag[whichfork]) &&
(ifp->if_broot_bytes > 0)) {
ASSERT(ifp->if_broot != NULL);
- ASSERT(ifp->if_broot_bytes <=
- (XFS_IFORK_SIZE(ip, whichfork) +
- XFS_BROOT_SIZE_ADJ));
+ ASSERT(XFS_BMAP_BMDR_SPACE(ifp->if_broot) <=
+ XFS_IFORK_SIZE(ip, whichfork));
xfs_bmbt_to_bmdr(mp, ifp->if_broot, ifp->if_broot_bytes,
(xfs_bmdr_block_t *)cp,
XFS_DFORK_SIZE(dip, mp, whichfork));
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/1] xfs: use XFS_BMAP_BMDR_SPACE vs. XFS_BROOT_SIZE_ADJ
2012-03-28 17:21 ` [PATCH 2/1] xfs: use XFS_BMAP_BMDR_SPACE vs. XFS_BROOT_SIZE_ADJ Eric Sandeen
@ 2012-03-29 0:55 ` Dave Chinner
0 siblings, 0 replies; 9+ messages in thread
From: Dave Chinner @ 2012-03-29 0:55 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss
On Wed, Mar 28, 2012 at 12:21:11PM -0500, Eric Sandeen wrote:
> XFS_BROOT_SIZE_ADJ is an undocumented macro which accounts for
> the difference in size between the on-disk and in-core btree
> root. It's much clearer to just use the newly-added
> XFS_BMAP_BMDR_SPACE macro which gives us the on-disk size
> directly.
>
> In one case, we must test that the if_broot exists before
> applying the macro, however.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c
2012-03-27 20:40 [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
2012-03-27 20:50 ` Eric Sandeen
2012-03-28 17:21 ` [PATCH 2/1] xfs: use XFS_BMAP_BMDR_SPACE vs. XFS_BROOT_SIZE_ADJ Eric Sandeen
@ 2013-06-19 21:51 ` Eric Sandeen
2013-06-20 17:09 ` Ben Myers
2 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2013-06-19 21:51 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
On 3/27/12 3:40 PM, Eric Sandeen wrote:
> xfs_swap_extents_check_format() contains checks to make sure that
> original and the temporary files during defrag are compatible;
> Gabriel VLASIU ran into a case where xfs_fsr returned EINVAL
> because the tests found the btree root to be of size 120,
> while the fork offset was only 104; IOW, they overlapped.
>
> However, this is just due to an error in the
> xfs_swap_extents_check_format() tests, because it is checking
> the in-memory btree root size against the on-disk fork offset.
> We should be checking the on-disk sizes in both cases.
>
> This patch adds a new macro to calculate this size, and uses
> it in the tests.
>
> With this change, the filesystem image provided by Gabriel
> allows for proper file degragmentation.
I think this and the followup patch 2/1 got lost.
Ben, any idea?
Thanks,
-Eric
> Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/fs/xfs/xfs_bmap_btree.h b/fs/xfs/xfs_bmap_btree.h
> index 0e66c4e..d9eeb64 100644
> --- a/fs/xfs/xfs_bmap_btree.h
> +++ b/fs/xfs/xfs_bmap_btree.h
> @@ -189,12 +189,14 @@ typedef __be64 xfs_bmbt_ptr_t, xfs_bmdr_ptr_t;
> #define XFS_BMAP_BROOT_SPACE_CALC(nrecs) \
> (int)(XFS_BTREE_LBLOCK_LEN + \
> ((nrecs) * (sizeof(xfs_bmbt_key_t) + sizeof(xfs_bmbt_ptr_t))))
> -
> #define XFS_BMAP_BROOT_SPACE(bb) \
> (XFS_BMAP_BROOT_SPACE_CALC(be16_to_cpu((bb)->bb_numrecs)))
> +
> #define XFS_BMDR_SPACE_CALC(nrecs) \
> (int)(sizeof(xfs_bmdr_block_t) + \
> ((nrecs) * (sizeof(xfs_bmbt_key_t) + sizeof(xfs_bmbt_ptr_t))))
> +#define XFS_BMAP_BMDR_SPACE(bb) \
> + (XFS_BMDR_SPACE_CALC(be16_to_cpu((bb)->bb_numrecs)))
>
> /*
> * Maximum number of bmap btree levels.
> diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
> index dd974a5..2707d86 100644
> --- a/fs/xfs/xfs_dfrag.c
> +++ b/fs/xfs/xfs_dfrag.c
> @@ -26,6 +26,9 @@
> #include "xfs_ag.h"
> #include "xfs_mount.h"
> #include "xfs_bmap_btree.h"
> +#include "xfs_alloc_btree.h"
> +#include "xfs_ialloc_btree.h"
> +#include "xfs_btree.h"
> #include "xfs_dinode.h"
> #include "xfs_inode.h"
> #include "xfs_inode_item.h"
> @@ -184,7 +187,7 @@ xfs_swap_extents_check_format(
> */
> if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) {
> if (XFS_IFORK_BOFF(ip) &&
> - tip->i_df.if_broot_bytes > XFS_IFORK_BOFF(ip))
> + XFS_BMAP_BMDR_SPACE(tip->i_df.if_broot) > XFS_IFORK_BOFF(ip))
> return EINVAL;
> if (XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK) <=
> XFS_IFORK_MAXEXT(ip, XFS_DATA_FORK))
> @@ -194,9 +197,8 @@ xfs_swap_extents_check_format(
> /* Reciprocal target->temp btree format checks */
> if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE) {
> if (XFS_IFORK_BOFF(tip) &&
> - ip->i_df.if_broot_bytes > XFS_IFORK_BOFF(tip))
> + XFS_BMAP_BMDR_SPACE(ip->i_df.if_broot) > XFS_IFORK_BOFF(tip))
> return EINVAL;
> -
> if (XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) <=
> XFS_IFORK_MAXEXT(tip, XFS_DATA_FORK))
> return EINVAL;
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c
2013-06-19 21:51 ` [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
@ 2013-06-20 17:09 ` Ben Myers
2013-07-09 16:26 ` Eric Sandeen
0 siblings, 1 reply; 9+ messages in thread
From: Ben Myers @ 2013-06-20 17:09 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss
On Wed, Jun 19, 2013 at 04:51:41PM -0500, Eric Sandeen wrote:
> On 3/27/12 3:40 PM, Eric Sandeen wrote:
> > xfs_swap_extents_check_format() contains checks to make sure that
> > original and the temporary files during defrag are compatible;
> > Gabriel VLASIU ran into a case where xfs_fsr returned EINVAL
> > because the tests found the btree root to be of size 120,
> > while the fork offset was only 104; IOW, they overlapped.
> >
> > However, this is just due to an error in the
> > xfs_swap_extents_check_format() tests, because it is checking
> > the in-memory btree root size against the on-disk fork offset.
> > We should be checking the on-disk sizes in both cases.
> >
> > This patch adds a new macro to calculate this size, and uses
> > it in the tests.
> >
> > With this change, the filesystem image provided by Gabriel
> > allows for proper file degragmentation.
>
> I think this and the followup patch 2/1 got lost.
>
> Ben, any idea?
Yeah. Sorry Eric.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c
2013-06-20 17:09 ` Ben Myers
@ 2013-07-09 16:26 ` Eric Sandeen
2013-07-09 18:22 ` Ben Myers
0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2013-07-09 16:26 UTC (permalink / raw)
To: Ben Myers; +Cc: Eric Sandeen, xfs-oss
On 6/20/13 12:09 PM, Ben Myers wrote:
> On Wed, Jun 19, 2013 at 04:51:41PM -0500, Eric Sandeen wrote:
>> On 3/27/12 3:40 PM, Eric Sandeen wrote:
>>> xfs_swap_extents_check_format() contains checks to make sure that
>>> original and the temporary files during defrag are compatible;
>>> Gabriel VLASIU ran into a case where xfs_fsr returned EINVAL
>>> because the tests found the btree root to be of size 120,
>>> while the fork offset was only 104; IOW, they overlapped.
>>>
>>> However, this is just due to an error in the
>>> xfs_swap_extents_check_format() tests, because it is checking
>>> the in-memory btree root size against the on-disk fork offset.
>>> We should be checking the on-disk sizes in both cases.
>>>
>>> This patch adds a new macro to calculate this size, and uses
>>> it in the tests.
>>>
>>> With this change, the filesystem image provided by Gabriel
>>> allows for proper file degragmentation.
>>
>> I think this and the followup patch 2/1 got lost.
>>
>> Ben, any idea?
>
> Yeah. Sorry Eric.
I see the first patch is now merged. Can you please also merge the
2nd patch? It is also reviewed already.
Thanks,
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c
2013-07-09 16:26 ` Eric Sandeen
@ 2013-07-09 18:22 ` Ben Myers
0 siblings, 0 replies; 9+ messages in thread
From: Ben Myers @ 2013-07-09 18:22 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss
On Tue, Jul 09, 2013 at 11:26:20AM -0500, Eric Sandeen wrote:
> On 6/20/13 12:09 PM, Ben Myers wrote:
> > On Wed, Jun 19, 2013 at 04:51:41PM -0500, Eric Sandeen wrote:
> >> On 3/27/12 3:40 PM, Eric Sandeen wrote:
> >>> xfs_swap_extents_check_format() contains checks to make sure that
> >>> original and the temporary files during defrag are compatible;
> >>> Gabriel VLASIU ran into a case where xfs_fsr returned EINVAL
> >>> because the tests found the btree root to be of size 120,
> >>> while the fork offset was only 104; IOW, they overlapped.
> >>>
> >>> However, this is just due to an error in the
> >>> xfs_swap_extents_check_format() tests, because it is checking
> >>> the in-memory btree root size against the on-disk fork offset.
> >>> We should be checking the on-disk sizes in both cases.
> >>>
> >>> This patch adds a new macro to calculate this size, and uses
> >>> it in the tests.
> >>>
> >>> With this change, the filesystem image provided by Gabriel
> >>> allows for proper file degragmentation.
> >>
> >> I think this and the followup patch 2/1 got lost.
> >>
> >> Ben, any idea?
> >
> > Yeah. Sorry Eric.
>
> I see the first patch is now merged. Can you please also merge the
> 2nd patch? It is also reviewed already.
Patch still looks good in light of the crc work. Applied.
Reviewed-by: Ben Myers <bpm@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-07-09 18:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-27 20:40 [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
2012-03-27 20:50 ` Eric Sandeen
2012-03-27 21:16 ` Dave Chinner
2012-03-28 17:21 ` [PATCH 2/1] xfs: use XFS_BMAP_BMDR_SPACE vs. XFS_BROOT_SIZE_ADJ Eric Sandeen
2012-03-29 0:55 ` Dave Chinner
2013-06-19 21:51 ` [PATCH] xfs: check on-disk (not incore) btree root size in dfrag.c Eric Sandeen
2013-06-20 17:09 ` Ben Myers
2013-07-09 16:26 ` Eric Sandeen
2013-07-09 18:22 ` Ben Myers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.