Re: [RFC PATCH] ceph: guard against __ceph_remove_cap races

[Luis Henriques <lhenriques@suse.de>]

Jeff Layton <jlayton@kernel.org> writes:

> On Wed, 2020-11-11 at 14:11 +0000, Luis Henriques wrote:
>> 
>> 
>
> It think this looks reasonable. Minor nits below:
>
>> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
>> index ded4229c314a..917dfaf0bd01 100644
>> --- a/fs/ceph/caps.c
>> +++ b/fs/ceph/caps.c
>> @@ -1140,12 +1140,17 @@ void __ceph_remove_cap(struct ceph_cap *cap, bool queue_release)
>>  {
>>         struct ceph_mds_session *session = cap->session;
>>         struct ceph_inode_info *ci = cap->ci;
>> -       struct ceph_mds_client *mdsc =
>> -               ceph_sb_to_client(ci->vfs_inode.i_sb)->mdsc;
>> +       struct ceph_mds_client *mdsc;
>> +
>
> nit: remove the above newline
>
>>         int removed = 0;
>>  
>
> Maybe add a comment here to the effect that a NULL cap->ci indicates
> that the remove has already been done?
>
>> +       if (!ci)
>> +               return;
>> +
>>         dout("__ceph_remove_cap %p from %p\n", cap, &ci->vfs_inode);
>>  
>> +       mdsc = ceph_sb_to_client(ci->vfs_inode.i_sb)->mdsc;
>> +
>
> There's a ceph_inode_to_client helper now that may make this a bit more
> readable.
>
>>         /* remove from inode's cap rbtree, and clear auth cap */
>>         rb_erase(&cap->ci_node, &ci->i_caps);
>>         if (ci->i_auth_cap == cap) {

Thanks Jeff.  I'll re-post this soon with your suggestions.  I just want
to run some more local tests to make sure things aren't breaking with this
change.

Cheers,
-- 
Luis