Re: [PATCH] Btrfs: fix orphan cleanup regression

[Simon Kirby <sim@hostway.ca>]

On Wed, Sep 21, 2011 at 02:40:06PM -0700, Simon Kirby wrote:

> On Wed, Sep 21, 2011 at 04:57:56PM -0400, Josef Bacik wrote:
> 
> > In fixing how we deal with bad inodes, we had a regression in the orphan cleanup
> > code, since it expects to get a bad inode back.  So fix it to deal with getting
> > -ESTALE back by deleting the orphan item manually and moving on.  Thanks,
> > 
> > Reported-by: Simon Kirby <sim@hostway.ca>
> > Signed-off-by: Josef Bacik <josef@redhat.com>
> 
> Seems to fix the problem, and looks good. Thanks!
> 
> Simon-
> 
> Tested-by: Simon Kirby <sim@hostway.ca>

By the way, this doesn't seem to have been merged yet.

Simon-

> > ---
> >  fs/btrfs/inode.c |   36 +++++++++++++++++-------------------
> >  1 files changed, 17 insertions(+), 19 deletions(-)
> > 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index b128fa0..d8bd665 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -2285,37 +2285,35 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
> >  		found_key.type = BTRFS_INODE_ITEM_KEY;
> >  		found_key.offset = 0;
> >  		inode = btrfs_iget(root->fs_info->sb, &found_key, root, NULL);
> > -		if (IS_ERR(inode)) {
> > -			ret = PTR_ERR(inode);
> > +		ret = PTR_RET(inode);
> > +		if (ret && ret != -ESTALE)
> >  			goto out;
> > -		}
> > -
> > -		/*
> > -		 * add this inode to the orphan list so btrfs_orphan_del does
> > -		 * the proper thing when we hit it
> > -		 */
> > -		spin_lock(&root->orphan_lock);
> > -		list_add(&BTRFS_I(inode)->i_orphan, &root->orphan_list);
> > -		spin_unlock(&root->orphan_lock);
> >  
> >  		/*
> > -		 * if this is a bad inode, means we actually succeeded in
> > -		 * removing the inode, but not the orphan record, which means
> > -		 * we need to manually delete the orphan since iput will just
> > -		 * do a destroy_inode
> > +		 * Inode is already gone but the orphan item is still there,
> > +		 * kill the orphan item.
> >  		 */
> > -		if (is_bad_inode(inode)) {
> > -			trans = btrfs_start_transaction(root, 0);
> > +		if (ret == -ESTALE) {
> > +			trans = btrfs_start_transaction(root, 1);
> >  			if (IS_ERR(trans)) {
> >  				ret = PTR_ERR(trans);
> >  				goto out;
> >  			}
> > -			btrfs_orphan_del(trans, inode);
> > +			ret = btrfs_del_orphan_item(trans, root,
> > +						    found_key.objectid);
> > +			BUG_ON(ret);
> >  			btrfs_end_transaction(trans, root);
> > -			iput(inode);
> >  			continue;
> >  		}
> >  
> > +		/*
> > +		 * add this inode to the orphan list so btrfs_orphan_del does
> > +		 * the proper thing when we hit it
> > +		 */
> > +		spin_lock(&root->orphan_lock);
> > +		list_add(&BTRFS_I(inode)->i_orphan, &root->orphan_list);
> > +		spin_unlock(&root->orphan_lock);
> > +
> >  		/* if we have links, this was a truncate, lets do that */
> >  		if (inode->i_nlink) {
> >  			if (!S_ISREG(inode->i_mode)) {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html