diff mbox series

ceph: don't let check_caps skip sending responses for revoke msgs

Message ID 20230627070101.170876-1-xiubli@redhat.com
State Superseded
Headers show
Series ceph: don't let check_caps skip sending responses for revoke msgs | expand

Commit Message

Xiubo Li June 27, 2023, 7:01 a.m. UTC
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.

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(+)

Comments

Xiubo Li June 27, 2023, 11:47 p.m. UTC | #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 mbox series

Patch

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;