* [PATCH v4] ocfs2: cache the mount context for dentry lock callbacks
@ 2026-10-20 14:25 Cen Zhang
2026-10-23 7:09 ` Joseph Qi
0 siblings, 1 reply; 3+ messages in thread
From: Cen Zhang @ 2026-10-20 14:25 UTC (permalink / raw)
To: mark, jlbec, joseph.qi
Cc: heming.zhao, ocfs2-devel, linux-kernel, stable, baijiaju1990,
jjzuming, zzzccc427
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 can proceed 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 a freed inode when the downconvert thread drops the
last inode reference in its post-unlock callback:
Downconvert thread DLM callback thread
------------------ -------------------
ocfs2_process_blocked_lock()
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 l_priv field and
use the default mount-context lookup. Recover the containing dentry
lock with container_of(), as already done for refcount locks, and remove
ocfs2_get_dentry_osb(). ocfs2_dismount_volume() shuts down the DLM before
freeing the mount context, so callbacks can use it without accessing the
inode. Preserve the existing iput-before-unlock order to avoid delaying
local inode and open lock release until after the DENTRY lock is released.
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 v4:
- Drop the convert-worker change from v3. Keep MAYBE_ORPHANED marking before
the extra-reference check, including the dl_count == 0 path needed for
local orphan cleanup.
- Retain the osb cache from v2 and the original iput-before-unlock order;
remove the incorrect worker-lifetime claims from the commit message.
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
Link to v3: https://lore.kernel.org/r/pm-ocfs2-objects-candidate-0012-v8-d01e30cb3f465e294c0a@gmail.com
fs/ocfs2/dcache.h | 5 ++---
fs/ocfs2/dlmglue.c | 16 ++++------------
2 files changed, 6 insertions(+), 15 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..437a726834c8 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,
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v4] ocfs2: cache the mount context for dentry lock callbacks
2026-10-20 14:25 [PATCH v4] ocfs2: cache the mount context for dentry lock callbacks Cen Zhang
@ 2026-10-23 7:09 ` Joseph Qi
2026-10-24 15:38 ` Cen Zhang
0 siblings, 1 reply; 3+ messages in thread
From: Joseph Qi @ 2026-10-23 7:09 UTC (permalink / raw)
To: Cen Zhang
Cc: mark, jlbec, heming.zhao, ocfs2-devel, linux-kernel, stable,
baijiaju1990, jjzuming
On 10/20/26 10:25 PM, Cen Zhang wrote:
> 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 can proceed 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 a freed inode when the downconvert thread drops the
> last inode reference in its post-unlock callback:
>
> Downconvert thread DLM callback thread
> ------------------ -------------------
> ocfs2_process_blocked_lock()
> 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 l_priv field and
> use the default mount-context lookup. Recover the containing dentry
> lock with container_of(), as already done for refcount locks, and remove
> ocfs2_get_dentry_osb(). ocfs2_dismount_volume() shuts down the DLM before
> freeing the mount context, so callbacks can use it without accessing the
> inode. Preserve the existing iput-before-unlock order to avoid delaying
> local inode and open lock release until after the DENTRY lock is released.
>
> 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 v4:
> - Drop the convert-worker change from v3. Keep MAYBE_ORPHANED marking before
> the extra-reference check, including the dl_count == 0 path needed for
> local orphan cleanup.
> - Retain the osb cache from v2 and the original iput-before-unlock order;
> remove the incorrect worker-lifetime claims from the commit message.
>
> 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
> Link to v3: https://lore.kernel.org/r/pm-ocfs2-objects-candidate-0012-v8-d01e30cb3f465e294c0a@gmail.com
>
> fs/ocfs2/dcache.h | 5 ++---
> fs/ocfs2/dlmglue.c | 16 ++++------------
> 2 files changed, 6 insertions(+), 15 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..437a726834c8 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,
Don't see any difference between your v2 sent before.
BTW, it seems v3 is also missing.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v4] ocfs2: cache the mount context for dentry lock callbacks
2026-10-23 7:09 ` Joseph Qi
@ 2026-10-24 15:38 ` Cen Zhang
0 siblings, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-10-24 15:38 UTC (permalink / raw)
To: Joseph Qi
Cc: mark, jlbec, heming.zhao, ocfs2-devel, linux-kernel, stable,
baijiaju1990, jjzuming
Hi Joseph,
Joseph Qi <joseph.qi@linux.alibaba.com> 于2026年10月23日周五 15:09写道:
>
> Don't see any difference between your v2 sent before.
> BTW, it seems v3 is also missing.
>
> Thanks,
> Joseph
>
Sorry for the confusion. I rechecked the early inode accesses you
pointed out in your v2 review:
https://lore.kernel.org/ocfs2-devel/018d8feb-2e63-4a73-ba36-10a5f94f3341@linux.alibaba.com/
My understanding is that these accesses have the following lifetime
protection:
- ocfs2_dentry_iput() keeps the VFS inode reference until after
ocfs2_dentry_lock_put() returns. Lock teardown waits for QUEUED to
clear if the worker is already running. The rename path likewise
keeps the source dentry/inode alive while dropping the old dentry
lock.
- A final put from ocfs2_dentry_post_unlock() runs synchronously on
the same downconvert thread as the worker. Recursive processing
from ocfs2_mark_lockres_freeing() sets FREEING before entering
ocfs2_process_blocked_lock(), so the worker is skipped.
Thus the early access does not rely on the later extra-reference
check. A zero dl_count can still coexist with a live VFS inode
reference, and MAYBE_ORPHANED still needs to be set on that path for
local orphan cleanup.
The asynchronous DLM callbacks can overlap the final post-unlock
iput(), which is the separate lifetime window addressed by caching
osb.
I incorrectly moved the orphan marking past the extra-reference
check in v3. I reverted that change in v4, whose code is identical
to v2.
Does this analysis miss any path that could drop the last inode
reference while the worker can still access it?
Best regards,
Cen Zhang
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-24 15:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-20 14:25 [PATCH v4] ocfs2: cache the mount context for dentry lock callbacks Cen Zhang
2026-10-23 7:09 ` Joseph Qi
2026-10-24 15:38 ` Cen Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®