xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 2/4] xfs: add xfs_set_inode32()
From: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
Date: Mon, 17 Sep 2012 23:40:45 -0300
In-reply-to: <5057D8AB.6000208@xxxxxxxxxx>
Mail-followup-to: xfs@xxxxxxxxxxx
References: <1347900702-23421-1-git-send-email-cmaiolino@xxxxxxxxxx> <1347900702-23421-3-git-send-email-cmaiolino@xxxxxxxxxx> <5057D8AB.6000208@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
--Carlos

<Prev in Thread] Current Thread [Next in Thread>