xfs
[Top] [All Lists]

Re: [PATCH 6/7] xfs: use generic inode iterator in xfs_qm_dqrele_all_ino

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/7] xfs: use generic inode iterator in xfs_qm_dqrele_all_inodes
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 05 Jun 2009 15:11:55 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20090605191744.GA17489@xxxxxxxxxxxxx>
References: <20090514171233.942489000@xxxxxxxxxxxxxxxxxxxxxx> <20090514171559.029154000@xxxxxxxxxxxxxxxxxxxxxx> <4A296EDA.9020802@xxxxxxxxxxx> <20090605191744.GA17489@xxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.21 (X11/20090320)
Christoph Hellwig wrote:
> On Fri, Jun 05, 2009 at 02:15:38PM -0500, Eric Sandeen wrote:
>> Does this need the same error propagation treatment as 5/7 ?
> 
> Yes, I've already fixed this up in my local copy:
> 
> Subject: xfs: use generic inode iterator in xfs_qm_dqrele_all_inodes
> From: Christoph Hellwig <hch@xxxxxx>
> 
> Use xfs_inode_ag_iterator instead of opencoding the inode walk in the
> quota code.  Mark xfs_inode_ag_iterator and xfs_sync_inode_valid non-static
> to allow using them from the quota code.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Josef 'Jeff' Sipek <jeffpc@xxxxxxxxxxxxxx>

Ok, this looks fine to me.

Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>

> 
> Index: xfs/fs/xfs/quota/xfs_qm_syscalls.c
> ===================================================================
> --- xfs.orig/fs/xfs/quota/xfs_qm_syscalls.c   2009-06-04 12:47:30.742821242 
> +0200
> +++ xfs/fs/xfs/quota/xfs_qm_syscalls.c        2009-06-04 13:03:51.039832673 
> +0200
> @@ -847,105 +847,55 @@ xfs_qm_export_flags(
>  }
>  
>  
> -/*
> - * Release all the dquots on the inodes in an AG.
> - */
> -STATIC void
> -xfs_qm_dqrele_inodes_ag(
> -     xfs_mount_t     *mp,
> -     int             ag,
> -     uint            flags)
> +STATIC int
> +xfs_dqrele_inode(
> +     struct xfs_inode        *ip,
> +     struct xfs_perag        *pag,
> +     int                     flags)
>  {
> -     xfs_inode_t     *ip = NULL;
> -     xfs_perag_t     *pag = &mp->m_perag[ag];
> -     int             first_index = 0;
> -     int             nr_found;
> -
> -     do {
> -             /*
> -              * use a gang lookup to find the next inode in the tree
> -              * as the tree is sparse and a gang lookup walks to find
> -              * the number of objects requested.
> -              */
> -             read_lock(&pag->pag_ici_lock);
> -             nr_found = radix_tree_gang_lookup(&pag->pag_ici_root,
> -                             (void**)&ip, first_index, 1);
> -
> -             if (!nr_found) {
> -                     read_unlock(&pag->pag_ici_lock);
> -                     break;
> -             }
> -
> -             /*
> -              * Update the index for the next lookup. Catch overflows
> -              * into the next AG range which can occur if we have inodes
> -              * in the last block of the AG and we are currently
> -              * pointing to the last inode.
> -              */
> -             first_index = XFS_INO_TO_AGINO(mp, ip->i_ino + 1);
> -             if (first_index < XFS_INO_TO_AGINO(mp, ip->i_ino)) {
> -                     read_unlock(&pag->pag_ici_lock);
> -                     break;
> -             }
> -
> -             /* skip quota inodes */
> -             if (ip == XFS_QI_UQIP(mp) || ip == XFS_QI_GQIP(mp)) {
> -                     ASSERT(ip->i_udquot == NULL);
> -                     ASSERT(ip->i_gdquot == NULL);
> -                     read_unlock(&pag->pag_ici_lock);
> -                     continue;
> -             }
> +     int                     error;
>  
> -             /*
> -              * If we can't get a reference on the inode, it must be
> -              * in reclaim. Leave it for the reclaim code to flush.
> -              */
> -             if (!igrab(VFS_I(ip))) {
> -                     read_unlock(&pag->pag_ici_lock);
> -                     continue;
> -             }
> +     /* skip quota inodes */
> +     if (ip == XFS_QI_UQIP(ip->i_mount) || ip == XFS_QI_GQIP(ip->i_mount)) {
> +             ASSERT(ip->i_udquot == NULL);
> +             ASSERT(ip->i_gdquot == NULL);
>               read_unlock(&pag->pag_ici_lock);
> +             return 0;
> +     }
>  
> -             /* avoid new inodes though we shouldn't find any here */
> -             if (xfs_iflags_test(ip, XFS_INEW)) {
> -                     IRELE(ip);
> -                     continue;
> -             }
> +     error = xfs_sync_inode_valid(ip, pag);
> +     if (error)
> +             return error;
>  
> -             xfs_ilock(ip, XFS_ILOCK_EXCL);
> -             if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
> -                     xfs_qm_dqrele(ip->i_udquot);
> -                     ip->i_udquot = NULL;
> -             }
> -             if (flags & (XFS_PQUOTA_ACCT|XFS_GQUOTA_ACCT) &&
> -                 ip->i_gdquot) {
> -                     xfs_qm_dqrele(ip->i_gdquot);
> -                     ip->i_gdquot = NULL;
> -             }
> -             xfs_iput(ip, XFS_ILOCK_EXCL);
> +     xfs_ilock(ip, XFS_ILOCK_EXCL);
> +     if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
> +             xfs_qm_dqrele(ip->i_udquot);
> +             ip->i_udquot = NULL;
> +     }
> +     if (flags & (XFS_PQUOTA_ACCT|XFS_GQUOTA_ACCT) && ip->i_gdquot) {
> +             xfs_qm_dqrele(ip->i_gdquot);
> +             ip->i_gdquot = NULL;
> +     }
> +     xfs_iput(ip, XFS_ILOCK_EXCL);
> +     IRELE(ip);
>  
> -     } while (nr_found);
> +     return 0;
>  }
>  
> +
>  /*
>   * Go thru all the inodes in the file system, releasing their dquots.
> + *
>   * Note that the mount structure gets modified to indicate that quotas are 
> off
> - * AFTER this, in the case of quotaoff. This also gets called from
> - * xfs_rootumount.
> + * AFTER this, in the case of quotaoff.
>   */
>  void
>  xfs_qm_dqrele_all_inodes(
>       struct xfs_mount *mp,
>       uint             flags)
>  {
> -     int             i;
> -
>       ASSERT(mp->m_quotainfo);
> -     for (i = 0; i < mp->m_sb.sb_agcount; i++) {
> -             if (!mp->m_perag[i].pag_ici_init)
> -                     continue;
> -             xfs_qm_dqrele_inodes_ag(mp, i, flags);
> -     }
> +     xfs_inode_ag_iterator(mp, xfs_dqrele_inode, flags, -1);
>  }
>  
>  /*------------------------------------------------------------------------*/
> Index: xfs/fs/xfs/linux-2.6/xfs_sync.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_sync.c      2009-06-04 13:01:29.388941822 
> +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_sync.c   2009-06-04 13:02:26.249965001 +0200
> @@ -141,7 +141,7 @@ restart:
>       return last_error;
>  }
>  
> -STATIC int
> +int
>  xfs_inode_ag_iterator(
>       struct xfs_mount        *mp,
>       int                     (*execute)(struct xfs_inode *ip,
> @@ -167,7 +167,7 @@ xfs_inode_ag_iterator(
>  }
>  
>  /* must be called with pag_ici_lock held and releases it */
> -STATIC int
> +int
>  xfs_sync_inode_valid(
>       struct xfs_inode        *ip,
>       struct xfs_perag        *pag)
> Index: xfs/fs/xfs/linux-2.6/xfs_sync.h
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_sync.h      2009-06-04 12:47:30.804939977 
> +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_sync.h   2009-06-04 13:02:26.249965001 +0200
> @@ -54,4 +54,10 @@ void xfs_inode_set_reclaim_tag(struct xf
>  void xfs_inode_clear_reclaim_tag(struct xfs_inode *ip);
>  void __xfs_inode_clear_reclaim_tag(struct xfs_mount *mp, struct xfs_perag 
> *pag,
>                               struct xfs_inode *ip);
> +
> +int xfs_sync_inode_valid(struct xfs_inode *ip, struct xfs_perag *pag);
> +int xfs_inode_ag_iterator(struct xfs_mount *mp,
> +     int (*execute)(struct xfs_inode *ip, struct xfs_perag *pag, int flags),
> +     int flags, int tag);
> +
>  #endif
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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