[PATCH 0/4 V2] inode32/inode64 allocation changes

[PATCH 0/4 V2] 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.

Carlos Maiolino (4):
  xfs: make inode64 as the default allocation mode
  xfs: add xfs_set_inode32()
  xfs: set xfs_initialize_perag() to use xfs_set_inode32/64()
  xfs: Make inode32 a remountable option

 fs/xfs/xfs_mount.c |  43 +++----------------
 fs/xfs/xfs_super.c | 120 +++++++++++++++++++++++++++++++++++++++++------------
 fs/xfs/xfs_super.h |   2 +
 3 files changed, 101 insertions(+), 64 deletions(-)

-- 
1.7.11.4

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

[PATCH 1/4] 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.

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

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f1f2968..b1aa2db 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,13 +499,13 @@ 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[] = {
 		/* the few simple ones we can get from the mount struct */
 		{ XFS_MOUNT_COMPAT_IOSIZE,	"," MNTOPT_LARGEIO },
 		{ XFS_MOUNT_BARRIER,		"," MNTOPT_NOBARRIER },
-		{ XFS_MOUNT_SMALL_INUMS,	"," MNTOPT_64BITINODE },
 		{ 0, NULL }
 	};
 	struct proc_xfs_info	*xfs_infop;
-- 
1.7.11.4

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

[PATCH 2/4] xfs: add xfs_set_inode32()

[Carlos Maiolino]

xfs_set_inode32() can 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 enable xfs_set_inode64() to return a xfs_agnumber_t value to
also be used during mount/remount, instead of duplicate code

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_super.c | 102 ++++++++++++++++++++++++++++++++++++++++-------------
 fs/xfs/xfs_super.h |   2 ++
 2 files changed, 80 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index b1aa2db..2d7a0d9 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -598,6 +598,84 @@ xfs_max_file_offset(
 	return (((__uint64_t)pagefactor) << bitshift) - 1;
 }
 
+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);
+	mp->m_maxagi = index;
+
+	return index;
+}
+
+xfs_agnumber_t
+xfs_set_inode32(struct xfs_mount *mp)
+{
+	xfs_agnumber_t	index;
+	xfs_agnumber_t	i = 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) {
+			i++;
+			pag = xfs_perag_get(mp, index);
+			pag->pagi_inodeok = 0;
+			pag->pagf_metadata = 1;
+			xfs_perag_put(pag);
+			continue;
+		}
+
+		pag = xfs_perag_get(mp, index);
+		pag->pagi_inodeok = 1;
+		if (index < max_metadata)
+			pag->pagf_metadata = 1;
+
+		xfs_perag_put(pag);
+	}
+
+	index -= i;
+	mp->m_flags |= (XFS_MOUNT_32BITINODES |
+			XFS_MOUNT_SMALL_INUMS);
+	return index;
+}
+
 STATIC int
 xfs_blkdev_get(
 	xfs_mount_t		*mp,
@@ -1037,30 +1115,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 3/4] xfs: set xfs_initialize_perag() to use xfs_set_inode32/64()

[Carlos Maiolino]

Set xfs_initialize_perag() to use xfs_set_inode32() and xfs_set_inode64()
functions, removing duplicated code

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_mount.c | 43 +++++--------------------------------------
 1 file changed, 5 insertions(+), 38 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;
-- 
1.7.11.4

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

[PATCH 4/4] 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 2d7a0d9..02b9f54 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}
 };
 
@@ -1143,6 +1148,9 @@ xfs_fs_remount(
 		case Opt_inode64:
 			xfs_set_inode64(mp);
 			break;
+		case Opt_inode32:
+			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 2/4] xfs: add xfs_set_inode32()

[Brian Foster]

On 09/17/2012 12:51 PM, Carlos Maiolino wrote:
> xfs_set_inode32() can 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 enable xfs_set_inode64() to return a xfs_agnumber_t value to
> also be used during mount/remount, instead of duplicate code
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/xfs_super.c | 102 ++++++++++++++++++++++++++++++++++++++++-------------
>  fs/xfs/xfs_super.h |   2 ++
>  2 files changed, 80 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index b1aa2db..2d7a0d9 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -598,6 +598,84 @@ xfs_max_file_offset(
>  	return (((__uint64_t)pagefactor) << bitshift) - 1;
>  }
>  
> +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);
> +	mp->m_maxagi = index;
> +
> +	return index;
> +}
> +
> +xfs_agnumber_t
> +xfs_set_inode32(struct xfs_mount *mp)
> +{
> +	xfs_agnumber_t	index;
> +	xfs_agnumber_t	i = 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) {
> +			i++;
> +			pag = xfs_perag_get(mp, index);
> +			pag->pagi_inodeok = 0;
> +			pag->pagf_metadata = 1;

Is it correct to set pagf_metadata here? I'm learning some of this code
as I review this so I could be wrong, but it looks like pagf_metadata
basically sets a preference on an ag for metadata (e.g., presumably
because we are limiting metadata space on a potentially large fs). The
existing code looks like it sets pagf_metadata only on AG's below the
max_metadata mark...

Brian

> +			xfs_perag_put(pag);
> +			continue;
> +		}
> +
> +		pag = xfs_perag_get(mp, index);
> +		pag->pagi_inodeok = 1;
> +		if (index < max_metadata)
> +			pag->pagf_metadata = 1;
> +
> +		xfs_perag_put(pag);
> +	}
> +
> +	index -= i;
> +	mp->m_flags |= (XFS_MOUNT_32BITINODES |
> +			XFS_MOUNT_SMALL_INUMS);
> +	return index;
> +}
> +
>  STATIC int
>  xfs_blkdev_get(
>  	xfs_mount_t		*mp,
> @@ -1037,30 +1115,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[];
> 

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

Re: [PATCH 2/4] xfs: add xfs_set_inode32()

[Carlos Maiolino]

On Mon, Sep 17, 2012 at 10:12:59PM -0400, Brian Foster wrote:
> On 09/17/2012 12:51 PM, Carlos Maiolino wrote:
> > xfs_set_inode32() can 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 enable xfs_set_inode64() to return a xfs_agnumber_t value to
> > also be used during mount/remount, instead of duplicate code
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >  fs/xfs/xfs_super.c | 102 ++++++++++++++++++++++++++++++++++++++++-------------
> >  fs/xfs/xfs_super.h |   2 ++
> >  2 files changed, 80 insertions(+), 24 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index b1aa2db..2d7a0d9 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -598,6 +598,84 @@ xfs_max_file_offset(
> >  	return (((__uint64_t)pagefactor) << bitshift) - 1;
> >  }
> >  
> > +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);
> > +	mp->m_maxagi = index;
> > +
> > +	return index;
> > +}
> > +
> > +xfs_agnumber_t
> > +xfs_set_inode32(struct xfs_mount *mp)
> > +{
> > +	xfs_agnumber_t	index;
> > +	xfs_agnumber_t	i = 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) {
> > +			i++;
> > +			pag = xfs_perag_get(mp, index);
> > +			pag->pagi_inodeok = 0;
> > +			pag->pagf_metadata = 1;
> 
> Is it correct to set pagf_metadata here? I'm learning some of this code
> as I review this so I could be wrong, but it looks like pagf_metadata
> basically sets a preference on an ag for metadata (e.g., presumably
> because we are limiting metadata space on a potentially large fs). The
> existing code looks like it sets pagf_metadata only on AG's below the
> max_metadata mark...
> 
> Brian
> 
This AG will be preferred to inode allocation, so, pagf_metadata = 1. And since
using 32bit inodes we are limited where we can make inode allocations,
set the first AGs as metadata preferred makes sense to me.
(but I can be wrong too :)

> > +			xfs_perag_put(pag);
> > +			continue;
> > +		}
> > +
> > +		pag = xfs_perag_get(mp, index);
> > +		pag->pagi_inodeok = 1;
> > +		if (index < max_metadata)
> > +			pag->pagf_metadata = 1;
> > +
> > +		xfs_perag_put(pag);
> > +	}
> > +
> > +	index -= i;
> > +	mp->m_flags |= (XFS_MOUNT_32BITINODES |
> > +			XFS_MOUNT_SMALL_INUMS);
> > +	return index;
> > +}
> > +
> >  STATIC int
> >  xfs_blkdev_get(
> >  	xfs_mount_t		*mp,
> > @@ -1037,30 +1115,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[];
> > 
> 
> _______________________________________________
> 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/4] xfs: make inode64 as the default allocation mode

[Dave Chinner]

On Mon, Sep 17, 2012 at 01:51:39PM -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.

This needs to explain why XFS_MOUNT_SMALL_INUMS is still the default
for !XFS_BIG_INUMS. i.e. inode64 is not an unconditional default
value.

> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/xfs_super.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index f1f2968..b1aa2db 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,13 +499,13 @@ 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[] = {
>  		/* the few simple ones we can get from the mount struct */
>  		{ XFS_MOUNT_COMPAT_IOSIZE,	"," MNTOPT_LARGEIO },
>  		{ XFS_MOUNT_BARRIER,		"," MNTOPT_NOBARRIER },
> -		{ XFS_MOUNT_SMALL_INUMS,	"," MNTOPT_64BITINODE },

I'd leave that in the table, that way people will still know if they
are using inode64 or not by grepping /proc/self/mounts.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 2/4] xfs: add xfs_set_inode32()

[Dave Chinner]

On Mon, Sep 17, 2012 at 11:40:45PM -0300, Carlos Maiolino wrote:
> On Mon, Sep 17, 2012 at 10:12:59PM -0400, Brian Foster wrote:
> > On 09/17/2012 12:51 PM, Carlos Maiolino wrote:
> > > xfs_set_inode32() can 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 enable xfs_set_inode64() to return a xfs_agnumber_t value to
> > > also be used during mount/remount, instead of duplicate code

"This patch also changes xfs_set_inode64() to return the maximum AG
number that inodes can be allocated in so that the behaviour is the
same as xfs_set_inode32(). This simplifies code that calls these
functions and neds to know the maximum AG that inodes can be
allocated in."

....

> > > +	for (index = 0; index < sbp->sb_agcount; index++) {
> > > +		ino = XFS_AGINO_TO_INO(mp, index, agino);
> > > +
> > > +		if (ino > XFS_MAXINUMBER_32) {
> > > +			i++;
> > > +			pag = xfs_perag_get(mp, index);
> > > +			pag->pagi_inodeok = 0;
> > > +			pag->pagf_metadata = 1;
> > 
> > Is it correct to set pagf_metadata here? I'm learning some of this code
> > as I review this so I could be wrong, but it looks like pagf_metadata
> > basically sets a preference on an ag for metadata (e.g., presumably
> > because we are limiting metadata space on a potentially large fs). The
> > existing code looks like it sets pagf_metadata only on AG's below the
> > max_metadata mark...
> > 
> > Brian
> > 
> This AG will be preferred to inode allocation, so, pagf_metadata = 1. And since
> using 32bit inodes we are limited where we can make inode allocations,
> set the first AGs as metadata preferred makes sense to me.
> (but I can be wrong too :)

This discussion is being had because this code addition is in a
different patch that removes the code that it is replacing.  Indeed,
if you look at the code that is being replaced by this function,
it's clear that pag->pagf_metadata is only set on AGs that also have
pag->pagi_inodeok set.

Part of this confusion also comes about because you are intrducing
new functionality inthe same patch you are trying to factor code.
i.e. making more than one change in a single patch.

Hence I think that some restructuring of the patch set needs to be
done so each patch makes one clear change. The first patch is fine,
the second should factor xfs_initialize_perag() to use
xfs_set_inode64()/xfs_set_inode32() without changing any logic,
the third should introduce the inode64->inode32 transition support
into xfs_set_inode32() and the last can stay the same introducing
the remount support for inode32.


> > > +			xfs_perag_put(pag);
> > > +			continue;
> > > +		}
> > > +
> > > +		pag = xfs_perag_get(mp, index);
> > > +		pag->pagi_inodeok = 1;
> > > +		if (index < max_metadata)
> > > +			pag->pagf_metadata = 1;
> > > +
> > > +		xfs_perag_put(pag);
> > > +	}
> > > +
> > > +	index -= i;

And this is pretty clunky. Much better is to introduce a "maxagi"
variable and set it each time through the loop when you set
pag->pagi_inodeok = 1, and return that value. It becomes
immediataely clear what the return parameter of the function is
without needing to document it....

> > > +	mp->m_flags |= (XFS_MOUNT_32BITINODES |
> > > +			XFS_MOUNT_SMALL_INUMS);
> > > +	return index;
> > > +}
> > > +
> > >  STATIC int
> > >  xfs_blkdev_get(
> > >  	xfs_mount_t		*mp,
> > > @@ -1037,30 +1115,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;
> > > -}

There's also a problem of symmetry here - xfs_set_inode64() sets
mp->m_maxagi, which will break growfs logic when it is called by
xfs_initialize_perag() in the next patch. For growfs, mp->m_maxagi
cannot be updated until after all the AG headers are initialised and
written to disk, but to do that we need to have the xfs_perag
headers already initialised. Hence we have to avoid using them by
not updating mp->m_maxagi until after the initialisation is
complete (i.e. after the growfs transaction commits).

This is another reason that factoring should be done in a single
patch without changing logic ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[PATCH 4/4] 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 c4899d3..052dccb 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}
 };
 
@@ -1145,6 +1150,9 @@ xfs_fs_remount(
 		case Opt_inode64:
 			xfs_set_inode64(mp);
 			break;
+		case Opt_inode32:
+			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