All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.