Done - updated (didn't add v2 to the pathset though). On Mon, Nov 6, 2023 at 5:55 AM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > > > On 11/3/23 17:29, Venky Shankar wrote: > > 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... > > Yeah. > > Then let's just do: > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 41be58baaa57..45d0f445cdef 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -4263,12 +4263,12 @@ static void handle_session(struct > ceph_mds_session *session, > pr_info_client(cl, "mds%d reconnect success\n", > session->s_mds); > > + session->s_features = features; > if (session->s_state == CEPH_MDS_SESSION_OPEN) { > pr_notice_client(cl, "mds%d is already opened\n", > session->s_mds); > } 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)) > > Thanks > > - Xiubo > > > >> + 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
