diff mbox series

ceph: always check dir caps asynchronously

Message ID 20240104041723.1120866-1-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series ceph: always check dir caps asynchronously | expand

Commit Message

Xiubo Li Jan. 4, 2024, 4:17 a.m. UTC
From: Xiubo Li <xiubli@redhat.com>

The MDS will issue the 'Fr' caps for async dirop, while there is
buggy in kclient and it could miss releasing the async dirop caps,
which is 'Fsxr'. And then the MDS will complain with:

"[WRN] client.xxx isn't responding to mclientcaps(revoke) ..."

So when releasing the dirop async requests or when they fail we
should always make sure that being revoked caps could be released.

URL: https://tracker.ceph.com/issues/50223
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/caps.c       | 6 ------
 fs/ceph/mds_client.c | 9 ++++-----
 fs/ceph/mds_client.h | 2 +-
 fs/ceph/super.h      | 2 --
 4 files changed, 5 insertions(+), 14 deletions(-)

Comments

Milind Changire Jan. 16, 2024, 6:02 a.m. UTC | #1
Approved.

Reviewed-by: Milind Changire <mchangir@redhat.com>

On Thu, Jan 4, 2024 at 9:49 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> The MDS will issue the 'Fr' caps for async dirop, while there is
> buggy in kclient and it could miss releasing the async dirop caps,
> which is 'Fsxr'. And then the MDS will complain with:
>
> "[WRN] client.xxx isn't responding to mclientcaps(revoke) ..."
>
> So when releasing the dirop async requests or when they fail we
> should always make sure that being revoked caps could be released.
>
> URL: https://tracker.ceph.com/issues/50223
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/caps.c       | 6 ------
>  fs/ceph/mds_client.c | 9 ++++-----
>  fs/ceph/mds_client.h | 2 +-
>  fs/ceph/super.h      | 2 --
>  4 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index a9e19f1f26e0..4dd92f09b16e 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3216,7 +3216,6 @@ static int ceph_try_drop_cap_snap(struct ceph_inode_info *ci,
>
>  enum put_cap_refs_mode {
>         PUT_CAP_REFS_SYNC = 0,
> -       PUT_CAP_REFS_NO_CHECK,
>         PUT_CAP_REFS_ASYNC,
>  };
>
> @@ -3332,11 +3331,6 @@ void ceph_put_cap_refs_async(struct ceph_inode_info *ci, int had)
>         __ceph_put_cap_refs(ci, had, PUT_CAP_REFS_ASYNC);
>  }
>
> -void ceph_put_cap_refs_no_check_caps(struct ceph_inode_info *ci, int had)
> -{
> -       __ceph_put_cap_refs(ci, had, PUT_CAP_REFS_NO_CHECK);
> -}
> -
>  /*
>   * Release @nr WRBUFFER refs on dirty pages for the given @snapc snap
>   * context.  Adjust per-snap dirty page accounting as appropriate.
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 7bdee08ec2eb..f278194a1a01 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1089,7 +1089,7 @@ void ceph_mdsc_release_request(struct kref *kref)
>         struct ceph_mds_request *req = container_of(kref,
>                                                     struct ceph_mds_request,
>                                                     r_kref);
> -       ceph_mdsc_release_dir_caps_no_check(req);
> +       ceph_mdsc_release_dir_caps_async(req);
>         destroy_reply_info(&req->r_reply_info);
>         if (req->r_request)
>                 ceph_msg_put(req->r_request);
> @@ -4428,7 +4428,7 @@ void ceph_mdsc_release_dir_caps(struct ceph_mds_request *req)
>         }
>  }
>
> -void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req)
> +void ceph_mdsc_release_dir_caps_async(struct ceph_mds_request *req)
>  {
>         struct ceph_client *cl = req->r_mdsc->fsc->client;
>         int dcaps;
> @@ -4436,8 +4436,7 @@ void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req)
>         dcaps = xchg(&req->r_dir_caps, 0);
>         if (dcaps) {
>                 doutc(cl, "releasing r_dir_caps=%s\n", ceph_cap_string(dcaps));
> -               ceph_put_cap_refs_no_check_caps(ceph_inode(req->r_parent),
> -                                               dcaps);
> +               ceph_put_cap_refs_async(ceph_inode(req->r_parent), dcaps);
>         }
>  }
>
> @@ -4473,7 +4472,7 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
>                 if (req->r_session->s_mds != session->s_mds)
>                         continue;
>
> -               ceph_mdsc_release_dir_caps_no_check(req);
> +               ceph_mdsc_release_dir_caps_async(req);
>
>                 __send_request(session, req, true);
>         }
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index e85172a92e6c..92695a280d7b 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -579,7 +579,7 @@ extern int ceph_mdsc_do_request(struct ceph_mds_client *mdsc,
>                                 struct inode *dir,
>                                 struct ceph_mds_request *req);
>  extern void ceph_mdsc_release_dir_caps(struct ceph_mds_request *req);
> -extern void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req);
> +extern void ceph_mdsc_release_dir_caps_async(struct ceph_mds_request *req);
>  static inline void ceph_mdsc_get_request(struct ceph_mds_request *req)
>  {
>         kref_get(&req->r_kref);
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index f418b43d0e05..8832da060253 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -1258,8 +1258,6 @@ extern void ceph_take_cap_refs(struct ceph_inode_info *ci, int caps,
>  extern void ceph_get_cap_refs(struct ceph_inode_info *ci, int caps);
>  extern void ceph_put_cap_refs(struct ceph_inode_info *ci, int had);
>  extern void ceph_put_cap_refs_async(struct ceph_inode_info *ci, int had);
> -extern void ceph_put_cap_refs_no_check_caps(struct ceph_inode_info *ci,
> -                                           int had);
>  extern void ceph_put_wrbuffer_cap_refs(struct ceph_inode_info *ci, int nr,
>                                        struct ceph_snap_context *snapc);
>  extern void __ceph_remove_capsnap(struct inode *inode,
> --
> 2.43.0
>
Venky Shankar Feb. 8, 2024, 6:46 a.m. UTC | #2
On Thu, Jan 4, 2024 at 9:49 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> The MDS will issue the 'Fr' caps for async dirop, while there is
> buggy in kclient and it could miss releasing the async dirop caps,
> which is 'Fsxr'. And then the MDS will complain with:
>
> "[WRN] client.xxx isn't responding to mclientcaps(revoke) ..."
>
> So when releasing the dirop async requests or when they fail we
> should always make sure that being revoked caps could be released.
>
> URL: https://tracker.ceph.com/issues/50223
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/caps.c       | 6 ------
>  fs/ceph/mds_client.c | 9 ++++-----
>  fs/ceph/mds_client.h | 2 +-
>  fs/ceph/super.h      | 2 --
>  4 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index a9e19f1f26e0..4dd92f09b16e 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3216,7 +3216,6 @@ static int ceph_try_drop_cap_snap(struct ceph_inode_info *ci,
>
>  enum put_cap_refs_mode {
>         PUT_CAP_REFS_SYNC = 0,
> -       PUT_CAP_REFS_NO_CHECK,
>         PUT_CAP_REFS_ASYNC,
>  };
>
> @@ -3332,11 +3331,6 @@ void ceph_put_cap_refs_async(struct ceph_inode_info *ci, int had)
>         __ceph_put_cap_refs(ci, had, PUT_CAP_REFS_ASYNC);
>  }
>
> -void ceph_put_cap_refs_no_check_caps(struct ceph_inode_info *ci, int had)
> -{
> -       __ceph_put_cap_refs(ci, had, PUT_CAP_REFS_NO_CHECK);
> -}
> -
>  /*
>   * Release @nr WRBUFFER refs on dirty pages for the given @snapc snap
>   * context.  Adjust per-snap dirty page accounting as appropriate.
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 7bdee08ec2eb..f278194a1a01 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1089,7 +1089,7 @@ void ceph_mdsc_release_request(struct kref *kref)
>         struct ceph_mds_request *req = container_of(kref,
>                                                     struct ceph_mds_request,
>                                                     r_kref);
> -       ceph_mdsc_release_dir_caps_no_check(req);
> +       ceph_mdsc_release_dir_caps_async(req);
>         destroy_reply_info(&req->r_reply_info);
>         if (req->r_request)
>                 ceph_msg_put(req->r_request);
> @@ -4428,7 +4428,7 @@ void ceph_mdsc_release_dir_caps(struct ceph_mds_request *req)
>         }
>  }
>
> -void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req)
> +void ceph_mdsc_release_dir_caps_async(struct ceph_mds_request *req)
>  {
>         struct ceph_client *cl = req->r_mdsc->fsc->client;
>         int dcaps;
> @@ -4436,8 +4436,7 @@ void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req)
>         dcaps = xchg(&req->r_dir_caps, 0);
>         if (dcaps) {
>                 doutc(cl, "releasing r_dir_caps=%s\n", ceph_cap_string(dcaps));
> -               ceph_put_cap_refs_no_check_caps(ceph_inode(req->r_parent),
> -                                               dcaps);
> +               ceph_put_cap_refs_async(ceph_inode(req->r_parent), dcaps);
>         }
>  }
>
> @@ -4473,7 +4472,7 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
>                 if (req->r_session->s_mds != session->s_mds)
>                         continue;
>
> -               ceph_mdsc_release_dir_caps_no_check(req);
> +               ceph_mdsc_release_dir_caps_async(req);
>
>                 __send_request(session, req, true);
>         }
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index e85172a92e6c..92695a280d7b 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -579,7 +579,7 @@ extern int ceph_mdsc_do_request(struct ceph_mds_client *mdsc,
>                                 struct inode *dir,
>                                 struct ceph_mds_request *req);
>  extern void ceph_mdsc_release_dir_caps(struct ceph_mds_request *req);
> -extern void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req);
> +extern void ceph_mdsc_release_dir_caps_async(struct ceph_mds_request *req);
>  static inline void ceph_mdsc_get_request(struct ceph_mds_request *req)
>  {
>         kref_get(&req->r_kref);
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index f418b43d0e05..8832da060253 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -1258,8 +1258,6 @@ extern void ceph_take_cap_refs(struct ceph_inode_info *ci, int caps,
>  extern void ceph_get_cap_refs(struct ceph_inode_info *ci, int caps);
>  extern void ceph_put_cap_refs(struct ceph_inode_info *ci, int had);
>  extern void ceph_put_cap_refs_async(struct ceph_inode_info *ci, int had);
> -extern void ceph_put_cap_refs_no_check_caps(struct ceph_inode_info *ci,
> -                                           int had);
>  extern void ceph_put_wrbuffer_cap_refs(struct ceph_inode_info *ci, int nr,
>                                        struct ceph_snap_context *snapc);
>  extern void __ceph_remove_capsnap(struct inode *inode,
> --
> 2.43.0
>

Tested-by: Venky Shankar <vshankar@redhat.com>
diff mbox series

Patch

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index a9e19f1f26e0..4dd92f09b16e 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -3216,7 +3216,6 @@  static int ceph_try_drop_cap_snap(struct ceph_inode_info *ci,
 
 enum put_cap_refs_mode {
 	PUT_CAP_REFS_SYNC = 0,
-	PUT_CAP_REFS_NO_CHECK,
 	PUT_CAP_REFS_ASYNC,
 };
 
@@ -3332,11 +3331,6 @@  void ceph_put_cap_refs_async(struct ceph_inode_info *ci, int had)
 	__ceph_put_cap_refs(ci, had, PUT_CAP_REFS_ASYNC);
 }
 
-void ceph_put_cap_refs_no_check_caps(struct ceph_inode_info *ci, int had)
-{
-	__ceph_put_cap_refs(ci, had, PUT_CAP_REFS_NO_CHECK);
-}
-
 /*
  * Release @nr WRBUFFER refs on dirty pages for the given @snapc snap
  * context.  Adjust per-snap dirty page accounting as appropriate.
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 7bdee08ec2eb..f278194a1a01 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1089,7 +1089,7 @@  void ceph_mdsc_release_request(struct kref *kref)
 	struct ceph_mds_request *req = container_of(kref,
 						    struct ceph_mds_request,
 						    r_kref);
-	ceph_mdsc_release_dir_caps_no_check(req);
+	ceph_mdsc_release_dir_caps_async(req);
 	destroy_reply_info(&req->r_reply_info);
 	if (req->r_request)
 		ceph_msg_put(req->r_request);
@@ -4428,7 +4428,7 @@  void ceph_mdsc_release_dir_caps(struct ceph_mds_request *req)
 	}
 }
 
-void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req)
+void ceph_mdsc_release_dir_caps_async(struct ceph_mds_request *req)
 {
 	struct ceph_client *cl = req->r_mdsc->fsc->client;
 	int dcaps;
@@ -4436,8 +4436,7 @@  void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req)
 	dcaps = xchg(&req->r_dir_caps, 0);
 	if (dcaps) {
 		doutc(cl, "releasing r_dir_caps=%s\n", ceph_cap_string(dcaps));
-		ceph_put_cap_refs_no_check_caps(ceph_inode(req->r_parent),
-						dcaps);
+		ceph_put_cap_refs_async(ceph_inode(req->r_parent), dcaps);
 	}
 }
 
@@ -4473,7 +4472,7 @@  static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 		if (req->r_session->s_mds != session->s_mds)
 			continue;
 
-		ceph_mdsc_release_dir_caps_no_check(req);
+		ceph_mdsc_release_dir_caps_async(req);
 
 		__send_request(session, req, true);
 	}
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index e85172a92e6c..92695a280d7b 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -579,7 +579,7 @@  extern int ceph_mdsc_do_request(struct ceph_mds_client *mdsc,
 				struct inode *dir,
 				struct ceph_mds_request *req);
 extern void ceph_mdsc_release_dir_caps(struct ceph_mds_request *req);
-extern void ceph_mdsc_release_dir_caps_no_check(struct ceph_mds_request *req);
+extern void ceph_mdsc_release_dir_caps_async(struct ceph_mds_request *req);
 static inline void ceph_mdsc_get_request(struct ceph_mds_request *req)
 {
 	kref_get(&req->r_kref);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index f418b43d0e05..8832da060253 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -1258,8 +1258,6 @@  extern void ceph_take_cap_refs(struct ceph_inode_info *ci, int caps,
 extern void ceph_get_cap_refs(struct ceph_inode_info *ci, int caps);
 extern void ceph_put_cap_refs(struct ceph_inode_info *ci, int had);
 extern void ceph_put_cap_refs_async(struct ceph_inode_info *ci, int had);
-extern void ceph_put_cap_refs_no_check_caps(struct ceph_inode_info *ci,
-					    int had);
 extern void ceph_put_wrbuffer_cap_refs(struct ceph_inode_info *ci, int nr,
 				       struct ceph_snap_context *snapc);
 extern void __ceph_remove_capsnap(struct inode *inode,