Re: [PATCH v2 3/5] ceph: add wait_list_lock for wait-list serialization

On Thu, 16 Jul 2026 at 02:30, Viacheslav Dubeyko <[email protected]> wrote:
>
> On Wed, 2026-07-15 at 11:58 +0800, Xiubo Li via B4 Relay wrote:
> > From: Xiubo Li <[email protected]>
> >
> > Add a dedicated spinlock to protect the waiting_for_map and
> > s_waiting lists, which are currently serialized by mdsc->mutex.
> > This is a preparatory step for removing mdsc->mutex from the
> > kick_requests() path: xa_for_each() is internally locked and the
> > per-request filter checks are lockless, so only list_del_init()
> > needs external protection.
> >
> > Signed-off-by: Xiubo Li <[email protected]>
> > ---
> >  fs/ceph/mds_client.c | 18 +++++++++++++++++-
> >  fs/ceph/mds_client.h |  3 +++
> >  2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index 239ed34886f9..a250938f32d3 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -3669,7 +3669,9 @@ static void __do_request(struct ceph_mds_client
> > *mdsc,
> >                       doutc(cl, "no mdsmap, waiting for map\n");
> >                       trace_ceph_mdsc_suspend_request(mdsc,
> > session, req,
> >                                                       ceph_mdsc_su
> > spend_reason_no_mdsmap);
> > +                     spin_lock(&mdsc->wait_list_lock);
> >                       list_add(&req->r_wait, &mdsc-
> > >waiting_for_map);
> > +                     spin_unlock(&mdsc->wait_list_lock);
> >                       return;
> >               }
> >               if (!(mdsc->fsc->mount_options->flags &
> > @@ -3692,7 +3694,9 @@ static void __do_request(struct ceph_mds_client
> > *mdsc,
> >               doutc(cl, "no mds or not active, waiting for
> > map\n");
> >               trace_ceph_mdsc_suspend_request(mdsc, session, req,
> >                                               ceph_mdsc_suspend_re
> > ason_no_active_mds);
> > +             spin_lock(&mdsc->wait_list_lock);
> >               list_add(&req->r_wait, &mdsc->waiting_for_map);
> > +             spin_unlock(&mdsc->wait_list_lock);
> >               return;
> >       }
> >
> > @@ -3740,9 +3744,12 @@ static void __do_request(struct
> > ceph_mds_client *mdsc,
> >                       if (ceph_test_mount_opt(mdsc->fsc,
> > CLEANRECOVER)) {
> >                               trace_ceph_mdsc_suspend_request(mdsc
> > , session, req,
> >                                                               ceph
> > _mdsc_suspend_reason_rejected);
> > +                             spin_lock(&mdsc->wait_list_lock);
> >                               list_add(&req->r_wait, &mdsc-
> > >waiting_for_map);
> > -                     } else
> > +                             spin_unlock(&mdsc->wait_list_lock);
> > +                     } else {
> >                               err = -EACCES;
> > +                     }
>
> Do we really need to add the curly brackets here?
>

Yeah.

Else the checkpatch.pl will warn:

WARNING: braces {} are necessary for all arms of this statement

It's a hard requirement of kernel coding style: if one branch of an if
has braces, the other must too.


Thanks
Xiubo

> Thanks,
> Slava.
[......]