Re: [PATCH 2/3] fs/ceph/caps: mark cap remove with RB_CLEAR_NODE() instead of ci=NULL

On Fri, Jun 12, 2026 at 7:05 PM Max Kellermann <[email protected]> wrote:
> @@ -1142,6 +1142,7 @@ static void __ceph_remove_cap(struct ceph_cap *cap, bool queue_release)
>
>         /* remove from inode's cap rbtree, and clear auth cap */
>         rb_erase(&cap->ci_node, &ci->i_caps);
> +       RB_CLEAR_NODE(&cap->ci_node);
>         if (ci->i_auth_cap == cap)
>                 ci->i_auth_cap = NULL;
>
> @@ -1158,8 +1159,6 @@ static void __ceph_remove_cap(struct ceph_cap *cap, bool queue_release)
>                 cap->session = NULL;
>                 removed = 1;
>         }
> -       /* protect backpointer with s_cap_lock: see iterate_session_caps */
> -       cap->ci = NULL;

Don't merge this patch; while writing follow-up optimizations, I found
out that this causes a data race because clearing cap->ci_node is no
longer protected by s_cap_lock.

Modifying this "cap removed" marker requires holding BOTH
ci->i_ceph_lock and session->s_cap_lock. The existing code comment
(which I ignored & removed, ugh!) is not sufficient.
(And setting the marker is really only necessary if
session->s_cap_iterator==cap.)

I will eventually post v2 without this bug. And with more
documentation on the locking semantics.

-- 
Max Kellermann
Principal Architect
Hosting Technology

cm4all | Im Mediapark 6a | 50670 Köln | Germany
General information about the company can be found here:
https://www.cm4all.com/impressum
A member of the IONOS Group