All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <jbacik@fusionio.com>
To: Miao Xie <miaox@cn.fujitsu.com>
Cc: Josef Bacik <JBacik@fusionio.com>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH] Btrfs: don't bother updating the inode when evicting
Date: Wed, 19 Dec 2012 10:02:59 -0500	[thread overview]
Message-ID: <20121219150259.GB4033@localhost.localdomain> (raw)
In-Reply-To: <50D11F49.4070005@cn.fujitsu.com>

On Tue, Dec 18, 2012 at 06:58:33PM -0700, Miao Xie wrote:
> On tue, 18 Dec 2012 15:51:57 -0500, Josef Bacik wrote:
> > We're deleting the stupid thing, no sense in updating the inode for the new
> > size.  We're running into having 50-100 orphans left over with xfstests 83
> > because of ENOSPC when trying to start the transaction for the inode update.
> > This patch fixes this problem.  Thanks,
> 
> This patch is wrong, it will introduce the inconsonant metadata in the snapshot
> tree. The reason is folloing:
> 
> commit 8407aa464331556e4f6784f974030b83fc7585ed
> Author: Miao Xie <miaox@cn.fujitsu.com>
> Date:   Fri Sep 7 01:43:32 2012 -0600
> 
>     Btrfs: fix corrupted metadata in the snapshot
>     
>     When we delete a inode, we will remove all the delayed items including delayed
>     inode update, and then truncate all the relative metadata. If there is lots of
>     metadata, we will end the current transaction, and start a new transaction to
>     truncate the left metadata. In this way, we will leave a inode item that its
>     link counter is > 0, and also may leave some directory index items in fs/file tree
>     after the current transaction ends. In other words, the metadata in this fs/file tree
>     is inconsistent. If we create a snapshot for this tree now, we will find a inode with
>     corrupted metadata in the new snapshot, and we won't continue to drop the left metadata,
>     because its link counter is not 0.
>     
>     We fix this problem by updating the inode item before the current transaction ends.
>     
>     Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> 

So why don't we fix unlink to call btrfs_update_inode_item so that the nlink
counter is set to 0?  The orphan item will be carried over into the snapshot if
we don't actually evict the inode before we do the snapshot and then the orphan
cleanup will take care of the rest?  Thanks,

Josef

  reply	other threads:[~2012-12-19 15:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-18 20:51 [PATCH] Btrfs: don't bother updating the inode when evicting Josef Bacik
2012-12-19  1:58 ` Miao Xie
2012-12-19 15:02   ` Josef Bacik [this message]
2012-12-20  3:04     ` Miao Xie
2012-12-19  6:59 ` [PATCH 1/2] Btrfs: cleanup similar code in delayed inode Miao Xie
2012-12-19  6:59 ` [PATCH 2/2] Btrfs: fix lots of orphan inodes when the space is not enough Miao Xie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121219150259.GB4033@localhost.localdomain \
    --to=jbacik@fusionio.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=miaox@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.