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