> On 15 Mar 2018, at 08:15, Patrick Donnelly <pdonnell@xxxxxxxxxx> wrote:
>
> On Wed, Mar 14, 2018 at 4:58 PM, Yan, Zheng <zyan@xxxxxxxxxx> wrote:
>>
>>
>>> On 15 Mar 2018, at 02:55, Gregory Farnum <gfarnum@xxxxxxxxxx> wrote:
>>>
>>> On Wed, Mar 14, 2018 at 1:14 AM Yan, Zheng <ukernel@xxxxxxxxx> wrote:
>>> On Tue, Mar 13, 2018 at 7:52 PM, Chengguang Xu <cgxu519@xxxxxxx> wrote:
>>>> Obviously wrong mutex lock/unlock for nothing.
>>>>
>>>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxx>
>>>> ---
>>>> fs/ceph/mds_client.c | 2 --
>>>> 1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
>>>> index 8fc37a8..5439dfd 100644
>>>> --- a/fs/ceph/mds_client.c
>>>> +++ b/fs/ceph/mds_client.c
>>>> @@ -3514,8 +3514,6 @@ static void drop_leases(struct ceph_mds_client *mdsc)
>>>> if (!s)
>>>> continue;
>>>> mutex_unlock(&mdsc->mutex);
>>>> - mutex_lock(&s->s_mutex);
>>>> - mutex_unlock(&s->s_mutex);
>>>> ceph_put_mds_session(s);
>>>> mutex_lock(&mdsc->mutex);
>>>> }
>>>> --
>>>> 1.8.3.1
>>>>
>>> Applied, thanks
>>> Yan, Zheng
>>>
>>> So I'm sure it's not the case, but when I saw this go by I was assuming that the mutex grab is to make sure we don't have anybody still running in a critical section that assumes those caps are held. Is that not an issue and/or definitely not the case? :)
>>> -Greg
>>
>> I think you are right. Thanks
>
> In that case, it needs a comment. :)
>
how about below patch
commit f02745a2134f2925ef0945f800d4cb8fa2d076d6 (HEAD -> testing)
Author: Chengguang Xu <cgxu519@xxxxxxx>
Date: Tue Mar 13 19:52:20 2018 +0800
ceph: rename function drop_leases() to a more descriptive name
Signed-off-by: "Yan, Zheng" <zyan@xxxxxxxxxx>
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index c6331aecd191..a21cd65e8f68 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3497,13 +3497,12 @@ void ceph_mdsc_lease_send_msg(struct ceph_mds_session *session,
}
/*
- * drop all leases (and dentry refs) in preparation for umount
+ * lock unlock sessions, to wait ongoing session activities
*/
-static void drop_leases(struct ceph_mds_client *mdsc)
+static void lock_unlock_sessions(struct ceph_mds_client *mdsc)
{
int i;
- dout("drop_leases\n");
mutex_lock(&mdsc->mutex);
for (i = 0; i < mdsc->max_sessions; i++) {
struct ceph_mds_session *s = __ceph_lookup_mds_session(mdsc, i);
@@ -3694,7 +3693,7 @@ void ceph_mdsc_pre_umount(struct ceph_mds_client *mdsc)
dout("pre_umount\n");
mdsc->stopping = 1;
- drop_leases(mdsc);
+ lock_unlock_sessions(mdsc);
ceph_flush_dirty_caps(mdsc);
wait_requests(mdsc);
> --
> Patrick Donnelly
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
