xfs
[Top] [All Lists]

RE: [PATCH V2] xfs: check for not fully initialized inodes in xfs_irecla

To: "Christoph Hellwig" <hch@xxxxxxxxxxxxx>
Subject: RE: [PATCH V2] xfs: check for not fully initialized inodes in xfs_ireclaim
From: "Alex Elder" <aelder@xxxxxxx>
Date: Tue, 15 Dec 2009 14:05:15 -0600
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <20091201181229.GA11982@xxxxxxxxxxxxx>
Thread-index: AcpytE5DPV0xlN2NSdqDWor2AM+SCgLDYQaA
Thread-topic: [PATCH V2] xfs: check for not fully initialized inodes in xfs_ireclaim
Christoph Hellwig wrote:
> Add an assert for inodes not added to the inode cache in xfs_ireclaim, to make
> sure we're not going to introduce something like the famous nfsd inode cache
> bug again.

It's a shame we can't do more than an ASSERT() here
(for non-debug builds).

Looks good though.

> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Reviewed-by: Alex Elder <aelder@xxxxxxx>

> Index: xfs/fs/xfs/xfs_iget.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_iget.c        2009-11-26 19:10:47.206253628 +0100
> +++ xfs/fs/xfs/xfs_iget.c     2009-12-01 19:09:43.310004091 +0100
> @@ -514,17 +514,21 @@ xfs_ireclaim(
>  {
>       struct xfs_mount        *mp = ip->i_mount;
>       struct xfs_perag        *pag;
> +     xfs_agino_t             agino = XFS_INO_TO_AGINO(mp, ip->i_ino);
> 
>       XFS_STATS_INC(xs_ig_reclaims);
> 
>       /*
> -      * Remove the inode from the per-AG radix tree.  It doesn't matter
> -      * if it was never added to it because radix_tree_delete can deal
> -      * with that case just fine.
> +      * Remove the inode from the per-AG radix tree.
> +      *
> +      * Because radix_tree_delete won't complain even if the item was never
> +      * added to the tree assert that it's been there before to catch
> +      * problems with the inode life time early on.
>        */
>       pag = xfs_get_perag(mp, ip->i_ino);
>       write_lock(&pag->pag_ici_lock);
> -     radix_tree_delete(&pag->pag_ici_root, XFS_INO_TO_AGINO(mp, ip->i_ino));
> +     if (!radix_tree_delete(&pag->pag_ici_root, agino))
> +             ASSERT(0);
>       write_unlock(&pag->pag_ici_lock);
>       xfs_put_perag(mp, pag);
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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