[PATCH v3] ocfs2: protect dentry lock callbacks during inode teardown

[PATCH v3] ocfs2: protect dentry lock callbacks during inode teardown

[Cen Zhang]

ocfs2_drop_dentry_lock() drops the inode reference before releasing
the DENTRY lock. This lets inode eviction release the local inode
and open locks before a remote unlink proceeds past the DENTRY lock.

A DLM callback can still arrive before the DENTRY lock has been
dropped. ocfs2_get_dentry_osb() obtains the mount context through
dl_inode->i_sb, which can access an inode already released by the
downconvert thread in its post-unlock callback:

  Downconvert thread                   DLM callback thread
  ------------------                   -------------------
  ocfs2_dentry_post_unlock()
    ocfs2_dentry_lock_put()
      ocfs2_drop_dentry_lock()
        iput(dl->dl_inode)
        [inode reclaimed after RCU]
                                       ocfs2_blocking_ast()
                                         ocfs2_get_dentry_osb()
                                           dl->dl_inode->i_sb
        ocfs2_simple_drop_lockres()

Store the mount context in the DENTRY lock resource's existing
l_priv field and use the default mount-context lookup. Recover the
containing dentry lock with container_of(), as for refcount locks.
The mount context remains valid until DLM shutdown completes.

The dentry convert worker also accesses dl_inode before checking
OCFS2_LOCK_FREEING and dl_count. Its early MAYBE_ORPHANED update
therefore precedes the reference that is meant to protect the inode.
ocfs2_unblock_lock() can invoke the worker without a FREEING test.

Move both the inode lookup and the flag update after the existing
guard has acquired an extra dentry lock reference. The check and
increment are serialized with final put by dentry_attach_lock, and
with FREEING by l_lock. A zero count also stops inode access in the
window before teardown sets FREEING. Once the extra reference is
held, final put cannot release dl_inode until the worker's existing
post-unlock path drops it.

Preserve iput-before-unlock ordering, normal orphan marking and the
existing extra-reference/post-unlock pairing.

Fixes: 8ed6b23709b3 ("ocfs2: revert iput deferring code in ocfs2_drop_dentry_lock")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

---
Changes in v3:
- Move the inode lookup and MAYBE_ORPHANED update after the existing
  FREEING/dl_count guard and successful extra-reference acquisition,
  addressing Joseph Qi's review.
- Keep the osb cache from v2 and preserve iput-before-unlock ordering.

Changes in v2:
- Preserve the existing iput-before-unlock order and cache the mount context
  in the lock resource's existing l_priv field instead.
- Recover the containing dentry lock with container_of() and remove the
  inode-dependent mount-context lookup.
- Correct the comment describing the dentry lock's inode reference.

Link to v1: https://lore.kernel.org/r/pm-ocfs2-objects-candidate-0012-v5-a0413a339b72c02fc885@gmail.com
Link to v2: https://lore.kernel.org/r/pm-ocfs2-objects-candidate-0012-v6-17b58652a3505eeecb0c@gmail.com

 fs/ocfs2/dcache.h  |  5 ++---
 fs/ocfs2/dlmglue.c | 38 ++++++++++++++++----------------------
 2 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/fs/ocfs2/dcache.h b/fs/ocfs2/dcache.h
index 7f246c5692d8..046ab9a80769 100644
--- a/fs/ocfs2/dcache.h
+++ b/fs/ocfs2/dcache.h
@@ -17,9 +17,8 @@ struct ocfs2_dentry_lock {
 	u64			dl_parent_blkno;
 
 	/*
-	 * The ocfs2_dentry_lock keeps an inode reference until
-	 * dl_lockres has been destroyed. This is usually done in
-	 * ->d_iput() anyway, so there should be minimal impact.
+	 * The inode reference is dropped before dl_lockres is unlocked.
+	 * Callbacks must not use it to find the mount context during teardown.
 	 */
 	struct inode		*dl_inode;
 	struct ocfs2_lock_res	dl_lockres;
diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 88d946466083..b0b649b0d471 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -58,7 +58,6 @@ struct ocfs2_mask_waiter {
 #endif
 };
 
-static struct ocfs2_super *ocfs2_get_dentry_osb(struct ocfs2_lock_res *lockres);
 static struct ocfs2_super *ocfs2_get_inode_osb(struct ocfs2_lock_res *lockres);
 static struct ocfs2_super *ocfs2_get_file_osb(struct ocfs2_lock_res *lockres);
 static struct ocfs2_super *ocfs2_get_qinfo_osb(struct ocfs2_lock_res *lockres);
@@ -258,7 +257,6 @@ static const struct ocfs2_lock_res_ops ocfs2_orphan_scan_lops = {
 };
 
 static const struct ocfs2_lock_res_ops ocfs2_dentry_lops = {
-	.get_osb	= ocfs2_get_dentry_osb,
 	.post_unlock	= ocfs2_dentry_post_unlock,
 	.downconvert_worker = ocfs2_dentry_convert_worker,
 	.flags		= 0,
@@ -309,7 +307,7 @@ static inline struct ocfs2_dentry_lock *ocfs2_lock_res_dl(struct ocfs2_lock_res
 {
 	BUG_ON(lockres->l_type != OCFS2_LOCK_TYPE_DENTRY);
 
-	return (struct ocfs2_dentry_lock *)lockres->l_priv;
+	return container_of(lockres, struct ocfs2_dentry_lock, dl_lockres);
 }
 
 static inline struct ocfs2_mem_dqinfo *ocfs2_lock_res_qinfo(struct ocfs2_lock_res *lockres)
@@ -610,13 +608,6 @@ static __u64 ocfs2_get_dentry_lock_ino(struct ocfs2_lock_res *lockres)
 	return be64_to_cpu(inode_blkno_be);
 }
 
-static struct ocfs2_super *ocfs2_get_dentry_osb(struct ocfs2_lock_res *lockres)
-{
-	struct ocfs2_dentry_lock *dl = lockres->l_priv;
-
-	return OCFS2_SB(dl->dl_inode->i_sb);
-}
-
 void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,
 				u64 parent, struct inode *inode)
 {
@@ -624,6 +615,7 @@ void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,
 	u64 inode_blkno = OCFS2_I(inode)->ip_blkno;
 	__be64 inode_blkno_be = cpu_to_be64(inode_blkno);
 	struct ocfs2_lock_res *lockres = &dl->dl_lockres;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 
 	ocfs2_lock_res_init_once(lockres);
 
@@ -652,9 +644,9 @@ void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,
 	memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START], &inode_blkno_be,
 	       sizeof(__be64));
 
-	ocfs2_lock_res_init_common(OCFS2_SB(inode->i_sb), lockres,
+	ocfs2_lock_res_init_common(osb, lockres,
 				   OCFS2_LOCK_TYPE_DENTRY, &ocfs2_dentry_lops,
-				   dl);
+				   osb);
 }
 
 static void ocfs2_super_lock_res_init(struct ocfs2_lock_res *res,
@@ -4099,7 +4091,7 @@ static int ocfs2_dentry_convert_worker(struct ocfs2_lock_res *lockres,
 				       int blocking)
 {
 	struct ocfs2_dentry_lock *dl = ocfs2_lock_res_dl(lockres);
-	struct ocfs2_inode_info *oi = OCFS2_I(dl->dl_inode);
+	struct ocfs2_inode_info *oi;
 	struct dentry *dentry;
 	unsigned long flags;
 	int extra_ref = 0;
@@ -4115,15 +4107,6 @@ static int ocfs2_dentry_convert_worker(struct ocfs2_lock_res *lockres,
 	if (blocking == DLM_LOCK_PR)
 		return UNBLOCK_CONTINUE;
 
-	/*
-	 * Mark this inode as potentially orphaned. The code in
-	 * ocfs2_delete_inode() will figure out whether it actually
-	 * needs to be freed or not.
-	 */
-	spin_lock(&oi->ip_lock);
-	oi->ip_flags |= OCFS2_INODE_MAYBE_ORPHANED;
-	spin_unlock(&oi->ip_lock);
-
 	/*
 	 * Yuck. We need to make sure however that the check of
 	 * OCFS2_LOCK_FREEING and the extra reference are atomic with
@@ -4150,6 +4133,17 @@ static int ocfs2_dentry_convert_worker(struct ocfs2_lock_res *lockres,
 	if (!extra_ref)
 		return UNBLOCK_CONTINUE;
 
+	/* The extra dentry lock reference keeps dl_inode alive. */
+	oi = OCFS2_I(dl->dl_inode);
+	/*
+	 * Mark this inode as potentially orphaned. The code in
+	 * ocfs2_delete_inode() will figure out whether it actually
+	 * needs to be freed or not.
+	 */
+	spin_lock(&oi->ip_lock);
+	oi->ip_flags |= OCFS2_INODE_MAYBE_ORPHANED;
+	spin_unlock(&oi->ip_lock);
+
 	spin_lock(&dentry_attach_lock);
 	while (1) {
 		dentry = ocfs2_find_local_alias(dl->dl_inode,