From mboxrd@z Thu Jan 1 00:00:00 1970 From: tristan Date: Sun, 21 Feb 2010 16:58:14 +0800 Subject: [Ocfs2-devel] [PATCH 2/2] Ocfs2: Handle deletion of refcounted oprhan_inode correctly. In-Reply-To: <4B80F4F4.8020604@oracle.com> References: <1266740977-8638-1-git-send-email-tristan.ye@oracle.com> <1266740977-8638-2-git-send-email-tristan.ye@oracle.com> <4B80F4F4.8020604@oracle.com> Message-ID: <4B80F5A6.3070600@oracle.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ocfs2-devel@oss.oracle.com Tao Ma wrote: > Hi tristan, > > Tristan Ye wrote: >> Current ocfs2 semantic for reflinking a file firstly create a >> new orphan_inode in orphan_dir, then remove it to target dir >> after refcounting operation done, these 2 steps makes logic >> straightfoward, and guarantee a crash during reflinking can >> be replayed(half-refcounted inode can be removed), while it >> brings us another issue cause these 2 steps is acquiring the >> orphan_dir lock respectively, the problem is, orphan_scan() >> may detect the half-refcounted inode in orphan_dir as its >> proper candidates to wipe off in a later time. actually it's >> not of course, we'd handle this correctly. >> >> Signed-off-by: Tristan Ye >> --- >> fs/ocfs2/inode.c | 32 ++++++++++++++++++++++++-------- >> 1 files changed, 24 insertions(+), 8 deletions(-) >> >> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c >> index 88459bd..61fb546 100644 >> --- a/fs/ocfs2/inode.c >> +++ b/fs/ocfs2/inode.c >> @@ -892,14 +892,30 @@ static int ocfs2_query_inode_wipe(struct inode >> *inode, >> di = (struct ocfs2_dinode *) di_bh->b_data; >> if (!(di->i_flags & cpu_to_le32(OCFS2_ORPHANED_FL))) { >> /* for lack of a better error? */ >> - status = -EEXIST; >> - mlog(ML_ERROR, >> - "Inode %llu (on-disk %llu) not orphaned! " >> - "Disk flags 0x%x, inode flags 0x%x\n", >> - (unsigned long long)oi->ip_blkno, >> - (unsigned long long)le64_to_cpu(di->i_blkno), >> - le32_to_cpu(di->i_flags), oi->ip_flags); >> - goto bail; >> + if (!(di->i_dyn_features & >> + cpu_to_le16(OCFS2_HAS_REFCOUNT_FL))) { >> + status = -EEXIST; >> + mlog(ML_ERROR, >> + "Inode %llu (on-disk %llu) not orphaned! " >> + "Disk flags 0x%x, inode flags 0x%x\n", >> + (unsigned long long)oi->ip_blkno, >> + (unsigned long long)le64_to_cpu(di->i_blkno), >> + le32_to_cpu(di->i_flags), oi->ip_flags); >> + goto bail; >> + } else { >> + /* >> + * It did happen to us, though it's a rare case: >> + * orphan_scan() detects the half-refcounted inode >> + * in orphan_dir, and delete_inode() attempts to >> + * wipe it after reflink operation done later. now >> + * we're not allowed to delete such a valid inode, >> + * instead, just bail out. >> + */ >> + mlog(0, "Skipping delete of %llu because it's a " >> + "reflinked inode\n", >> + (unsigned long long)oi->ip_blkno); >> + goto bail; >> + } > We set i_dyn_features when when attach the tree to the file. This is > very early. So I am curious why i_dyn_features can tell you whether > this isn't a crashed reflink inode? Oh, you mean you will never delete > a reflinked inode in orphan scan? Tao, Not exactly, if it's reflink operation was crashed somehow, it's OCFS2_ORPHANED_FL must be set:), and if it was the case, we then will never skip the deletion which is really needed. Regards, Tristan. > > Regards, > Tao