* [PATCH v3 1/3] ceph/mds_client: pin sessions while checking a new MDS map
2026-08-28 17:45 [PATCH v3 0/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
@ 2026-08-28 17:45 ` Max Kellermann
2026-08-28 17:45 ` [PATCH v3 2/3] ceph/mds_client: stop checking a stale MDS map after dropping mutex Max Kellermann
2026-08-28 17:45 ` [PATCH v3 3/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
2 siblings, 0 replies; 4+ messages in thread
From: Max Kellermann @ 2026-08-28 17:45 UTC (permalink / raw)
To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel; +Cc: Max Kellermann
check_new_map() drops `mdsc->mutex` while it locks a session, prepares
a reconnect, or cleans up caps. A concurrent teardown can unregister
and release a session, invalidating check_new_map()'s local variable
`s`.
Fix this by taking a temporary session reference using
__ceph_lookup_mds_session() instead of accessing the `sessions[]`
array directly.
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
fs/ceph/mds_client.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 3c692ad02c85..160f23e2edd3 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -5798,9 +5798,9 @@ static void check_new_map(struct ceph_mds_client *mdsc,
}
for (i = 0; i < oldmap->possible_max_rank && i < mdsc->max_sessions; i++) {
- if (!mdsc->sessions[i])
+ s = __ceph_lookup_mds_session(mdsc, i);
+ if (!s)
continue;
- s = mdsc->sessions[i];
oldstate = ceph_mdsmap_get_state(oldmap, i);
newstate = ceph_mdsmap_get_state(newmap, i);
@@ -5813,7 +5813,6 @@ static void check_new_map(struct ceph_mds_client *mdsc,
if (i >= newmap->possible_max_rank) {
/* force close session for stopped mds */
- ceph_get_mds_session(s);
__unregister_session(mdsc, s);
__wake_requests(mdsc, &s->s_waiting);
mutex_unlock(&mdsc->mutex);
@@ -5841,6 +5840,7 @@ static void check_new_map(struct ceph_mds_client *mdsc,
mutex_unlock(&s->s_mutex);
s->s_state = CEPH_MDS_SESSION_RESTARTING;
} else if (oldstate == newstate) {
+ ceph_put_mds_session(s);
continue; /* nothing new with this mds */
}
@@ -5878,6 +5878,7 @@ static void check_new_map(struct ceph_mds_client *mdsc,
mutex_unlock(&s->s_mutex);
wake_up_session_caps(s, RECONNECT);
}
+ ceph_put_mds_session(s);
}
/*
--
2.47.3
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v3 2/3] ceph/mds_client: stop checking a stale MDS map after dropping mutex
2026-08-28 17:45 [PATCH v3 0/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
2026-08-28 17:45 ` [PATCH v3 1/3] ceph/mds_client: pin sessions while checking a new MDS map Max Kellermann
@ 2026-08-28 17:45 ` Max Kellermann
2026-08-28 17:45 ` [PATCH v3 3/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
2 siblings, 0 replies; 4+ messages in thread
From: Max Kellermann @ 2026-08-28 17:45 UTC (permalink / raw)
To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel; +Cc: Max Kellermann
check_new_map() drops `mdsc->mutex` in several slow paths. Another
map handler can install a newer map and destroy the map being checked
before the original invocation re-locks the mutex.
Use the `mdsmap->m_epoch` field to identify stale maps.
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
fs/ceph/mds_client.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 160f23e2edd3..03809328e4aa 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -5782,6 +5782,7 @@ static void check_new_map(struct ceph_mds_client *mdsc,
struct ceph_mdsmap *newmap,
struct ceph_mdsmap *oldmap)
{
+ u32 map_epoch = newmap->m_epoch;
int i, j, err;
int oldstate, newstate;
struct ceph_mds_session *s;
@@ -5825,6 +5826,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
ceph_put_mds_session(s);
mutex_lock(&mdsc->mutex);
+ if (mdsc->mdsmap->m_epoch != map_epoch)
+ return;
kick_requests(mdsc, i);
continue;
}
@@ -5836,6 +5839,11 @@ static void check_new_map(struct ceph_mds_client *mdsc,
mutex_unlock(&mdsc->mutex);
mutex_lock(&s->s_mutex);
mutex_lock(&mdsc->mutex);
+ if (mdsc->mdsmap->m_epoch != map_epoch) {
+ mutex_unlock(&s->s_mutex);
+ ceph_put_mds_session(s);
+ return;
+ }
ceph_con_close(&s->s_con);
mutex_unlock(&s->s_mutex);
s->s_state = CEPH_MDS_SESSION_RESTARTING;
@@ -5859,6 +5867,10 @@ static void check_new_map(struct ceph_mds_client *mdsc,
"mds%d reconnect failed: %d\n",
i, rc);
mutex_lock(&mdsc->mutex);
+ if (mdsc->mdsmap->m_epoch != map_epoch) {
+ ceph_put_mds_session(s);
+ return;
+ }
}
/*
@@ -5874,6 +5886,11 @@ static void check_new_map(struct ceph_mds_client *mdsc,
mutex_unlock(&mdsc->mutex);
mutex_lock(&s->s_mutex);
mutex_lock(&mdsc->mutex);
+ if (mdsc->mdsmap->m_epoch != map_epoch) {
+ mutex_unlock(&s->s_mutex);
+ ceph_put_mds_session(s);
+ return;
+ }
ceph_kick_flushing_caps(mdsc, s);
mutex_unlock(&s->s_mutex);
wake_up_session_caps(s, RECONNECT);
@@ -5929,6 +5946,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
i, err);
ceph_put_mds_session(s);
mutex_lock(&mdsc->mutex);
+ if (mdsc->mdsmap->m_epoch != map_epoch)
+ return;
}
for (i = 0; i < newmap->possible_max_rank && i < mdsc->max_sessions; i++) {
--
2.47.3
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v3 3/3] ceph: don't unregister an MDS session before removing its caps
2026-08-28 17:45 [PATCH v3 0/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
2026-08-28 17:45 ` [PATCH v3 1/3] ceph/mds_client: pin sessions while checking a new MDS map Max Kellermann
2026-08-28 17:45 ` [PATCH v3 2/3] ceph/mds_client: stop checking a stale MDS map after dropping mutex Max Kellermann
@ 2026-08-28 17:45 ` Max Kellermann
2 siblings, 0 replies; 4+ messages in thread
From: Max Kellermann @ 2026-08-28 17:45 UTC (permalink / raw)
To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel; +Cc: Max Kellermann
handle_session() removed the session from mdsc->sessions[] at the very
top of `CEPH_SESSION_CLOSE` handling, before taking `s_mutex`.
Between session unregistration and remove_session_caps(), the MDS rank
has no registered session while the old session still owns all caps it
was granted.
Any concurrent filesystem operation may walk into that and the next
__do_request() call registers a new session for this rank. Once it is
open and the MDS issues caps, ceph_fill_inode() calls
ceph_add_cap(), which looks caps up by rank, not
by session identity, finding old caps linked to the old session.
The list_move_tail() call then moves the cap object to the new
session, which is already a bad thing to do. Since it doesn't
decrement `old_session->s_nr_caps`, this will quickly run into a BUG()
instead of crashing:
kernel BUG at fs/ceph/mds_client.c:1959!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
[...]
Workqueue: ceph-msgr ceph_con_workfn
pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : remove_session_caps+0x2bc/0x2d8
lr : remove_session_caps+0x74/0x2d8
[...]
Call trace:
remove_session_caps+0x2bc/0x2d8 (P)
mds_dispatch+0xf48/0x1b60
ceph_con_process_message+0x74/0xa0
ceph_con_v1_try_read+0x3a0/0x1510
ceph_con_workfn+0x260/0x460
process_one_work+0x168/0x3b8
worker_thread+0x1bc/0x3a0
kthread+0x118/0x1e0
ret_from_fork+0x10/0x20
That's BUG_ON(session->s_nr_caps > 0).
I was able to reproduce this reliably by delaying the close and
starting I/O during the delay.
This patch keeps the session registered with
`CEPH_MDS_SESSION_CLOSED`. New requests will be put on the
`s_waiting` list where they will be resumed on the new session.
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
v1->v2: skip CLOSED sessions in check_new_map()
v2->v3: split session pinning and stale-map guards into two preparatory
patches; rework CLOSED-session and export-target teardown handling
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
fs/ceph/caps.c | 3 +
fs/ceph/mds_client.c | 138 ++++++++++++++++++++++++++++++++++++++-----
2 files changed, 127 insertions(+), 14 deletions(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index d7283fb54cec..b21d0a9ac324 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -4195,6 +4195,9 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex,
}
new_cap = ceph_get_cap(mdsc, NULL);
} else {
+ if (tsession == ERR_PTR(-EAGAIN))
+ return;
+
WARN_ON(1);
tsession = NULL;
target = -1;
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 03809328e4aa..ec8ee95cad6e 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1733,6 +1733,30 @@ static int __open_session(struct ceph_mds_client *mdsc,
return 0;
}
+/* Is this rank still occupied by a session being torn down? */
+static bool __mds_rank_closing(struct ceph_mds_client *mdsc, int mds)
+{
+ return mds < mdsc->max_sessions && mdsc->sessions[mds] &&
+ mdsc->sessions[mds]->s_state == CEPH_MDS_SESSION_CLOSED;
+}
+
+/*
+ * Wait until the rank is no longer occupied by a CLOSED session.
+ *
+ * The caller must hold mdsc->mutex. The mutex is dropped while sleeping and
+ * held again on return. Another session may occupy the rank by then. The
+ * wait also ends when shutdown starts, so the caller must check
+ * mdsc->stopping before proceeding.
+ */
+static void wait_for_mds_rank_not_closing(struct ceph_mds_client *mdsc,
+ int mds)
+{
+ wait_event_cmd(mdsc->session_close_wq,
+ !__mds_rank_closing(mdsc, mds) || mdsc->stopping,
+ mutex_unlock(&mdsc->mutex),
+ mutex_lock(&mdsc->mutex));
+}
+
/*
* open sessions for any export targets for the given mds
*
@@ -1750,6 +1774,16 @@ __open_export_target_session(struct ceph_mds_client *mdsc, int target)
if (IS_ERR(session))
return session;
}
+ if (session->s_state == CEPH_MDS_SESSION_CLOSED) {
+ /*
+ * handle_session() is currently closing this session;
+ * it stays registered until its caps are gone. Do
+ * not return it to our caller because we don't want
+ * it to attach new caps to it.
+ */
+ ceph_put_mds_session(session);
+ return ERR_PTR(-EAGAIN);
+ }
if (session->s_state == CEPH_MDS_SESSION_NEW ||
session->s_state == CEPH_MDS_SESSION_CLOSING) {
ret = __open_session(mdsc, session);
@@ -1769,7 +1803,19 @@ ceph_mdsc_open_export_target_session(struct ceph_mds_client *mdsc, int target)
doutc(cl, "to mds%d\n", target);
mutex_lock(&mdsc->mutex);
- session = __open_export_target_session(mdsc, target);
+ for (;;) {
+ session = __open_export_target_session(mdsc, target);
+ if (session != ERR_PTR(-EAGAIN) || mdsc->stopping)
+ break;
+
+ /*
+ * Keep the exported cap on its old session until the
+ * target rank is vacant. Dropping it here can discard a
+ * dirty auth cap; handle_session() wakes us after removing
+ * the old target session's caps and unregistering it.
+ */
+ wait_for_mds_rank_not_closing(mdsc, target);
+ }
mutex_unlock(&mdsc->mutex);
return session;
@@ -4492,8 +4538,20 @@ static void handle_session(struct ceph_mds_session *session,
ceph_metric_bind_session(mdsc, session);
}
if (op == CEPH_SESSION_CLOSE) {
+ /*
+ * Pin the session for the rest of this function. The
+ * __unregister_session() call is deferred until after
+ * remove_session_caps() below, or else other
+ * processes may find caps still assigned to this
+ * session while working with a new session object.
+ */
ceph_get_mds_session(session);
- __unregister_session(mdsc, session);
+
+ if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
+ pr_info_client(cl, "mds%d reconnect denied\n",
+ session->s_mds);
+
+ session->s_state = CEPH_MDS_SESSION_CLOSED;
}
/* FIXME: this ttl calculation is generous */
session->s_ttl = jiffies + HZ*mdsc->mdsmap->m_session_autoclose;
@@ -4551,12 +4609,24 @@ static void handle_session(struct ceph_mds_session *session,
break;
case CEPH_SESSION_CLOSE:
- if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
- pr_info_client(cl, "mds%d reconnect denied\n",
- session->s_mds);
- session->s_state = CEPH_MDS_SESSION_CLOSED;
cleanup_session_requests(mdsc, session);
remove_session_caps(session);
+
+ /*
+ * Now that all caps are removed, it is safe release
+ * the MDS rank and allow other processes to create a
+ * new session object.
+ *
+ * A concurrent ceph_mdsc_close_sessions() or
+ * check_new_map() may have unregistered the session
+ * already, so check __verify_registered_session()
+ * first.
+ */
+ mutex_lock(&mdsc->mutex);
+ if (!__verify_registered_session(mdsc, session))
+ __unregister_session(mdsc, session);
+ mutex_unlock(&mdsc->mutex);
+
wake = 2; /* for good measure */
wake_up_all(&mdsc->session_close_wq);
break;
@@ -5802,6 +5872,10 @@ static void check_new_map(struct ceph_mds_client *mdsc,
s = __ceph_lookup_mds_session(mdsc, i);
if (!s)
continue;
+ if (s->s_state == CEPH_MDS_SESSION_CLOSED) {
+ ceph_put_mds_session(s);
+ continue;
+ }
oldstate = ceph_mdsmap_get_state(oldmap, i);
newstate = ceph_mdsmap_get_state(newmap, i);
@@ -5814,18 +5888,24 @@ static void check_new_map(struct ceph_mds_client *mdsc,
if (i >= newmap->possible_max_rank) {
/* force close session for stopped mds */
- __unregister_session(mdsc, s);
- __wake_requests(mdsc, &s->s_waiting);
+ s->s_state = CEPH_MDS_SESSION_CLOSED;
mutex_unlock(&mdsc->mutex);
mutex_lock(&s->s_mutex);
cleanup_session_requests(mdsc, s);
remove_session_caps(s);
+
+ mutex_lock(&mdsc->mutex);
+ if (!__verify_registered_session(mdsc, s))
+ __unregister_session(mdsc, s);
+ mutex_unlock(&mdsc->mutex);
mutex_unlock(&s->s_mutex);
- ceph_put_mds_session(s);
+ wake_up_all(&mdsc->session_close_wq);
mutex_lock(&mdsc->mutex);
+ __wake_requests(mdsc, &s->s_waiting);
+ ceph_put_mds_session(s);
if (mdsc->mdsmap->m_epoch != map_epoch)
return;
kick_requests(mdsc, i);
@@ -5844,6 +5924,16 @@ static void check_new_map(struct ceph_mds_client *mdsc,
ceph_put_mds_session(s);
return;
}
+ if (s->s_state == CEPH_MDS_SESSION_CLOSED) {
+ /*
+ * handle_session() set state=CLOSED
+ * in the mutex gap above and is tearing it
+ * down
+ */
+ mutex_unlock(&s->s_mutex);
+ ceph_put_mds_session(s);
+ continue;
+ }
ceph_con_close(&s->s_con);
mutex_unlock(&s->s_mutex);
s->s_state = CEPH_MDS_SESSION_RESTARTING;
@@ -5902,6 +5992,9 @@ static void check_new_map(struct ceph_mds_client *mdsc,
* Only open and reconnect sessions that don't exist yet.
*/
for (i = 0; i < newmap->possible_max_rank; i++) {
+ if (mdsc->stopping)
+ return;
+
/*
* In case the import MDS is crashed just after
* the EImportStart journal is flushed, so when
@@ -5927,6 +6020,19 @@ static void check_new_map(struct ceph_mds_client *mdsc,
* reconnection request in up:reconnect state.
*/
s = __ceph_lookup_mds_session(mdsc, i);
+ if (s && s->s_state == CEPH_MDS_SESSION_CLOSED) {
+ /*
+ * Wait for handle_session() to remove the caps and
+ * unregister this session, so the reconnect below
+ * uses a fresh session on the now vacant rank
+ */
+ ceph_put_mds_session(s);
+ wait_for_mds_rank_not_closing(mdsc, i);
+ if (mdsc->stopping ||
+ mdsc->mdsmap->m_epoch != map_epoch)
+ return;
+ s = NULL;
+ }
if (likely(!s)) {
s = __open_export_target_session(mdsc, i);
if (IS_ERR(s)) {
@@ -7087,9 +7193,7 @@ static void mds_peer_reset(struct ceph_connection *con)
* Snapshot session state with READ_ONCE, then revalidate under
* mdsc->mutex before acting. The subsequent mdsc->mutex
* section rechecks s_state to catch concurrent transitions, so
- * the lockless snapshot here is safe. s->s_mutex is taken
- * separately for cleanup after unregistration, which avoids
- * introducing a new s->s_mutex + mdsc->mutex nesting.
+ * the lockless snapshot here is safe.
*/
session_state = READ_ONCE(s->s_state);
@@ -7114,18 +7218,24 @@ static void mds_peer_reset(struct ceph_connection *con)
ceph_get_mds_session(s);
s->s_state = CEPH_MDS_SESSION_CLOSED;
- __unregister_session(mdsc, s);
- __wake_requests(mdsc, &s->s_waiting);
mutex_unlock(&mdsc->mutex);
mutex_lock(&s->s_mutex);
cleanup_session_requests(mdsc, s);
remove_session_caps(s);
+
+ /* Keep the rank occupied until all old-session caps are gone. */
+ mutex_lock(&mdsc->mutex);
+ if (!__verify_registered_session(mdsc, s))
+ __unregister_session(mdsc, s);
+ mutex_unlock(&mdsc->mutex);
+
mutex_unlock(&s->s_mutex);
wake_up_all(&mdsc->session_close_wq);
mutex_lock(&mdsc->mutex);
+ __wake_requests(mdsc, &s->s_waiting);
kick_requests(mdsc, s->s_mds);
mutex_unlock(&mdsc->mutex);
--
2.47.3
^ permalink raw reply [flat|nested] 4+ messages in thread