Jeff Layton <jlayton@xxxxxxxxxx> 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
