On Thu, 2022-05-26 at 20:45 +0800, Xiubo Li wrote: > On 5/26/22 6:44 PM, Jeff Layton wrote: > > On Thu, 2022-05-26 at 14:06 +0800, Xiubo Li wrote: > > > If the connection was accidently closed due to the socket issue or > > > something else the clients will try to open the opened sessions, the > > > MDSes will send the session open reply one more time if the clients > > > support the notify feature. > > > > > > When the clients retry to open the sessions the s_seq will be 0 as > > > default, we need to update it anyway. > > > > > > URL: https://tracker.ceph.com/issues/53911 > > > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > > > --- > > > fs/ceph/mds_client.c | 25 ++++++++++++++++++++----- > > > fs/ceph/mds_client.h | 5 ++++- > > > 2 files changed, 24 insertions(+), 6 deletions(-) > > > > > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > > index 4ced8d1e18ba..3e528b89b77a 100644 > > > --- a/fs/ceph/mds_client.c > > > +++ b/fs/ceph/mds_client.c > > > @@ -3569,11 +3569,26 @@ static void handle_session(struct ceph_mds_session *session, > > > case CEPH_SESSION_OPEN: > > > if (session->s_state == CEPH_MDS_SESSION_RECONNECTING) > > > pr_info("mds%d reconnect success\n", session->s_mds); > > > - 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); > > > + > > > + if (session->s_state == CEPH_MDS_SESSION_OPEN) { > > > + pr_info("mds%d already opened\n", session->s_mds); > > Does the above pr_info actually help anything? What will the admin do > > with this info? I'd probably just skip this since this is sort of > > expected to happen from time to time. > > Currently from the MDS side code, it won't be allowed to send the > session open reply more than once. So this should be something wrong > somewhere just like this issue we are fixing. So it's should be okay, > but maybe a warning instead ? > Ok. I'd probably go with pr_notice instead. It's not indicative of a kernel bug, necessarily, and we don't really need the admin to take any action (other than maybe report it to the ceph administrators). > > > > + } 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); > > > + } > > > + > > > + /* > > > + * The connection maybe broken and the session in client > > > + * side has been reinitialized, need to update the seq > > > + * anyway. > > > + */ > > > + if (!session->s_seq && seq) > > > + session->s_seq = seq; > > > + > > > wake = 1; > > > if (mdsc->stopping) > > > __close_session(mdsc, session); > > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > > > index d8ec2ac93da3..256e3eada6c1 100644 > > > --- a/fs/ceph/mds_client.h > > > +++ b/fs/ceph/mds_client.h > > > @@ -29,8 +29,10 @@ enum ceph_feature_type { > > > CEPHFS_FEATURE_MULTI_RECONNECT, > > > CEPHFS_FEATURE_DELEG_INO, > > > CEPHFS_FEATURE_METRIC_COLLECT, > > > + CEPHFS_FEATURE_ALTERNATE_NAME, > > > + CEPHFS_FEATURE_NOTIFY_SESSION_STATE, > > > > > > - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, > > > + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_NOTIFY_SESSION_STATE, > > > }; > > > > > > #define CEPHFS_FEATURES_CLIENT_SUPPORTED { \ > > > @@ -41,6 +43,7 @@ enum ceph_feature_type { > > > CEPHFS_FEATURE_MULTI_RECONNECT, \ > > > CEPHFS_FEATURE_DELEG_INO, \ > > > CEPHFS_FEATURE_METRIC_COLLECT, \ > > > + CEPHFS_FEATURE_NOTIFY_SESSION_STATE, \ > > > } > > > > > > /* > > The rest looks OK though. > -- Jeff Layton <jlayton@xxxxxxxxxx>
