On 18/4/12 03:31, Ashish Samant wrote: > While reflinking an inode, we create a new inode in orphan directory, then > take EX lock on it, reflink the original inode to orphan inode and release > EX lock. Once the lock is released another node could request it in EX mode > from ocfs2_recover_orphans() which causes downconvert of the lock, on this > node, to NL mode. > > Later we attempt to initialize security acl for the orphan inode and move > it to the reflink destination. However, while doing this we dont take EX > lock on the inode. This could potentially cause problems because we could > be starting transaction, accessing journal and modifying metadata of the > inode while holding NL lock and with another node holding EX lock on the > inode. > > Fix this by taking orphan inode cluster lock in EX mode before > initializing security and moving orphan inode to reflink destination. > Use the __tracker variant while taking inode lock to avoid recursive > locking in the ocfs2_init_security_and_acl() call chain. > > Signed-off-by: Ashish Samant <ashish.sam...@oracle.com> > Reviewed-by: Joseph Qi <jiangqi...@gmail.com>
> V1->V2: > Modify commit message to better reflect the problem in upstream kernel. > --- > fs/ocfs2/refcounttree.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c > index ab156e3..1b1283f 100644 > --- a/fs/ocfs2/refcounttree.c > +++ b/fs/ocfs2/refcounttree.c > @@ -4250,10 +4250,11 @@ static int __ocfs2_reflink(struct dentry *old_dentry, > static int ocfs2_reflink(struct dentry *old_dentry, struct inode *dir, > struct dentry *new_dentry, bool preserve) > { > - int error; > + int error, had_lock; > struct inode *inode = d_inode(old_dentry); > struct buffer_head *old_bh = NULL; > struct inode *new_orphan_inode = NULL; > + struct ocfs2_lock_holder oh; > > if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb))) > return -EOPNOTSUPP; > @@ -4295,6 +4296,14 @@ static int ocfs2_reflink(struct dentry *old_dentry, > struct inode *dir, > goto out; > } > > + had_lock = ocfs2_inode_lock_tracker(new_orphan_inode, NULL, 1, > + &oh); > + if (had_lock < 0) { > + error = had_lock; > + mlog_errno(error); > + goto out; > + } > + > /* If the security isn't preserved, we need to re-initialize them. */ > if (!preserve) { > error = ocfs2_init_security_and_acl(dir, new_orphan_inode, > @@ -4302,14 +4311,15 @@ static int ocfs2_reflink(struct dentry *old_dentry, > struct inode *dir, > if (error) > mlog_errno(error); > } > -out: > if (!error) { > error = ocfs2_mv_orphaned_inode_to_new(dir, new_orphan_inode, > new_dentry); > if (error) > mlog_errno(error); > } > + ocfs2_inode_unlock_tracker(new_orphan_inode, 1, &oh, had_lock); > > +out: > if (new_orphan_inode) { > /* > * We need to open_unlock the inode no matter whether we > _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel