Re: [PATCH] ceph: reinitialize mds feature bit even when session in open

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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







[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux