| Message ID | 20230627070101.170876-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: don't let check_caps skip sending responses for revoke msgs | expand |
On Tue, Jun 27, 2023 at 12:33 PM <xiubli@redhat.com> wrote: > > From: Xiubo Li <xiubli@redhat.com> > > If just before the revoke request, which will increase the 'seq', is > sent out the clients released the corresponding caps and sent out > the cap update request to MDS with old 'seq', the mds will miss > checking the seqs and calculating the caps. > > We should always send an ack for revoke requests. I think the commit message needs to be rephrased for better understanding to something like: If a client sends out a cap update request with the old 'seq' just before a pending cap revoke request, then the MDS might miscalculate the 'seqs' and caps. It's therefore always a good idea to ack the cap revoke request with the bumped up 'seq'. Xiubo, please let me know if this sounds okay to you. > > Cc: stable@vger.kernel.org > Cc: Patrick Donnelly <pdonnell@redhat.com> > URL: https://tracker.ceph.com/issues/61782 > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/caps.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index 1052885025b3..eee2fbca3430 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -3737,6 +3737,15 @@ static void handle_cap_grant(struct inode *inode, > } > BUG_ON(cap->issued & ~cap->implemented); > > + /* don't let check_caps skip sending a response to MDS for revoke msgs */ > + if (le32_to_cpu(grant->op) == CEPH_CAP_OP_REVOKE) { > + cap->mds_wanted = 0; > + if (cap == ci->i_auth_cap) > + check_caps = 1; /* check auth cap only */ > + else > + check_caps = 2; /* check all caps */ > + } > + > if (extra_info->inline_version > 0 && > extra_info->inline_version >= ci->i_inline_version) { > ci->i_inline_version = extra_info->inline_version; > -- > 2.40.1 >
On 6/27/23 21:57, Milind Changire wrote: > On Tue, Jun 27, 2023 at 12:33 PM <xiubli@redhat.com> wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> If just before the revoke request, which will increase the 'seq', is >> sent out the clients released the corresponding caps and sent out >> the cap update request to MDS with old 'seq', the mds will miss >> checking the seqs and calculating the caps. >> >> We should always send an ack for revoke requests. > I think the commit message needs to be rephrased for better > understanding to something like: > > If a client sends out a cap update request with the old 'seq' just > before a pending cap revoke request, then the MDS might miscalculate > the 'seqs' and caps. It's therefore always a good idea to ack the cap > revoke request with the bumped up 'seq'. > > Xiubo, please let me know if this sounds okay to you. > Milind, Yeah, this looks much better. I will update it. Thanks - Xiubo >> Cc: stable@vger.kernel.org >> Cc: Patrick Donnelly <pdonnell@redhat.com> >> URL: https://tracker.ceph.com/issues/61782 >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> fs/ceph/caps.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c >> index 1052885025b3..eee2fbca3430 100644 >> --- a/fs/ceph/caps.c >> +++ b/fs/ceph/caps.c >> @@ -3737,6 +3737,15 @@ static void handle_cap_grant(struct inode *inode, >> } >> BUG_ON(cap->issued & ~cap->implemented); >> >> + /* don't let check_caps skip sending a response to MDS for revoke msgs */ >> + if (le32_to_cpu(grant->op) == CEPH_CAP_OP_REVOKE) { >> + cap->mds_wanted = 0; >> + if (cap == ci->i_auth_cap) >> + check_caps = 1; /* check auth cap only */ >> + else >> + check_caps = 2; /* check all caps */ >> + } >> + >> if (extra_info->inline_version > 0 && >> extra_info->inline_version >= ci->i_inline_version) { >> ci->i_inline_version = extra_info->inline_version; >> -- >> 2.40.1 >> >
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index 1052885025b3..eee2fbca3430 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -3737,6 +3737,15 @@ static void handle_cap_grant(struct inode *inode, } BUG_ON(cap->issued & ~cap->implemented); + /* don't let check_caps skip sending a response to MDS for revoke msgs */ + if (le32_to_cpu(grant->op) == CEPH_CAP_OP_REVOKE) { + cap->mds_wanted = 0; + if (cap == ci->i_auth_cap) + check_caps = 1; /* check auth cap only */ + else + check_caps = 2; /* check all caps */ + } + if (extra_info->inline_version > 0 && extra_info->inline_version >= ci->i_inline_version) { ci->i_inline_version = extra_info->inline_version;