diff mbox series

[1/5] ceph: export ceph_create_session_msg

Message ID 20210629044241.30359-2-xiubli@redhat.com
State New
Headers show
Series [1/5] ceph: export ceph_create_session_msg | expand

Commit Message

Xiubo Li June 29, 2021, 4:42 a.m. UTC 
From: Xiubo Li <xiubli@redhat.com>

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

Comments

Ilya Dryomov June 30, 2021, 12:17 p.m. UTC | #1 
On Tue, Jun 29, 2021 at 3:27 PM Xiubo Li <xiubli@redhat.com> wrote:
>

>

> On 6/29/21 9:12 PM, Jeff Layton wrote:

> > On Tue, 2021-06-29 at 12:42 +0800, xiubli@redhat.com wrote:

> >> From: Xiubo Li <xiubli@redhat.com>

> >>

> > nit: the subject of this patch is not quite right. You aren't exporting

> > it here, just making it a global symbol (within ceph.ko).

> >

>

> Yeah, will fix it.

>

>

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

> >> ---

> >>   fs/ceph/mds_client.c | 15 ++++++++-------

> >>   fs/ceph/mds_client.h |  1 +

> >>   2 files changed, 9 insertions(+), 7 deletions(-)

> >>

> >> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c

> >> index 2d7dcd295bb9..e49d3e230712 100644

> >> --- a/fs/ceph/mds_client.c

> >> +++ b/fs/ceph/mds_client.c

> >> @@ -1150,7 +1150,7 @@ static int __choose_mds(struct ceph_mds_client *mdsc,

> >>   /*

> >>    * session messages

> >>    */

> >> -static struct ceph_msg *create_session_msg(u32 op, u64 seq)

> >> +struct ceph_msg *ceph_create_session_msg(u32 op, u64 seq)

> >>   {

> >>      struct ceph_msg *msg;

> >>      struct ceph_mds_session_head *h;

> >> @@ -1158,7 +1158,7 @@ static struct ceph_msg *create_session_msg(u32 op, u64 seq)

> >>      msg = ceph_msg_new(CEPH_MSG_CLIENT_SESSION, sizeof(*h), GFP_NOFS,

> >>                         false);

> >>      if (!msg) {

> >> -            pr_err("create_session_msg ENOMEM creating msg\n");

> >> +            pr_err("ceph_create_session_msg ENOMEM creating msg\n");

> > instead of hardcoding the function names in these error messages, use

> > __func__ instead? That makes it easier to keep up with code changes.

> >

> >       pr_err("%s ENOMEM creating msg\n", __func__);

>

> Sure, will fix this too.


I think this can be just "ENOMEM creating session msg" without the
function name to avoid repetition.

Thanks,

                Ilya
Xiubo Li July 1, 2021, 1:50 a.m. UTC | #2 
On 6/30/21 8:17 PM, Ilya Dryomov wrote:
> On Tue, Jun 29, 2021 at 3:27 PM Xiubo Li <xiubli@redhat.com> wrote:

>>

>> On 6/29/21 9:12 PM, Jeff Layton wrote:

>>> On Tue, 2021-06-29 at 12:42 +0800, xiubli@redhat.com wrote:

>>>> From: Xiubo Li <xiubli@redhat.com>

>>>>

>>> nit: the subject of this patch is not quite right. You aren't exporting

>>> it here, just making it a global symbol (within ceph.ko).

>>>

>> Yeah, will fix it.

>>

>>

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

>>>> ---

>>>>    fs/ceph/mds_client.c | 15 ++++++++-------

>>>>    fs/ceph/mds_client.h |  1 +

>>>>    2 files changed, 9 insertions(+), 7 deletions(-)

>>>>

>>>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c

>>>> index 2d7dcd295bb9..e49d3e230712 100644

>>>> --- a/fs/ceph/mds_client.c

>>>> +++ b/fs/ceph/mds_client.c

>>>> @@ -1150,7 +1150,7 @@ static int __choose_mds(struct ceph_mds_client *mdsc,

>>>>    /*

>>>>     * session messages

>>>>     */

>>>> -static struct ceph_msg *create_session_msg(u32 op, u64 seq)

>>>> +struct ceph_msg *ceph_create_session_msg(u32 op, u64 seq)

>>>>    {

>>>>       struct ceph_msg *msg;

>>>>       struct ceph_mds_session_head *h;

>>>> @@ -1158,7 +1158,7 @@ static struct ceph_msg *create_session_msg(u32 op, u64 seq)

>>>>       msg = ceph_msg_new(CEPH_MSG_CLIENT_SESSION, sizeof(*h), GFP_NOFS,

>>>>                          false);

>>>>       if (!msg) {

>>>> -            pr_err("create_session_msg ENOMEM creating msg\n");

>>>> +            pr_err("ceph_create_session_msg ENOMEM creating msg\n");

>>> instead of hardcoding the function names in these error messages, use

>>> __func__ instead? That makes it easier to keep up with code changes.

>>>

>>>        pr_err("%s ENOMEM creating msg\n", __func__);

>> Sure, will fix this too.

> I think this can be just "ENOMEM creating session msg" without the

> function name to avoid repetition.


Will fix it by using:

pr_err("ENOMEM creating session %s msg", ceph_session_op_name(op));

Thanks.

>

> Thanks,

>

>                  Ilya

>
diff mbox series

Patch

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 2d7dcd295bb9..e49d3e230712 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1150,7 +1150,7 @@  static int __choose_mds(struct ceph_mds_client *mdsc,
 /*
  * session messages
  */
-static struct ceph_msg *create_session_msg(u32 op, u64 seq)
+struct ceph_msg *ceph_create_session_msg(u32 op, u64 seq)
 {
 	struct ceph_msg *msg;
 	struct ceph_mds_session_head *h;
@@ -1158,7 +1158,7 @@  static struct ceph_msg *create_session_msg(u32 op, u64 seq)
 	msg = ceph_msg_new(CEPH_MSG_CLIENT_SESSION, sizeof(*h), GFP_NOFS,
 			   false);
 	if (!msg) {
-		pr_err("create_session_msg ENOMEM creating msg\n");
+		pr_err("ceph_create_session_msg ENOMEM creating msg\n");
 		return NULL;
 	}
 	h = msg->front.iov_base;
@@ -1289,7 +1289,7 @@  static struct ceph_msg *create_session_open_msg(struct ceph_mds_client *mdsc, u6
 	msg = ceph_msg_new(CEPH_MSG_CLIENT_SESSION, sizeof(*h) + extra_bytes,
 			   GFP_NOFS, false);
 	if (!msg) {
-		pr_err("create_session_msg ENOMEM creating msg\n");
+		pr_err("ceph_create_session_msg ENOMEM creating msg\n");
 		return ERR_PTR(-ENOMEM);
 	}
 	p = msg->front.iov_base;
@@ -1801,8 +1801,8 @@  static int send_renew_caps(struct ceph_mds_client *mdsc,
 
 	dout("send_renew_caps to mds%d (%s)\n", session->s_mds,
 		ceph_mds_state_name(state));
-	msg = create_session_msg(CEPH_SESSION_REQUEST_RENEWCAPS,
-				 ++session->s_renew_seq);
+	msg = ceph_create_session_msg(CEPH_SESSION_REQUEST_RENEWCAPS,
+				      ++session->s_renew_seq);
 	if (!msg)
 		return -ENOMEM;
 	ceph_con_send(&session->s_con, msg);
@@ -1816,7 +1816,7 @@  static int send_flushmsg_ack(struct ceph_mds_client *mdsc,
 
 	dout("send_flushmsg_ack to mds%d (%s)s seq %lld\n",
 	     session->s_mds, ceph_session_state_name(session->s_state), seq);
-	msg = create_session_msg(CEPH_SESSION_FLUSHMSG_ACK, seq);
+	msg = ceph_create_session_msg(CEPH_SESSION_FLUSHMSG_ACK, seq);
 	if (!msg)
 		return -ENOMEM;
 	ceph_con_send(&session->s_con, msg);
@@ -1868,7 +1868,8 @@  static int request_close_session(struct ceph_mds_session *session)
 	dout("request_close_session mds%d state %s seq %lld\n",
 	     session->s_mds, ceph_session_state_name(session->s_state),
 	     session->s_seq);
-	msg = create_session_msg(CEPH_SESSION_REQUEST_CLOSE, session->s_seq);
+	msg = ceph_create_session_msg(CEPH_SESSION_REQUEST_CLOSE,
+				      session->s_seq);
 	if (!msg)
 		return -ENOMEM;
 	ceph_con_send(&session->s_con, msg);
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index bf99c5ba47fc..bf2683f0ba43 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -523,6 +523,7 @@  static inline void ceph_mdsc_put_request(struct ceph_mds_request *req)
 	kref_put(&req->r_kref, ceph_mdsc_release_request);
 }
 
+extern struct ceph_msg *ceph_create_session_msg(u32 op, u64 seq);
 extern void __ceph_queue_cap_release(struct ceph_mds_session *session,
 				    struct ceph_cap *cap);
 extern void ceph_flush_cap_releases(struct ceph_mds_client *mdsc,