* [PATCH] ceph: add session already open notify support
@ 2022-05-26 6:06 Xiubo Li
2022-05-26 10:44 ` Jeff Layton
0 siblings, 1 reply; 5+ messages in thread
From: Xiubo Li @ 2022-05-26 6:06 UTC (permalink / raw)
To: jlayton, idryomov; +Cc: lhenriques, vshankar, ceph-devel, Xiubo Li
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@redhat.com>
---
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);
+ } 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, \
}
/*
--
2.36.0.rc1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: add session already open notify support
2022-05-26 6:06 [PATCH] ceph: add session already open notify support Xiubo Li
@ 2022-05-26 10:44 ` Jeff Layton
2022-05-26 12:45 ` Xiubo Li
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2022-05-26 10:44 UTC (permalink / raw)
To: Xiubo Li, idryomov; +Cc: lhenriques, vshankar, ceph-devel
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@redhat.com>
> ---
> 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.
> + } 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@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: add session already open notify support
2022-05-26 10:44 ` Jeff Layton
@ 2022-05-26 12:45 ` Xiubo Li
2022-05-26 14:20 ` Jeff Layton
0 siblings, 1 reply; 5+ messages in thread
From: Xiubo Li @ 2022-05-26 12:45 UTC (permalink / raw)
To: Jeff Layton, idryomov; +Cc: lhenriques, vshankar, ceph-devel
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@redhat.com>
>> ---
>> 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 ?
>> + } 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: add session already open notify support
2022-05-26 12:45 ` Xiubo Li
@ 2022-05-26 14:20 ` Jeff Layton
2022-05-27 0:28 ` Xiubo Li
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2022-05-26 14:20 UTC (permalink / raw)
To: Xiubo Li, idryomov; +Cc: lhenriques, vshankar, ceph-devel
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@redhat.com>
> > > ---
> > > 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@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: add session already open notify support
2022-05-26 14:20 ` Jeff Layton
@ 2022-05-27 0:28 ` Xiubo Li
0 siblings, 0 replies; 5+ messages in thread
From: Xiubo Li @ 2022-05-27 0:28 UTC (permalink / raw)
To: Jeff Layton, idryomov; +Cc: lhenriques, vshankar, ceph-devel
On 5/26/22 10:20 PM, Jeff Layton wrote:
> 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@redhat.com>
>>>> ---
>>>> 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).
Sure, will switch to notice instead.
Thanks
-- Xiubo
>>>> + } 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-27 0:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26 6:06 [PATCH] ceph: add session already open notify support Xiubo Li
2022-05-26 10:44 ` Jeff Layton
2022-05-26 12:45 ` Xiubo Li
2022-05-26 14:20 ` Jeff Layton
2022-05-27 0:28 ` Xiubo Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.