On Fri, Nov 3, 2023 at 1:23 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > > > On 11/3/23 14:43, Venky Shankar wrote: > > On Fri, Nov 3, 2023 at 12:10 PM Venky Shankar <vshankar@xxxxxxxxxx> wrote: > >> Following along the same lines as per the user-space fix. Right > >> now this isn't really an issue with the ceph kernel driver because > >> of the feature bit laginess, however, that can change over time > >> (when the new snaprealm info type is ported to the kernel driver) > >> and depending on the MDS version that's being upgraded can cause > >> message decoding issues - so, fix that early on. > >> > >> URL: Fixes: http://tracker.ceph.com/issues/63188 > >> Signed-off-by: Venky Shankar <vshankar@xxxxxxxxxx> > >> --- > >> fs/ceph/mds_client.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > >> index a7bffb030036..48d75e17115c 100644 > >> --- a/fs/ceph/mds_client.c > >> +++ b/fs/ceph/mds_client.c > >> @@ -4192,6 +4192,7 @@ static void handle_session(struct ceph_mds_session *session, > >> if (session->s_state == CEPH_MDS_SESSION_OPEN) { > >> pr_notice_client(cl, "mds%d is already opened\n", > >> session->s_mds); > >> + session->s_features = features; > > Xiubo, the metrics stuff isn't done here (as it's done in the else > > case). That's probably required I guess?? > > That should be okay, but it harmless to do it here. > > So let's just fix it by: > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 41be58baaa57..de3c6b6cbd07 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -4263,17 +4263,16 @@ static void handle_session(struct > ceph_mds_session *session, > pr_info_client(cl, "mds%d reconnect success\n", > session->s_mds); > > - if (session->s_state == CEPH_MDS_SESSION_OPEN) { > + if (session->s_state == CEPH_MDS_SESSION_OPEN) > pr_notice_client(cl, "mds%d is already opened\n", > session->s_mds); > - } else { > + else > session->s_state = CEPH_MDS_SESSION_OPEN; > - session->s_features = features; > - renewed_caps(mdsc, session, 0); > - if (test_bit(CEPHFS_FEATURE_METRIC_COLLECT, > - &session->s_features)) > - metric_schedule_delayed(&mdsc->metric); > - } > + session->s_features = features; > + renewed_caps(mdsc, session, 0); Call to renewed_caps() isn't really required if the session state is already open, isn't it? Doesn't harm to call it I guess, but... > + if (test_bit(CEPHFS_FEATURE_METRIC_COLLECT, > + &session->s_features)) > + metric_schedule_delayed(&mdsc->metric); > > /* > * The connection maybe broken and the session in client > > Thanks > > - Xiubo > > > > > >> } else { > >> session->s_state = CEPH_MDS_SESSION_OPEN; > >> session->s_features = features; > >> -- > >> 2.39.3 > >> > > > -- Cheers, Venky
