Staging: lustre: Replace macros with static inline functions

PATCH Superseded Ksenija Stanojevic 2 participants 7 messages Oct 21, 2015 – Oct 25, 2015 Staging

lore

Share & tools

lore

Ksenija Stanojevic

author

2015-10-21 21:18 UTC #0 lore raw

cover Staging: lustre: Replace macros with static inline functions

Convert macros LPROCFS_CLIMP_CHECK and LPROCFS_CLIMP_EXIT into static inline functions

Signed-off-by Ksenija Stanojevic <[email protected]>

Ksenija Stanojevic (3):
Staging: lustre: Replace LPROCFS_CLIMP_CHECK with lprocfs_climp_check
Staging: lustre: Replace LPROCFS_CLIMP_EXIT with lprocfs_climp_exit
Staging: lustre: Remove unused macros

 .../staging/lustre/lustre/include/lprocfs_status.h | 13 ------
 drivers/staging/lustre/lustre/include/obd_class.h  | 16 +++++++
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |  8 +++-
 .../lustre/lustre/obdclass/lprocfs_status.c        | 50 ++++++++++++++++------
 .../staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c    | 22 +++++++---
 5 files changed, 74 insertions(+), 35 deletions(-)

Signature · 1 line

1.9.1

Ksenija Stanojevic

author

2015-10-21 21:19 UTC #1 lore raw

PATCH 1/3 Staging: lustre: Replace LPROCFS_CLIMP_CHECK with lprocfs_climp_check

Static inline functions are preferred over macros. The function is placed in obd_class.h instead lprocfs_status.h because obd_class.h includes header obd.h which contains definition of struct obd_device and in that way avoids build error: Dereferencing pointer to incomplete type.

Signed-off-by Ksenija Stanojevic <[email protected]>

---
 drivers/staging/lustre/lustre/include/obd_class.h  | 11 +++++++
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |  6 +++-
 .../lustre/lustre/obdclass/lprocfs_status.c        | 38 +++++++++++++++++-----
 .../staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c    | 16 +++++++--
 4 files changed, 59 insertions(+), 12 deletions(-)

drivers/staging/lustre/lustre/include/obd_class.h +11 -0

index bf2bde5..56c2096 100644
@@ -251,6 +251,17 @@ static inline enum obd_option exp_flags_from_obd(struct obd_device *obd)
 		0);
 }
 
+static inline int lprocfs_climp_check(struct obd_device *obd)
+{
+	typecheck(struct obd_device *, obd);
+	down_read(&(obd)->u.cli.cl_sem);
+	if (!(obd)->u.cli.cl_import) {
+		up_read(&(obd)->u.cli.cl_sem);
+		return -ENODEV;
+	}
+	return 0;
+}
+
 struct inode;
 struct lu_attr;
 struct obdo;

drivers/staging/lustre/lustre/mgc/mgc_request.c +5 -1

index 7315733..b81efcd 100644
@@ -440,8 +440,12 @@ int lprocfs_mgc_rd_ir_state(struct seq_file *m, void *data)
 	struct obd_import       *imp;
 	struct obd_connect_data *ocd;
 	struct config_llog_data *cld;
+	int rc;
+
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
 
-	LPROCFS_CLIMP_CHECK(obd);
 	imp = obd->u.cli.cl_import;
 	ocd = &imp->imp_connect_data;

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +30 -8

index 3228fee..4494096 100644
@@ -489,9 +489,13 @@ int lprocfs_rd_server_uuid(struct seq_file *m, void *data)
 	struct obd_device *obd = data;
 	struct obd_import *imp;
 	char *imp_state_name = NULL;
+	int rc;
 
 	LASSERT(obd != NULL);
-	LPROCFS_CLIMP_CHECK(obd);
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
+
 	imp = obd->u.cli.cl_import;
 	imp_state_name = ptlrpc_import_state_name(imp->imp_state);
 	seq_printf(m, "%s\t%s%s\n",

@@ -508,10 +512,14 @@ int lprocfs_rd_conn_uuid(struct seq_file *m, void *data)
 {
 	struct obd_device *obd = data;
 	struct ptlrpc_connection *conn;
+	int rc;
 
 	LASSERT(obd != NULL);
 
-	LPROCFS_CLIMP_CHECK(obd);
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
+
 	conn = obd->u.cli.cl_import->imp_connection;
 	if (conn && obd->u.cli.cl_import)
 		seq_printf(m, "%s\n", conn->c_remote_uuid.uuid);

@@ -618,9 +626,13 @@ int lprocfs_rd_import(struct seq_file *m, void *data)
 	int				j;
 	int				k;
 	int				rw	= 0;
+	int				rc;
 
 	LASSERT(obd != NULL);
-	LPROCFS_CLIMP_CHECK(obd);
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
+
 	imp = obd->u.cli.cl_import;
 
 	seq_printf(m,

@@ -759,10 +771,13 @@ int lprocfs_rd_state(struct seq_file *m, void *data)
 {
 	struct obd_device *obd = data;
 	struct obd_import *imp;
-	int j, k;
+	int j, k, rc;
 
 	LASSERT(obd != NULL);
-	LPROCFS_CLIMP_CHECK(obd);
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
+
 	imp = obd->u.cli.cl_import;
 
 	seq_printf(m, "current_state: %s\n",

@@ -802,10 +817,13 @@ int lprocfs_rd_timeouts(struct seq_file *m, void *data)
 	unsigned int cur, worst;
 	time64_t now, worstt;
 	struct dhms ts;
-	int i;
+	int i, rc;
 
 	LASSERT(obd != NULL);
-	LPROCFS_CLIMP_CHECK(obd);
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
+
 	imp = obd->u.cli.cl_import;
 
 	now = ktime_get_real_seconds();

@@ -845,8 +863,12 @@ int lprocfs_rd_connect_flags(struct seq_file *m, void *data)
 {
 	struct obd_device *obd = data;
 	__u64 flags;
+	int rc;
+
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
 
-	LPROCFS_CLIMP_CHECK(obd);
 	flags = obd->u.cli.cl_import->imp_connect_data.ocd_connect_flags;
 	seq_printf(m, "flags=%#llx\n", flags);
 	obd_connect_seq_flags2str(m, flags, "\n");

drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c +13 -3

index 415817c..33184c7 100644
@@ -1189,7 +1189,10 @@ int lprocfs_wr_ping(struct file *file, const char __user *buffer,
 	struct ptlrpc_request *req;
 	int rc;
 
-	LPROCFS_CLIMP_CHECK(obd);
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
+
 	req = ptlrpc_prep_ping(obd->u.cli.cl_import);
 	LPROCFS_CLIMP_EXIT(obd);
 	if (req == NULL)

@@ -1278,8 +1281,12 @@ int lprocfs_rd_pinger_recov(struct seq_file *m, void *n)
 {
 	struct obd_device *obd = m->private;
 	struct obd_import *imp = obd->u.cli.cl_import;
+	int rc;
+
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
 
-	LPROCFS_CLIMP_CHECK(obd);
 	seq_printf(m, "%d\n", !imp->imp_no_pinger_recover);
 	LPROCFS_CLIMP_EXIT(obd);

@@ -1302,7 +1309,10 @@ int lprocfs_wr_pinger_recov(struct file *file, const char __user *buffer,
 	if (val != 0 && val != 1)
 		return -ERANGE;
 
-	LPROCFS_CLIMP_CHECK(obd);
+	rc = lprocfs_climp_check(obd);
+	if (rc)
+		return rc;
+
 	spin_lock(&imp->imp_lock);
 	imp->imp_no_pinger_recover = !val;
 	spin_unlock(&imp->imp_lock);

Signature · 1 line

1.9.1

Greg KH

maintainer

2015-10-25 08:28 UTC #2 lore raw

Re: [Outreachy kernel] [PATCH 1/3] Staging: lustre: Replace LPROCFS_CLIMP_CHECK with lprocfs_climp_check

Quoted 6 lines · Ksenija Stanojevic

Static inline functions are preferred over macros. The function is placed in obd_class.h instead lprocfs_status.h because obd_class.h includes header obd.h which contains definition of struct obd_device and in that way avoids build error: Dereferencing pointer to incomplete type.

Quoted 7 lines

Signed-off-by Ksenija Stanojevic <[email protected]>

---
 drivers/staging/lustre/lustre/include/obd_class.h  | 11 +++++++
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |  6 +++-
 .../lustre/lustre/obdclass/lprocfs_status.c        | 38 +++++++++++++++++-----
 .../staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c    | 16 +++++++--
 4 files changed, 59 insertions(+), 12 deletions(-)

Why haven't you deleted the macro as well? It's hard to validate this patch is correct without seeing the macro removed.

thanks,

greg k-h

Ksenija Stanojevic

author

2015-10-21 21:20 UTC #3 lore raw

PATCH 2/3 Staging: lustre: Replace LPROCFS_CLIMP_EXIT with lprocfs_climp_exit

In reply to Ksenija Stanojevic

Signed-off-by Ksenija Stanojevic <[email protected]>

---
 drivers/staging/lustre/lustre/include/obd_class.h       |  5 +++++
 drivers/staging/lustre/lustre/mgc/mgc_request.c         |  2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 12 ++++++------
 drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c     |  6 +++---
 4 files changed, 15 insertions(+), 10 deletions(-)

drivers/staging/lustre/lustre/include/obd_class.h +5 -0

index 56c2096..ad15a10 100644
@@ -262,6 +262,11 @@ static inline int lprocfs_climp_check(struct obd_device *obd)
 	return 0;
 }
 
+static inline void lprocfs_climp_exit(struct obd_device *obd)
+{
+	up_read(&(obd)->u.cli.cl_sem);
+}
+
 struct inode;
 struct lu_attr;
 struct obdo;

drivers/staging/lustre/lustre/mgc/mgc_request.c +1 -1

index b81efcd..82faabc 100644
@@ -463,7 +463,7 @@ int lprocfs_mgc_rd_ir_state(struct seq_file *m, void *data)
 	}
 	spin_unlock(&config_list_lock);
 
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 	return 0;
 }

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +6 -6

index 4494096..a096fac 100644
@@ -502,7 +502,7 @@ int lprocfs_rd_server_uuid(struct seq_file *m, void *data)
 		   obd2cli_tgt(obd), imp_state_name,
 		   imp->imp_deactive ? "\tDEACTIVATED" : "");
 
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 
 	return 0;
 }

@@ -526,7 +526,7 @@ int lprocfs_rd_conn_uuid(struct seq_file *m, void *data)
 	else
 		seq_puts(m, "<none>\n");
 
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 
 	return 0;
 }

@@ -762,7 +762,7 @@ int lprocfs_rd_import(struct seq_file *m, void *data)
 	}
 
 out_climp:
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 	return 0;
 }
 EXPORT_SYMBOL(lprocfs_rd_import);

@@ -793,7 +793,7 @@ int lprocfs_rd_state(struct seq_file *m, void *data)
 			   ptlrpc_import_state_name(ish->ish_state));
 	}
 
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 	return 0;
 }
 EXPORT_SYMBOL(lprocfs_rd_state);

@@ -854,7 +854,7 @@ int lprocfs_rd_timeouts(struct seq_file *m, void *data)
 		lprocfs_at_hist_helper(m, &imp->imp_at.iat_service_estimate[i]);
 	}
 
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 	return 0;
 }
 EXPORT_SYMBOL(lprocfs_rd_timeouts);

@@ -873,7 +873,7 @@ int lprocfs_rd_connect_flags(struct seq_file *m, void *data)
 	seq_printf(m, "flags=%#llx\n", flags);
 	obd_connect_seq_flags2str(m, flags, "\n");
 	seq_printf(m, "\n");
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 	return 0;
 }
 EXPORT_SYMBOL(lprocfs_rd_connect_flags);

drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c +3 -3

index 33184c7..b431c3b 100644
@@ -1194,7 +1194,7 @@ int lprocfs_wr_ping(struct file *file, const char __user *buffer,
 		return rc;
 
 	req = ptlrpc_prep_ping(obd->u.cli.cl_import);
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 	if (req == NULL)
 		return -ENOMEM;

@@ -1288,7 +1288,7 @@ int lprocfs_rd_pinger_recov(struct seq_file *m, void *n)
 		return rc;
 
 	seq_printf(m, "%d\n", !imp->imp_no_pinger_recover);
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 
 	return 0;
 }

@@ -1316,7 +1316,7 @@ int lprocfs_wr_pinger_recov(struct file *file, const char __user *buffer,
 	spin_lock(&imp->imp_lock);
 	imp->imp_no_pinger_recover = !val;
 	spin_unlock(&imp->imp_lock);
-	LPROCFS_CLIMP_EXIT(obd);
+	lprocfs_climp_exit(obd);
 
 	return count;

Signature · 1 line

1.9.1

Greg KH

maintainer

2015-10-25 08:28 UTC #4 lore raw

Re: [Outreachy kernel] [PATCH 2/3] Staging: lustre: Replace LPROCFS_CLIMP_EXIT with lprocfs_climp_exit

Quoted 19 lines · Ksenija Stanojevic

Static inline functions are preferred over macros. The function is placed in obd_class.h instead lprocfs_status.h because obd_class.h includes header obd.h which contains definition of struct obd_device and in that way avoids build error: Dereferencing pointer to incomplete type.
Signed-off-by Ksenija Stanojevic <[email protected]>
---
 drivers/staging/lustre/lustre/include/obd_class.h       |  5 +++++
 drivers/staging/lustre/lustre/mgc/mgc_request.c         |  2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 12 ++++++------
 drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c     |  6 +++---
 4 files changed, 15 insertions(+), 10 deletions(-)
drivers/staging/lustre/lustre/include/obd_class.h +0 -0
index 56c2096..ad15a10 100644

drivers/staging/lustre/lustre/include/obd_class.h static inline int lprocfs_climp_check(struct obd_device *obd) · line 268

Quoted 7 lines

drivers/staging/lustre/lustre/include/obd_class.h +4 -0

@@ -262,6 +262,11 @@ static inline int lprocfs_climp_check(struct obd_device *obd)
 	return 0;
 }
 
+static inline void lprocfs_climp_exit(struct obd_device *obd)
+{
+	up_read(&(obd)->u.cli.cl_sem);
+}

Why not just use up_read() everywhere instead? You'll just be removing this wrapper function in a later patch anyway :)

thanks,

greg k-h

Ksenija Stanojevic

author

2015-10-21 21:22 UTC #5 lore raw

PATCH 3/3 Staging: lustre: Remove unused macros

In reply to Ksenija Stanojevic

Macros LPROCFS_CLIMP_CHECK and LPROCFS_CLIMP_EXIT are not used anymore, therefore remove them along with the comment.

Signed-off-by Ksenija Stanojevic <[email protected]>

---
 drivers/staging/lustre/lustre/include/lprocfs_status.h | 13 -------------
 1 file changed, 13 deletions(-)

drivers/staging/lustre/lustre/include/lprocfs_status.h +0 ~~-13~~

index 6e41129..532d195 100644
@@ -626,19 +626,6 @@ void lprocfs_stats_collect(struct lprocfs_stats *stats, int idx,
 int lprocfs_single_release(struct inode *, struct file *);
 int lprocfs_seq_release(struct inode *, struct file *);
 
-/* You must use these macros when you want to refer to
- * the import in a client obd_device for a lprocfs entry */
-#define LPROCFS_CLIMP_CHECK(obd) do {	   \
-	typecheck(struct obd_device *, obd);    \
-	down_read(&(obd)->u.cli.cl_sem);    \
-	if ((obd)->u.cli.cl_import == NULL) {   \
-	     up_read(&(obd)->u.cli.cl_sem); \
-	     return -ENODEV;		    \
-	}				       \
-} while (0)
-#define LPROCFS_CLIMP_EXIT(obd)		 \
-	up_read(&(obd)->u.cli.cl_sem)
-
 /* write the name##_seq_show function, call LPROC_SEQ_FOPS_RO for read-only
   proc entries; otherwise, you will define name##_seq_write function also for
   a read-write proc entry, and then call LPROC_SEQ_SEQ instead. Finally,

Signature · 1 line

1.9.1

Greg KH

maintainer

2015-10-25 08:28 UTC #6 lore raw

Re: [Outreachy kernel] [PATCH 3/3] Staging: lustre: Remove unused macros

Quoted 13 lines · Ksenija Stanojevic

Macros LPROCFS_CLIMP_CHECK and LPROCFS_CLIMP_EXIT are not used anymore, therefore remove them along with the comment.
Signed-off-by Ksenija Stanojevic <[email protected]>
---
 drivers/staging/lustre/lustre/include/lprocfs_status.h | 13 -------------
 1 file changed, 13 deletions(-)
drivers/staging/lustre/lustre/include/lprocfs_status.h +0 -0
index 6e41129..532d195 100644

drivers/staging/lustre/lustre/include/lprocfs_status.h void lprocfs_stats_collect(struct lprocfs_stats *stats, int idx, · line 632

Quoted 7 lines

drivers/staging/lustre/lustre/include/lprocfs_status.h +0 -4

@@ -626,19 +626,6 @@ void lprocfs_stats_collect(struct lprocfs_stats *stats, int idx,
 int lprocfs_single_release(struct inode *, struct file *);
 int lprocfs_seq_release(struct inode *, struct file *);
 
-/* You must use these macros when you want to refer to
- * the import in a client obd_device for a lprocfs entry */
-#define LPROCFS_CLIMP_CHECK(obd) do {	   \
-	typecheck(struct obd_device *, obd);    \

Note, when you turn this into an inline function, you don't need the typecheck() call anymore, right?

thanks,

greg k-h