[PATCH 4/6] xfs: Fix mp->m_maxagi update during inode64 remount

[PATCH 4/6] xfs: Fix mp->m_maxagi update during inode64 remount

[Carlos Maiolino]

With the changes made on xfs_set_inode64(), to make it behave as
xfs_set_inode32() (now leaving to the caller the responsibility to update
mp->m_maxagi), we use the return value of xfs_set_inode64() to update
mp->m_maxagi during remount.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0c2e06f..966f56c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1129,7 +1129,7 @@ xfs_fs_remount(
 			mp->m_flags &= ~XFS_MOUNT_BARRIER;
 			break;
 		case Opt_inode64:
-			xfs_set_inode64(mp);
+			mp->m_maxagi = xfs_set_inode64(mp);
 			break;
 		default:
 			/*
-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 0/6 V4] inode32/inode64 allocation changes

[Carlos Maiolino]

This patch set adds inode64 as the default allocation mode, but also includes 2
patches to remove duplicated code and another one to make inode32 able to be
remounted.

NOTE: This patch has as dependency "Make inode64 a remountable option" patch.

V4 properly set pagf_metadata in patch 5

V3 has a better factoring of each patch goal, including other dchinner's
suggestions in V2. 
Also it contains a fix in how m_agirotor reset is made (patch 01) that I've
found while doing tests with these patches.

Carlos Maiolino (6):
  xfs: Fix m_agirotor reset during AG selection
  xfs: make inode64 as the default allocation mode
  xfs: reduce code duplication handling inode32/64 options
  xfs: Fix mp->m_maxagi update during inode64 remount
  xfs: add inode64->inode32 transition into xfs_set_inode32()
  xfs: Make inode32 a remountable option

 fs/xfs/xfs_ialloc.c |   2 +-
 fs/xfs/xfs_mount.c  |  43 +++----------------
 fs/xfs/xfs_super.c  | 117 ++++++++++++++++++++++++++++++++++++++++------------
 fs/xfs/xfs_super.h  |   2 +
 4 files changed, 99 insertions(+), 65 deletions(-)

-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/6] xfs: Fix m_agirotor reset during AG selection

[Carlos Maiolino]

xfs_ialloc_next_ag() currently resets m_agirotor when it is equal to m_maxagi:

         if (++mp->m_agirotor == mp->m_maxagi)
	         mp->m_agirotor = 0;

But, if for some reason mp->m_maxagi changes to a lower value than current
m_agirotor, this condition will never be true, causing m_agirotor to exceed the
maximum allowed value (m_maxagi).

This implies mainly during lookups for xfs_perag structs in its radix tree,
since the agno value used for the lookup is based on m_agirotor. An out-of-range
m_agirotor may cause a lookup failure which in case will return NULL.

As an example, the value of m_maxagi is decreased during inode64->inode32
remount process, case where I've found this problem.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_ialloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 5aceb3f..445bf1a 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -431,7 +431,7 @@ xfs_ialloc_next_ag(
 
 	spin_lock(&mp->m_agirotor_lock);
 	agno = mp->m_agirotor;
-	if (++mp->m_agirotor == mp->m_maxagi)
+	if (++mp->m_agirotor >= mp->m_maxagi)
 		mp->m_agirotor = 0;
 	spin_unlock(&mp->m_agirotor_lock);
 
-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/6] xfs: make inode64 as the default allocation mode

[Carlos Maiolino]

since 64-bit inodes can be accessed while using inode32, and these can also be
used on 32-bit kernels, there is no reason to still keep inode32 as the default
mount option.
If the filesystem cannot handle 64bit inode numbers (i.e CONFIG_LBDAF is not
enabled and BITS_PER_LONG == 32), XFS_MOUNT_SMALL_INUMS will still be set by
default, so inode64 is not an unconditional default value.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_super.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f1f2968..7686eee 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -88,6 +88,8 @@ mempool_t *xfs_ioend_pool;
 					 * unwritten extent conversion */
 #define MNTOPT_NOBARRIER "nobarrier"	/* .. disable */
 #define MNTOPT_64BITINODE   "inode64"	/* inodes can be allocated anywhere */
+#define MNTOPT_32BITINODE   "inode32"	/* inode allocation limited to
+					 * XFS_MAXINUMBER_32 */
 #define MNTOPT_IKEEP	"ikeep"		/* do not free empty inode clusters */
 #define MNTOPT_NOIKEEP	"noikeep"	/* free empty inode clusters */
 #define MNTOPT_LARGEIO	   "largeio"	/* report large I/O sizes in stat() */
@@ -198,7 +200,9 @@ xfs_parseargs(
 	 */
 	mp->m_flags |= XFS_MOUNT_BARRIER;
 	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
+#if !XFS_BIG_INUMS
 	mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+#endif
 
 	/*
 	 * These can be overridden by the mount option parsing.
@@ -295,6 +299,8 @@ xfs_parseargs(
 				return EINVAL;
 			}
 			dswidth = simple_strtoul(value, &eov, 10);
+		} else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
+			mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
 		} else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
 			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
 #if !XFS_BIG_INUMS
@@ -493,6 +499,7 @@ xfs_showargs(
 		{ XFS_MOUNT_FILESTREAMS,	"," MNTOPT_FILESTREAM },
 		{ XFS_MOUNT_GRPID,		"," MNTOPT_GRPID },
 		{ XFS_MOUNT_DISCARD,		"," MNTOPT_DISCARD },
+		{ XFS_MOUNT_SMALL_INUMS,	"," MNTOPT_32BITINODE },
 		{ 0, NULL }
 	};
 	static struct proc_xfs_info xfs_info_unset[] = {
-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/6] xfs: reduce code duplication handling inode32/64 options

[Carlos Maiolino]

Add xfs_set_inode32() to be used to enable inode32 allocation mode. this will
reduce the amount of duplicated code needed to mount/remount a filesystem with
inode32 option.
This patch also changes xfs_set_inode64() to return the maximum AG number that
inodes can be allocated instead of set mp->m_maxagi by itself, so that the
behaviour is the same as xfs_set_inode32().
This simplifies code that calls these functions and needs to know the maximum
AG that inodes can be allocated in.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_mount.c | 43 +++-----------------------
 fs/xfs/xfs_super.c | 89 +++++++++++++++++++++++++++++++++++++++---------------
 fs/xfs/xfs_super.h |  2 ++
 3 files changed, 72 insertions(+), 62 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 29c2f83..b2bd3a0 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -440,7 +440,7 @@ xfs_initialize_perag(
 	xfs_agnumber_t	agcount,
 	xfs_agnumber_t	*maxagi)
 {
-	xfs_agnumber_t	index, max_metadata;
+	xfs_agnumber_t	index;
 	xfs_agnumber_t	first_initialised = 0;
 	xfs_perag_t	*pag;
 	xfs_agino_t	agino;
@@ -500,43 +500,10 @@ xfs_initialize_perag(
 	else
 		mp->m_flags &= ~XFS_MOUNT_32BITINODES;
 
-	if (mp->m_flags & XFS_MOUNT_32BITINODES) {
-		/*
-		 * Calculate how much should be reserved for inodes to meet
-		 * the max inode percentage.
-		 */
-		if (mp->m_maxicount) {
-			__uint64_t	icount;
-
-			icount = sbp->sb_dblocks * sbp->sb_imax_pct;
-			do_div(icount, 100);
-			icount += sbp->sb_agblocks - 1;
-			do_div(icount, sbp->sb_agblocks);
-			max_metadata = icount;
-		} else {
-			max_metadata = agcount;
-		}
-
-		for (index = 0; index < agcount; index++) {
-			ino = XFS_AGINO_TO_INO(mp, index, agino);
-			if (ino > XFS_MAXINUMBER_32) {
-				index++;
-				break;
-			}
-
-			pag = xfs_perag_get(mp, index);
-			pag->pagi_inodeok = 1;
-			if (index < max_metadata)
-				pag->pagf_metadata = 1;
-			xfs_perag_put(pag);
-		}
-	} else {
-		for (index = 0; index < agcount; index++) {
-			pag = xfs_perag_get(mp, index);
-			pag->pagi_inodeok = 1;
-			xfs_perag_put(pag);
-		}
-	}
+	if (mp->m_flags & XFS_MOUNT_32BITINODES)
+		index = xfs_set_inode32(mp);
+	else
+		index = xfs_set_inode64(mp);
 
 	if (maxagi)
 		*maxagi = index;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 7686eee..0c2e06f 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -599,6 +599,71 @@ xfs_max_file_offset(
 	return (((__uint64_t)pagefactor) << bitshift) - 1;
 }
 
+xfs_agnumber_t
+xfs_set_inode32(struct xfs_mount *mp)
+{
+	xfs_agnumber_t	index = 0;
+	xfs_sb_t	*sbp = &mp->m_sb;
+	xfs_agnumber_t	max_metadata;
+	xfs_agino_t	agino =	XFS_OFFBNO_TO_AGINO(mp, sbp->sb_agblocks -1, 0);
+	xfs_ino_t	ino = XFS_AGINO_TO_INO(mp, sbp->sb_agcount -1, agino);
+	xfs_perag_t	*pag;
+
+	/* Calculate how much should be reserved for inodes to meet
+	 * the max inode percentage.
+	 */
+	if (mp->m_maxicount) {
+		__uint64_t	icount;
+
+		icount = sbp->sb_dblocks * sbp->sb_imax_pct;
+		do_div(icount, 100);
+		icount += sbp->sb_agblocks - 1;
+		do_div(icount, sbp->sb_agblocks);
+		max_metadata = icount;
+	} else {
+		max_metadata = sbp->sb_agcount;
+	}
+
+	for (index = 0; index < sbp->sb_agcount; index++) {
+		ino = XFS_AGINO_TO_INO(mp, index, agino);
+		if (ino > XFS_MAXINUMBER_32) {
+			index++;
+			break;
+		}
+
+		pag = xfs_perag_get(mp, index);
+		pag->pagi_inodeok = 1;
+		if (index < max_metadata)
+			pag->pagf_metadata = 1;
+		xfs_perag_put(pag);
+	}
+	return index;
+}
+
+xfs_agnumber_t
+xfs_set_inode64(struct xfs_mount *mp)
+{
+	xfs_agnumber_t index = 0;
+
+	for (index = 0; index < mp->m_sb.sb_agcount; index++) {
+		struct xfs_perag	*pag;
+
+		pag = xfs_perag_get(mp, index);
+		pag->pagi_inodeok = 1;
+		pag->pagf_metadata = 0;
+		xfs_perag_put(pag);
+	}
+
+	/* There is no need for lock protection on m_flags,
+	 * the rw_semaphore of the VFS superblock is locked
+	 * during mount/umount/remount operations, so this is
+	 * enough to avoid concurency on the m_flags field
+	 */
+	mp->m_flags &= ~(XFS_MOUNT_32BITINODES |
+			 XFS_MOUNT_SMALL_INUMS);
+	return index;
+}
+
 STATIC int
 xfs_blkdev_get(
 	xfs_mount_t		*mp,
@@ -1038,30 +1103,6 @@ xfs_restore_resvblks(struct xfs_mount *mp)
 	xfs_reserve_blocks(mp, &resblks, NULL);
 }
 
-STATIC void
-xfs_set_inode64(struct xfs_mount *mp)
-{
-	int i = 0;
-
-	for (i = 0; i < mp->m_sb.sb_agcount; i++) {
-		struct xfs_perag	*pag;
-
-		pag = xfs_perag_get(mp, i);
-		pag->pagi_inodeok = 1;
-		pag->pagf_metadata = 0;
-		xfs_perag_put(pag);
-	}
-
-	/* There is no need for lock protection on m_flags,
-	 * the rw_semaphore of the VFS superblock is locked
-	 * during mount/umount/remount operations, so this is
-	 * enough to avoid concurency on the m_flags field
-	 */
-	mp->m_flags &= ~(XFS_MOUNT_32BITINODES |
-			 XFS_MOUNT_SMALL_INUMS);
-	mp->m_maxagi = i;
-}
-
 STATIC int
 xfs_fs_remount(
 	struct super_block	*sb,
diff --git a/fs/xfs/xfs_super.h b/fs/xfs/xfs_super.h
index 09b0c26..9de4a92 100644
--- a/fs/xfs/xfs_super.h
+++ b/fs/xfs/xfs_super.h
@@ -75,6 +75,8 @@ struct block_device;
 extern __uint64_t xfs_max_file_offset(unsigned int);
 
 extern void xfs_blkdev_issue_flush(struct xfs_buftarg *);
+extern xfs_agnumber_t xfs_set_inode32(struct xfs_mount *);
+extern xfs_agnumber_t xfs_set_inode64(struct xfs_mount *);
 
 extern const struct export_operations xfs_export_operations;
 extern const struct xattr_handler *xfs_xattr_handlers[];
-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/6] xfs: Fix mp->m_maxagi update during inode64 remount

[Carlos Maiolino]

With the changes made on xfs_set_inode64(), to make it behave as
xfs_set_inode32() (now leaving to the caller the responsibility to update
mp->m_maxagi), we use the return value of xfs_set_inode64() to update
mp->m_maxagi during remount.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0c2e06f..966f56c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1129,7 +1129,7 @@ xfs_fs_remount(
 			mp->m_flags &= ~XFS_MOUNT_BARRIER;
 			break;
 		case Opt_inode64:
-			xfs_set_inode64(mp);
+			mp->m_maxagi = xfs_set_inode64(mp);
 			break;
 		default:
 			/*
-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/6] xfs: add inode64->inode32 transition into xfs_set_inode32()

[Carlos Maiolino]

To make inode32 a remountable option, xfs_set_inode32() should be able to make a
transition from inode64 option, disabling inode allocation on higher AGs.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_super.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 966f56c..3329296 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -603,6 +603,7 @@ xfs_agnumber_t
 xfs_set_inode32(struct xfs_mount *mp)
 {
 	xfs_agnumber_t	index = 0;
+	xfs_agnumber_t	maxagi = 0;
 	xfs_sb_t	*sbp = &mp->m_sb;
 	xfs_agnumber_t	max_metadata;
 	xfs_agino_t	agino =	XFS_OFFBNO_TO_AGINO(mp, sbp->sb_agblocks -1, 0);
@@ -626,18 +627,26 @@ xfs_set_inode32(struct xfs_mount *mp)
 
 	for (index = 0; index < sbp->sb_agcount; index++) {
 		ino = XFS_AGINO_TO_INO(mp, index, agino);
+
 		if (ino > XFS_MAXINUMBER_32) {
-			index++;
-			break;
+			pag = xfs_perag_get(mp, index);
+			pag->pagi_inodeok = 0;
+			pag->pagf_metadata = 0;
+			xfs_perag_put(pag);
+			continue;
 		}
 
 		pag = xfs_perag_get(mp, index);
 		pag->pagi_inodeok = 1;
+		maxagi++;
 		if (index < max_metadata)
 			pag->pagf_metadata = 1;
 		xfs_perag_put(pag);
 	}
-	return index;
+	mp->m_flags |= (XFS_MOUNT_32BITINODES |
+			XFS_MOUNT_SMALL_INUMS);
+
+	return maxagi;
 }
 
 xfs_agnumber_t
-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 6/6] xfs: Make inode32 a remountable option

[Carlos Maiolino]

As inode64 is the default option now, and was also made remountable previously,
inode32 can also be remounted on-the-fly when it is needed.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_super.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 3329296..545ab6f 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -122,13 +122,18 @@ mempool_t *xfs_ioend_pool;
  * in the future, too.
  */
 enum {
-	Opt_barrier, Opt_nobarrier, Opt_inode64, Opt_err
+	Opt_barrier,
+	Opt_nobarrier,
+	Opt_inode64,
+	Opt_inode32,
+	Opt_err
 };
 
 static const match_table_t tokens = {
 	{Opt_barrier, "barrier"},
 	{Opt_nobarrier, "nobarrier"},
 	{Opt_inode64, "inode64"},
+	{Opt_inode32, "inode32"},
 	{Opt_err, NULL}
 };
 
@@ -1140,6 +1145,9 @@ xfs_fs_remount(
 		case Opt_inode64:
 			mp->m_maxagi = xfs_set_inode64(mp);
 			break;
+		case Opt_inode32:
+			mp->m_maxagi = xfs_set_inode32(mp);
+			break;
 		default:
 			/*
 			 * Logically we would return an error here to prevent
-- 
1.7.11.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/6 V4] inode32/inode64 allocation changes

[Brian Foster]

On 09/20/2012 09:32 AM, Carlos Maiolino wrote:
> This patch set adds inode64 as the default allocation mode, but also includes 2
> patches to remove duplicated code and another one to make inode32 able to be
> remounted.
> 
> NOTE: This patch has as dependency "Make inode64 a remountable option" patch.
> 
> V4 properly set pagf_metadata in patch 5
> 

Hi Carlos,

With that fix, this patchset looks good to me, thanks.

Reviewed-by: Brian Foster <bfoster@redhat.com>

Brian

> V3 has a better factoring of each patch goal, including other dchinner's
> suggestions in V2. 
> Also it contains a fix in how m_agirotor reset is made (patch 01) that I've
> found while doing tests with these patches.
> 
> Carlos Maiolino (6):
>   xfs: Fix m_agirotor reset during AG selection
>   xfs: make inode64 as the default allocation mode
>   xfs: reduce code duplication handling inode32/64 options
>   xfs: Fix mp->m_maxagi update during inode64 remount
>   xfs: add inode64->inode32 transition into xfs_set_inode32()
>   xfs: Make inode32 a remountable option
> 
>  fs/xfs/xfs_ialloc.c |   2 +-
>  fs/xfs/xfs_mount.c  |  43 +++----------------
>  fs/xfs/xfs_super.c  | 117 ++++++++++++++++++++++++++++++++++++++++------------
>  fs/xfs/xfs_super.h  |   2 +
>  4 files changed, 99 insertions(+), 65 deletions(-)
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/6 V4] inode32/inode64 allocation changes

[Ben Myers]

Hey Carlos,

On Thu, Sep 20, 2012 at 10:32:35AM -0300, Carlos Maiolino wrote:
> This patch set adds inode64 as the default allocation mode, but also includes 2
> patches to remove duplicated code and another one to make inode32 able to be
> remounted.
> 
> NOTE: This patch has as dependency "Make inode64 a remountable option" patch.
> 
> V4 properly set pagf_metadata in patch 5
> 
> V3 has a better factoring of each patch goal, including other dchinner's
> suggestions in V2. 
> Also it contains a fix in how m_agirotor reset is made (patch 01) that I've
> found while doing tests with these patches.
> 
> Carlos Maiolino (6):
>   xfs: Fix m_agirotor reset during AG selection
>   xfs: make inode64 as the default allocation mode
>   xfs: reduce code duplication handling inode32/64 options
>   xfs: Fix mp->m_maxagi update during inode64 remount
>   xfs: add inode64->inode32 transition into xfs_set_inode32()
>   xfs: Make inode32 a remountable option
> 
>  fs/xfs/xfs_ialloc.c |   2 +-
>  fs/xfs/xfs_mount.c  |  43 +++----------------
>  fs/xfs/xfs_super.c  | 117 ++++++++++++++++++++++++++++++++++++++++------------
>  fs/xfs/xfs_super.h  |   2 +
>  4 files changed, 99 insertions(+), 65 deletions(-)

This patch does not apply cleanly on the master branch.  What are you running?

Thanks,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/6 V4] inode32/inode64 allocation changes

[Ben Myers]

On Fri, Sep 21, 2012 at 04:22:25PM -0500, Ben Myers wrote:
> On Thu, Sep 20, 2012 at 10:32:35AM -0300, Carlos Maiolino wrote:
> > This patch set adds inode64 as the default allocation mode, but also includes 2
> > patches to remove duplicated code and another one to make inode32 able to be
> > remounted.
> > 
> > NOTE: This patch has as dependency "Make inode64 a remountable option" patch.
> > 
> > V4 properly set pagf_metadata in patch 5
> > 
> > V3 has a better factoring of each patch goal, including other dchinner's
> > suggestions in V2. 
> > Also it contains a fix in how m_agirotor reset is made (patch 01) that I've
> > found while doing tests with these patches.
> > 
> > Carlos Maiolino (6):
> >   xfs: Fix m_agirotor reset during AG selection
> >   xfs: make inode64 as the default allocation mode
> >   xfs: reduce code duplication handling inode32/64 options
> >   xfs: Fix mp->m_maxagi update during inode64 remount
> >   xfs: add inode64->inode32 transition into xfs_set_inode32()
> >   xfs: Make inode32 a remountable option
> > 
> >  fs/xfs/xfs_ialloc.c |   2 +-
> >  fs/xfs/xfs_mount.c  |  43 +++----------------
> >  fs/xfs/xfs_super.c  | 117 ++++++++++++++++++++++++++++++++++++++++------------
> >  fs/xfs/xfs_super.h  |   2 +
> >  4 files changed, 99 insertions(+), 65 deletions(-)
> 
> This patch does not apply cleanly on the master branch.  What are you running?

Never mind.  I needed Message-Id: <1345238378-10123-1-git-send-email-cmaiolino@redhat.com>

Regards,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/6] xfs: Fix m_agirotor reset during AG selection

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/6] xfs: make inode64 as the default allocation mode

[Christoph Hellwig]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 748 bytes --]

On Thu, Sep 20, 2012 at 10:32:37AM -0300, Carlos Maiolino wrote:
> since 64-bit inodes can be accessed while using inode32, and these can also be
> used on 32-bit kernels, there is no reason to still keep inode32 as the default
> mount option.
> If the filesystem cannot handle 64bit inode numbers (i.e CONFIG_LBDAF is not
> enabled and BITS_PER_LONG == 32), XFS_MOUNT_SMALL_INUMS will still be set by
> default, so inode64 is not an unconditional default value.

Btw, I think we should kill the !BIG_INUMS mode and just
require CONFIG_LBDAF on 32-bit, as the small in-core inums are å
codepath that rarely if ever is tested and not really useful these days.

But that should be a separate patch, so:


Reviewed-by: Christoph Hellwig <hch@lst.de>


[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/6] xfs: reduce code duplication handling inode32/64 options

[Christoph Hellwig]

On Thu, Sep 20, 2012 at 10:32:38AM -0300, Carlos Maiolino wrote:
> Add xfs_set_inode32() to be used to enable inode32 allocation mode. this will
> reduce the amount of duplicated code needed to mount/remount a filesystem with
> inode32 option.
> This patch also changes xfs_set_inode64() to return the maximum AG number that
> inodes can be allocated instead of set mp->m_maxagi by itself, so that the
> behaviour is the same as xfs_set_inode32().
> This simplifies code that calls these functions and needs to know the maximum
> AG that inodes can be allocated in.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/6] xfs: Fix mp->m_maxagi update during inode64 remount

[Christoph Hellwig]

On Thu, Sep 20, 2012 at 10:32:39AM -0300, Carlos Maiolino wrote:
> With the changes made on xfs_set_inode64(), to make it behave as
> xfs_set_inode32() (now leaving to the caller the responsibility to update
> mp->m_maxagi), we use the return value of xfs_set_inode64() to update
> mp->m_maxagi during remount.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Shouldn;t this be part of the previous patch?

Otherwise looks good.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 5/6] xfs: add inode64->inode32 transition into xfs_set_inode32()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 6/6] xfs: Make inode32 a remountable option

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/6] xfs: Fix mp->m_maxagi update during inode64 remount

[Carlos Maiolino]

On Tue, Sep 25, 2012 at 05:37:15AM -0400, Christoph Hellwig wrote:
> On Thu, Sep 20, 2012 at 10:32:39AM -0300, Carlos Maiolino wrote:
> > With the changes made on xfs_set_inode64(), to make it behave as
> > xfs_set_inode32() (now leaving to the caller the responsibility to update
> > mp->m_maxagi), we use the return value of xfs_set_inode64() to update
> > mp->m_maxagi during remount.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> Shouldn;t this be part of the previous patch?
> 
> Otherwise looks good.
> 
dchinner suggested to keep small changes into different patches, once the
xfs_set_inode64() was added previously, adding a separate patche to add
xfs_set_inode32() and another one to change the behaviour of both functions
looks reasonable to me
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
--Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/6] xfs: Fix m_agirotor reset during AG selection

[Mark Tinguely]

On 09/20/12 08:32, Carlos Maiolino wrote:
> xfs_ialloc_next_ag() currently resets m_agirotor when it is equal to m_maxagi:
>
>           if (++mp->m_agirotor == mp->m_maxagi)
> 	         mp->m_agirotor = 0;
>
> But, if for some reason mp->m_maxagi changes to a lower value than current
> m_agirotor, this condition will never be true, causing m_agirotor to exceed the
> maximum allowed value (m_maxagi).
>
> This implies mainly during lookups for xfs_perag structs in its radix tree,
> since the agno value used for the lookup is based on m_agirotor. An out-of-range
> m_agirotor may cause a lookup failure which in case will return NULL.
>
> As an example, the value of m_maxagi is decreased during inode64->inode32
> remount process, case where I've found this problem.
>
> Signed-off-by: Carlos Maiolino<cmaiolino@redhat.com>
> ---


Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/6] xfs: make inode64 as the default allocation mode

[Mark Tinguely]

On 09/20/12 08:32, Carlos Maiolino wrote:
> since 64-bit inodes can be accessed while using inode32, and these can also be
> used on 32-bit kernels, there is no reason to still keep inode32 as the default
> mount option.
> If the filesystem cannot handle 64bit inode numbers (i.e CONFIG_LBDAF is not
> enabled and BITS_PER_LONG == 32), XFS_MOUNT_SMALL_INUMS will still be set by
> default, so inode64 is not an unconditional default value.
>
> Signed-off-by: Carlos Maiolino<cmaiolino@redhat.com>
> ---

Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/6] xfs: reduce code duplication handling inode32/64 options

[Mark Tinguely]

On 09/20/12 08:32, Carlos Maiolino wrote:
> Add xfs_set_inode32() to be used to enable inode32 allocation mode. this will
> reduce the amount of duplicated code needed to mount/remount a filesystem with
> inode32 option.
> This patch also changes xfs_set_inode64() to return the maximum AG number that
> inodes can be allocated instead of set mp->m_maxagi by itself, so that the
> behaviour is the same as xfs_set_inode32().
> This simplifies code that calls these functions and needs to know the maximum
> AG that inodes can be allocated in.
>
> Signed-off-by: Carlos Maiolino<cmaiolino@redhat.com>
> ---

Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/6] xfs: Fix mp->m_maxagi update during inode64 remount

[Mark Tinguely]

On 09/20/12 08:32, Carlos Maiolino wrote:
> With the changes made on xfs_set_inode64(), to make it behave as
> xfs_set_inode32() (now leaving to the caller the responsibility to update
> mp->m_maxagi), we use the return value of xfs_set_inode64() to update
> mp->m_maxagi during remount.
>
> Signed-off-by: Carlos Maiolino<cmaiolino@redhat.com>
> ---

Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 5/6] xfs: add inode64->inode32 transition into xfs_set_inode32()

[Mark Tinguely]

On 09/20/12 08:32, Carlos Maiolino wrote:
> To make inode32 a remountable option, xfs_set_inode32() should be able to make a
> transition from inode64 option, disabling inode allocation on higher AGs.
>
> Signed-off-by: Carlos Maiolino<cmaiolino@redhat.com>
> ---

Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 6/6] xfs: Make inode32 a remountable option

[Mark Tinguely]

On 09/20/12 08:32, Carlos Maiolino wrote:
> As inode64 is the default option now, and was also made remountable previously,
> inode32 can also be remounted on-the-fly when it is needed.
>
> Signed-off-by: Carlos Maiolino<cmaiolino@redhat.com>
> ---


Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/6 V4] inode32/inode64 allocation changes

[Ben Myers]

On Thu, Sep 20, 2012 at 10:32:35AM -0300, Carlos Maiolino wrote:
> This patch set adds inode64 as the default allocation mode, but also includes 2
> patches to remove duplicated code and another one to make inode32 able to be
> remounted.
> 
> NOTE: This patch has as dependency "Make inode64 a remountable option" patch.
> 
> V4 properly set pagf_metadata in patch 5
> 
> V3 has a better factoring of each patch goal, including other dchinner's
> suggestions in V2. 
> Also it contains a fix in how m_agirotor reset is made (patch 01) that I've
> found while doing tests with these patches.
> 
> Carlos Maiolino (6):
>   xfs: Fix m_agirotor reset during AG selection
>   xfs: make inode64 as the default allocation mode
>   xfs: reduce code duplication handling inode32/64 options
>   xfs: Fix mp->m_maxagi update during inode64 remount
>   xfs: add inode64->inode32 transition into xfs_set_inode32()
>   xfs: Make inode32 a remountable option
> 
>  fs/xfs/xfs_ialloc.c |   2 +-
>  fs/xfs/xfs_mount.c  |  43 +++----------------
>  fs/xfs/xfs_super.c  | 117 ++++++++++++++++++++++++++++++++++++++++------------
>  fs/xfs/xfs_super.h  |   2 +
>  4 files changed, 99 insertions(+), 65 deletions(-)

This series has been committed to git://oss.sgi.com/xfs/xfs.git, master and for-next branches.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs