diff mbox series

[v3] ceph: rename get_session and switch to use ceph_get_mds_session

Message ID 20191220004409.12793-1-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v3] ceph: rename get_session and switch to use ceph_get_mds_session | expand

Commit Message

Xiubo Li Dec. 20, 2019, 12:44 a.m. UTC 
From: Xiubo Li <xiubli@redhat.com>

Just in case the session's refcount reach 0 and is releasing, and
if we get the session without checking it, we may encounter kernel
crash.

Rename get_session to ceph_get_mds_session and make it global.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
---

Changed in V3:
- Clean all the local commit and pull it and rebased again, it is based
  the following commit:

  commit 3a1deab1d5c1bb693c268cc9b717c69554c3ca5e
  Author: Xiubo Li <xiubli@redhat.com>
  Date:   Wed Dec 4 06:57:39 2019 -0500

      ceph: add possible_max_rank and make the code more readable
        


 fs/ceph/mds_client.c | 16 ++++++++--------
 fs/ceph/mds_client.h |  9 ++-------
 2 files changed, 10 insertions(+), 15 deletions(-)

Comments

Ilya Dryomov Dec. 20, 2019, 9:11 a.m. UTC | #1 
On Fri, Dec 20, 2019 at 1:44 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> Just in case the session's refcount reach 0 and is releasing, and
> if we get the session without checking it, we may encounter kernel
> crash.
>
> Rename get_session to ceph_get_mds_session and make it global.
>
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>
> Changed in V3:
> - Clean all the local commit and pull it and rebased again, it is based
>   the following commit:
>
>   commit 3a1deab1d5c1bb693c268cc9b717c69554c3ca5e
>   Author: Xiubo Li <xiubli@redhat.com>
>   Date:   Wed Dec 4 06:57:39 2019 -0500
>
>       ceph: add possible_max_rank and make the code more readable

Hi Xiubo,

The base is correct, but the patch still appears to have been
corrupted, either by your email client or somewhere in transit.

Thanks,

                Ilya
Xiubo Li Dec. 20, 2019, 9:21 a.m. UTC | #2 
On 2019/12/20 17:11, Ilya Dryomov wrote:
> On Fri, Dec 20, 2019 at 1:44 AM <xiubli@redhat.com> wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Just in case the session's refcount reach 0 and is releasing, and
>> if we get the session without checking it, we may encounter kernel
>> crash.
>>
>> Rename get_session to ceph_get_mds_session and make it global.
>>
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>
>> Changed in V3:
>> - Clean all the local commit and pull it and rebased again, it is based
>>    the following commit:
>>
>>    commit 3a1deab1d5c1bb693c268cc9b717c69554c3ca5e
>>    Author: Xiubo Li <xiubli@redhat.com>
>>    Date:   Wed Dec 4 06:57:39 2019 -0500
>>
>>        ceph: add possible_max_rank and make the code more readable
> Hi Xiubo,
>
> The base is correct, but the patch still appears to have been
> corrupted, either by your email client or somewhere in transit.

Ah, I have no idea of this now, I was doing the following command to 
post it:

# git send-email --smtp-server=... --to=...

And my git version is:

# git --version
git version 2.21.0

I attached it or should I post it again ?

Thanks.


> Thanks,
>
>                  Ilya
>
From e19bb31fa74309f2b64c8294127392d719f5708e Mon Sep 17 00:00:00 2001
From: Xiubo Li <xiubli@redhat.com>
Date: Wed, 18 Dec 2019 06:49:37 -0500
Subject: [PATCH] ceph: rename get_session and switch to use
 ceph_get_mds_session

Just in case the session's refcount reach 0 and is releasing, and
if we get the session without checking it, we may encounter kernel
crash.

Rename get_session to ceph_get_mds_session and make it global.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/mds_client.c | 16 ++++++++--------
 fs/ceph/mds_client.h |  9 ++-------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index d8bb3eebfaeb..a64f9ccdc2ff 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -538,7 +538,7 @@ const char *ceph_session_state_name(int s)
 	}
 }
 
-static struct ceph_mds_session *get_session(struct ceph_mds_session *s)
+struct ceph_mds_session *ceph_get_mds_session(struct ceph_mds_session *s)
 {
 	if (refcount_inc_not_zero(&s->s_ref)) {
 		dout("mdsc get_session %p %d -> %d\n", s,
@@ -569,7 +569,7 @@ struct ceph_mds_session *__ceph_lookup_mds_session(struct ceph_mds_client *mdsc,
 {
 	if (mds >= mdsc->max_sessions || !mdsc->sessions[mds])
 		return NULL;
-	return get_session(mdsc->sessions[mds]);
+	return ceph_get_mds_session(mdsc->sessions[mds]);
 }
 
 static bool __have_session(struct ceph_mds_client *mdsc, int mds)
@@ -1990,7 +1990,7 @@ void ceph_flush_cap_releases(struct ceph_mds_client *mdsc,
 	if (mdsc->stopping)
 		return;
 
-	get_session(session);
+	ceph_get_mds_session(session);
 	if (queue_work(mdsc->fsc->cap_wq,
 		       &session->s_cap_release_work)) {
 		dout("cap release work queued\n");
@@ -2605,7 +2605,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 			goto finish;
 		}
 	}
-	req->r_session = get_session(session);
+	req->r_session = ceph_get_mds_session(session);
 
 	dout("do_request mds%d session %p state %s\n", mds, session,
 	     ceph_session_state_name(session->s_state));
@@ -3129,7 +3129,7 @@ static void handle_session(struct ceph_mds_session *session,
 
 	mutex_lock(&mdsc->mutex);
 	if (op == CEPH_SESSION_CLOSE) {
-		get_session(session);
+		ceph_get_mds_session(session);
 		__unregister_session(mdsc, session);
 	}
 	/* FIXME: this ttl calculation is generous */
@@ -3804,7 +3804,7 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 
 		if (i >= newmap->m_num_mds) {
 			/* force close session for stopped mds */
-			get_session(s);
+			ceph_get_mds_session(s);
 			__unregister_session(mdsc, s);
 			__wake_requests(mdsc, &s->s_waiting);
 			mutex_unlock(&mdsc->mutex);
@@ -4404,7 +4404,7 @@ void ceph_mdsc_close_sessions(struct ceph_mds_client *mdsc)
 	mutex_lock(&mdsc->mutex);
 	for (i = 0; i < mdsc->max_sessions; i++) {
 		if (mdsc->sessions[i]) {
-			session = get_session(mdsc->sessions[i]);
+			session = ceph_get_mds_session(mdsc->sessions[i]);
 			__unregister_session(mdsc, session);
 			mutex_unlock(&mdsc->mutex);
 			mutex_lock(&session->s_mutex);
@@ -4632,7 +4632,7 @@ static struct ceph_connection *con_get(struct ceph_connection *con)
 {
 	struct ceph_mds_session *s = con->private;
 
-	if (get_session(s)) {
+	if (ceph_get_mds_session(s)) {
 		dout("mdsc con_get %p ok (%d)\n", s, refcount_read(&s->s_ref));
 		return con;
 	}
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 9fb2063b0600..a7a94cf57150 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -443,15 +443,10 @@ extern const char *ceph_mds_op_name(int op);
 extern struct ceph_mds_session *
 __ceph_lookup_mds_session(struct ceph_mds_client *, int mds);
 
-static inline struct ceph_mds_session *
-ceph_get_mds_session(struct ceph_mds_session *s)
-{
-	refcount_inc(&s->s_ref);
-	return s;
-}
-
 extern const char *ceph_session_state_name(int s);
 
+extern struct ceph_mds_session *
+ceph_get_mds_session(struct ceph_mds_session *s);
 extern void ceph_put_mds_session(struct ceph_mds_session *s);
 
 extern int ceph_send_msg_mds(struct ceph_mds_client *mdsc,
Ilya Dryomov Dec. 20, 2019, 10:46 a.m. UTC | #3 
On Fri, Dec 20, 2019 at 10:21 AM Xiubo Li <xiubli@redhat.com> wrote:
>
> On 2019/12/20 17:11, Ilya Dryomov wrote:
> > On Fri, Dec 20, 2019 at 1:44 AM <xiubli@redhat.com> wrote:
> >> From: Xiubo Li <xiubli@redhat.com>
> >>
> >> Just in case the session's refcount reach 0 and is releasing, and
> >> if we get the session without checking it, we may encounter kernel
> >> crash.
> >>
> >> Rename get_session to ceph_get_mds_session and make it global.
> >>
> >> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> >> ---
> >>
> >> Changed in V3:
> >> - Clean all the local commit and pull it and rebased again, it is based
> >>    the following commit:
> >>
> >>    commit 3a1deab1d5c1bb693c268cc9b717c69554c3ca5e
> >>    Author: Xiubo Li <xiubli@redhat.com>
> >>    Date:   Wed Dec 4 06:57:39 2019 -0500
> >>
> >>        ceph: add possible_max_rank and make the code more readable
> > Hi Xiubo,
> >
> > The base is correct, but the patch still appears to have been
> > corrupted, either by your email client or somewhere in transit.
>
> Ah, I have no idea of this now, I was doing the following command to
> post it:
>
> # git send-email --smtp-server=... --to=...

Hrm, I've looked through my archives and the last non-mangled patch
I see from you is "[PATCH RFC] libceph: remove the useless monc check"
dated Oct 15.  If you are using the same send-email command as before
and haven't changed anything on your end, it's probably one of the
intermediate servers...

>
> And my git version is:
>
> # git --version
> git version 2.21.0
>
> I attached it or should I post it again ?

You attached the old version ;)  It's not mangled, but it doesn't
apply.

Jeff, are you getting Xiubo's patches intact?

Thanks,

                Ilya
Jeff Layton Dec. 20, 2019, 11:09 a.m. UTC | #4 
On Fri, 2019-12-20 at 11:46 +0100, Ilya Dryomov wrote:
> On Fri, Dec 20, 2019 at 10:21 AM Xiubo Li <xiubli@redhat.com> wrote:
> > On 2019/12/20 17:11, Ilya Dryomov wrote:
> > > On Fri, Dec 20, 2019 at 1:44 AM <xiubli@redhat.com> wrote:
> > > > From: Xiubo Li <xiubli@redhat.com>
> > > > 
> > > > Just in case the session's refcount reach 0 and is releasing, and
> > > > if we get the session without checking it, we may encounter kernel
> > > > crash.
> > > > 
> > > > Rename get_session to ceph_get_mds_session and make it global.
> > > > 
> > > > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > > > ---
> > > > 
> > > > Changed in V3:
> > > > - Clean all the local commit and pull it and rebased again, it is based
> > > >    the following commit:
> > > > 
> > > >    commit 3a1deab1d5c1bb693c268cc9b717c69554c3ca5e
> > > >    Author: Xiubo Li <xiubli@redhat.com>
> > > >    Date:   Wed Dec 4 06:57:39 2019 -0500
> > > > 
> > > >        ceph: add possible_max_rank and make the code more readable
> > > Hi Xiubo,
> > > 
> > > The base is correct, but the patch still appears to have been
> > > corrupted, either by your email client or somewhere in transit.
> > 
> > Ah, I have no idea of this now, I was doing the following command to
> > post it:
> > 
> > # git send-email --smtp-server=... --to=...
> 
> Hrm, I've looked through my archives and the last non-mangled patch
> I see from you is "[PATCH RFC] libceph: remove the useless monc check"
> dated Oct 15.  If you are using the same send-email command as before
> and haven't changed anything on your end, it's probably one of the
> intermediate servers...
> 
> > And my git version is:
> > 
> > # git --version
> > git version 2.21.0
> > 
> > I attached it or should I post it again ?
> 
> You attached the old version ;)  It's not mangled, but it doesn't
> apply.
> 
> Jeff, are you getting Xiubo's patches intact?
> 

Yep. This patch applied just fine using git-am. Patch looks reasonable
to me -- I like guarding against a 0->1 transition on a refcount.  I'll
go ahead and push it to testing.

Thanks,
Xiubo Li Dec. 20, 2019, 12:17 p.m. UTC | #5 
On 2019/12/20 19:09, Jeff Layton wrote:
> On Fri, 2019-12-20 at 11:46 +0100, Ilya Dryomov wrote:
>> On Fri, Dec 20, 2019 at 10:21 AM Xiubo Li <xiubli@redhat.com> wrote:
>>> On 2019/12/20 17:11, Ilya Dryomov wrote:
>>>> On Fri, Dec 20, 2019 at 1:44 AM <xiubli@redhat.com> wrote:
>>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>>
>>>>> Just in case the session's refcount reach 0 and is releasing, and
>>>>> if we get the session without checking it, we may encounter kernel
>>>>> crash.
>>>>>
>>>>> Rename get_session to ceph_get_mds_session and make it global.
>>>>>
>>>>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>>>>> ---
>>>>>
>>>>> Changed in V3:
>>>>> - Clean all the local commit and pull it and rebased again, it is based
>>>>>     the following commit:
>>>>>
>>>>>     commit 3a1deab1d5c1bb693c268cc9b717c69554c3ca5e
>>>>>     Author: Xiubo Li <xiubli@redhat.com>
>>>>>     Date:   Wed Dec 4 06:57:39 2019 -0500
>>>>>
>>>>>         ceph: add possible_max_rank and make the code more readable
>>>> Hi Xiubo,
>>>>
>>>> The base is correct, but the patch still appears to have been
>>>> corrupted, either by your email client or somewhere in transit.
>>> Ah, I have no idea of this now, I was doing the following command to
>>> post it:
>>>
>>> # git send-email --smtp-server=... --to=...
>> Hrm, I've looked through my archives and the last non-mangled patch
>> I see from you is "[PATCH RFC] libceph: remove the useless monc check"
>> dated Oct 15.  If you are using the same send-email command as before
>> and haven't changed anything on your end, it's probably one of the
>> intermediate servers...
>>
>>> And my git version is:
>>>
>>> # git --version
>>> git version 2.21.0
>>>
>>> I attached it or should I post it again ?
>> You attached the old version ;)  It's not mangled, but it doesn't
>> apply.
>>
>> Jeff, are you getting Xiubo's patches intact?
>>
> Yep. This patch applied just fine using git-am. Patch looks reasonable
> to me -- I like guarding against a 0->1 transition on a refcount.  I'll
> go ahead and push it to testing.

Cool.

Thanks.


> Thanks,
diff mbox series

Patch

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 4080067d1672..d6c3d8d854e0 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -538,7 +538,7 @@  const char *ceph_session_state_name(int s)
 	}
 }
 
-static struct ceph_mds_session *get_session(struct ceph_mds_session *s)
+struct ceph_mds_session *ceph_get_mds_session(struct ceph_mds_session *s)
 {
 	if (refcount_inc_not_zero(&s->s_ref)) {
 		dout("mdsc get_session %p %d -> %d\n", s,
@@ -569,7 +569,7 @@  struct ceph_mds_session *__ceph_lookup_mds_session(struct ceph_mds_client *mdsc,
 {
 	if (mds >= mdsc->max_sessions || !mdsc->sessions[mds])
 		return NULL;
-	return get_session(mdsc->sessions[mds]);
+	return ceph_get_mds_session(mdsc->sessions[mds]);
 }
 
 static bool __have_session(struct ceph_mds_client *mdsc, int mds)
@@ -1977,7 +1977,7 @@  void ceph_flush_cap_releases(struct ceph_mds_client *mdsc,
 	if (mdsc->stopping)
 		return;
 
-	get_session(session);
+	ceph_get_mds_session(session);
 	if (queue_work(mdsc->fsc->cap_wq,
 		       &session->s_cap_release_work)) {
 		dout("cap release work queued\n");
@@ -2613,7 +2613,7 @@  static void __do_request(struct ceph_mds_client *mdsc,
 			goto finish;
 		}
 	}
-	req->r_session = get_session(session);
+	req->r_session = ceph_get_mds_session(session);
 
 	dout("do_request mds%d session %p state %s\n", mds, session,
 	     ceph_session_state_name(session->s_state));
@@ -3135,7 +3135,7 @@  static void handle_session(struct ceph_mds_session *session,
 
 	mutex_lock(&mdsc->mutex);
 	if (op == CEPH_SESSION_CLOSE) {
-		get_session(session);
+		ceph_get_mds_session(session);
 		__unregister_session(mdsc, session);
 	}
 	/* FIXME: this ttl calculation is generous */
@@ -3797,7 +3797,7 @@  static void check_new_map(struct ceph_mds_client *mdsc,
 
 		if (i >= newmap->possible_max_rank) {
 			/* force close session for stopped mds */
-			get_session(s);
+			ceph_get_mds_session(s);
 			__unregister_session(mdsc, s);
 			__wake_requests(mdsc, &s->s_waiting);
 			mutex_unlock(&mdsc->mutex);
@@ -4398,7 +4398,7 @@  void ceph_mdsc_close_sessions(struct ceph_mds_client *mdsc)
 	mutex_lock(&mdsc->mutex);
 	for (i = 0; i < mdsc->max_sessions; i++) {
 		if (mdsc->sessions[i]) {
-			session = get_session(mdsc->sessions[i]);
+			session = ceph_get_mds_session(mdsc->sessions[i]);
 			__unregister_session(mdsc, session);
 			mutex_unlock(&mdsc->mutex);
 			mutex_lock(&session->s_mutex);
@@ -4626,7 +4626,7 @@  static struct ceph_connection *con_get(struct ceph_connection *con)
 {
 	struct ceph_mds_session *s = con->private;
 
-	if (get_session(s))
+	if (ceph_get_mds_session(s))
 		return con;
 	return NULL;
 }
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index fe085e06adf5..c021df5f50ce 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -452,15 +452,10 @@  extern const char *ceph_mds_op_name(int op);
 extern struct ceph_mds_session *
 __ceph_lookup_mds_session(struct ceph_mds_client *, int mds);
 
-static inline struct ceph_mds_session *
-ceph_get_mds_session(struct ceph_mds_session *s)
-{
-	refcount_inc(&s->s_ref);
-	return s;
-}
-
 extern const char *ceph_session_state_name(int s);
 
+extern struct ceph_mds_session *
+ceph_get_mds_session(struct ceph_mds_session *s);
 extern void ceph_put_mds_session(struct ceph_mds_session *s);
 
 extern int ceph_send_msg_mds(struct ceph_mds_client *mdsc,