All of lore.kernel.org
 help / color / mirror / Atom feed
* [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups
@ 2018-06-27 19:38 James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 01/13] lustre: libcfs: move tracefile locking from linux-tracefile.c to tracefile.c James Simmons
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

The first three patches in this series are a repost of Neil's patches.
I updated the 4th patch but it contained to many changes so I broke
it into 6 smaller patches for easier review. It address all the
brokeness of cfs_print_to_console() which shows up in my testing.
Besides the fixing more cleanups of tracefile.h was done in the
new patches. The rest of the patches are Neil's work rebased due
the changes I introduced.

James Simmons (6):
  lustre: libcfs: fix cfs_print_to_console()
  lustre: libcfs: properly handle failure paths in cfs_tracefile_init_arch()
  lustre: libcfs: remove cfs_trace_refill_stack()
  lustre: libcfs: move cfs_trace_data data to tracefile.c
  lustre: libcfs: cleanup tracefile.h
  lustre: libcfs: format macros in tracefile.h

NeilBrown (7):
  lustre: libcfs: move tracefile locking from linux-tracefile.c to tracefile.c
  lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb()
  lustre: libcfs: move tcd locking across to tracefile.c
  lustre: libcfs: merge linux-tracefile.c into tracefile.c
  lustre: libcfs: fold cfs_tracefile_*_arch into their only callers.
  lustre: libcfs: renamed CFS_TCD_TYPE_MAX to CFS_TCD_TYPE_CNT
  lustre: libcfs: discard TCD_MAX_TYPES

-------------
Changelog

1) Initial patches from Neil Brown

2) Neil's patches pushed with the 4th patch heavly updated.

3) Broke up the 4th patch I pushed into smaller patches for
   easier review.

 drivers/staging/lustre/lnet/libcfs/Makefile        |   2 +-
 drivers/staging/lustre/lnet/libcfs/debug.c         |   6 +-
 .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 258 --------------------
 drivers/staging/lustre/lnet/libcfs/tracefile.c     | 267 +++++++++++++++++----
 drivers/staging/lustre/lnet/libcfs/tracefile.h     | 117 +--------
 5 files changed, 237 insertions(+), 413 deletions(-)
 delete mode 100644 drivers/staging/lustre/lnet/libcfs/linux-tracefile.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 01/13] lustre: libcfs: move tracefile locking from linux-tracefile.c to tracefile.c
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb() James Simmons
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

From: NeilBrown <neilb@suse.com>

There is no value in keeping it separate.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 22 -------------------
 drivers/staging/lustre/lnet/libcfs/tracefile.c     | 25 +++++++++++-----------
 drivers/staging/lustre/lnet/libcfs/tracefile.h     |  5 -----
 3 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
index 3471384..9e72220 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
@@ -47,8 +47,6 @@
 
 char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
 
-static DECLARE_RWSEM(cfs_tracefile_sem);
-
 int cfs_tracefile_init_arch(void)
 {
 	int i;
@@ -109,26 +107,6 @@ void cfs_tracefile_fini_arch(void)
 	}
 }
 
-void cfs_tracefile_read_lock(void)
-{
-	down_read(&cfs_tracefile_sem);
-}
-
-void cfs_tracefile_read_unlock(void)
-{
-	up_read(&cfs_tracefile_sem);
-}
-
-void cfs_tracefile_write_lock(void)
-{
-	down_write(&cfs_tracefile_sem);
-}
-
-void cfs_tracefile_write_unlock(void)
-{
-	up_write(&cfs_tracefile_sem);
-}
-
 enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
 {
 	if (in_irq())
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 7ca562e..5f31933 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -59,6 +59,7 @@
 static int thread_running;
 
 static atomic_t cfs_tage_allocated = ATOMIC_INIT(0);
+static DECLARE_RWSEM(cfs_tracefile_sem);
 
 struct page_collection {
 	struct list_head	pc_pages;
@@ -711,7 +712,7 @@ int cfs_tracefile_dump_all_pages(char *filename)
 	mm_segment_t __oldfs;
 	int rc;
 
-	cfs_tracefile_write_lock();
+	down_write(&cfs_tracefile_sem);
 
 	filp = filp_open(filename, O_CREAT | O_EXCL | O_WRONLY | O_LARGEFILE,
 			 0600);
@@ -759,7 +760,7 @@ int cfs_tracefile_dump_all_pages(char *filename)
 close:
 	filp_close(filp, NULL);
 out:
-	cfs_tracefile_write_unlock();
+	up_write(&cfs_tracefile_sem);
 	return rc;
 }
 
@@ -873,12 +874,12 @@ int cfs_trace_daemon_command(char *str)
 {
 	int rc = 0;
 
-	cfs_tracefile_write_lock();
+	down_write(&cfs_tracefile_sem);
 
 	if (!strcmp(str, "stop")) {
-		cfs_tracefile_write_unlock();
+		up_write(&cfs_tracefile_sem);
 		cfs_trace_stop_thread();
-		cfs_tracefile_write_lock();
+		down_write(&cfs_tracefile_sem);
 		memset(cfs_tracefile, 0, sizeof(cfs_tracefile));
 
 	} else if (!strncmp(str, "size=", 5)) {
@@ -905,7 +906,7 @@ int cfs_trace_daemon_command(char *str)
 		cfs_trace_start_thread();
 	}
 
-	cfs_tracefile_write_unlock();
+	up_write(&cfs_tracefile_sem);
 	return rc;
 }
 
@@ -950,12 +951,12 @@ int cfs_trace_set_debug_mb(int mb)
 	mb /= num_possible_cpus();
 	pages = mb << (20 - PAGE_SHIFT);
 
-	cfs_tracefile_write_lock();
+	down_write(&cfs_tracefile_sem);
 
 	cfs_tcd_for_each(tcd, i, j)
 		tcd->tcd_max_pages = (pages * tcd->tcd_pages_factor) / 100;
 
-	cfs_tracefile_write_unlock();
+	up_write(&cfs_tracefile_sem);
 
 	return 0;
 }
@@ -967,12 +968,12 @@ int cfs_trace_get_debug_mb(void)
 	struct cfs_trace_cpu_data *tcd;
 	int total_pages = 0;
 
-	cfs_tracefile_read_lock();
+	down_read(&cfs_tracefile_sem);
 
 	cfs_tcd_for_each(tcd, i, j)
 		total_pages += tcd->tcd_max_pages;
 
-	cfs_tracefile_read_unlock();
+	up_read(&cfs_tracefile_sem);
 
 	return (total_pages >> (20 - PAGE_SHIFT)) + 1;
 }
@@ -1002,7 +1003,7 @@ static int tracefiled(void *arg)
 			goto end_loop;
 
 		filp = NULL;
-		cfs_tracefile_read_lock();
+		down_read(&cfs_tracefile_sem);
 		if (cfs_tracefile[0]) {
 			filp = filp_open(cfs_tracefile,
 					 O_CREAT | O_RDWR | O_LARGEFILE,
@@ -1014,7 +1015,7 @@ static int tracefiled(void *arg)
 					rc);
 			}
 		}
-		cfs_tracefile_read_unlock();
+		up_read(&cfs_tracefile_sem);
 		if (!filp) {
 			put_pages_on_daemon_list(&pc);
 			__LASSERT(list_empty(&pc.pc_pages));
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index 0608240..9f6b73d 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -65,11 +65,6 @@ enum cfs_trace_buf_type {
 int  cfs_tracefile_init_arch(void);
 void cfs_tracefile_fini_arch(void);
 
-void cfs_tracefile_read_lock(void);
-void cfs_tracefile_read_unlock(void);
-void cfs_tracefile_write_lock(void);
-void cfs_tracefile_write_unlock(void);
-
 int cfs_tracefile_dump_all_pages(char *filename);
 void cfs_trace_debug_print(void);
 void cfs_trace_flush_pages(void);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb()
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 01/13] lustre: libcfs: move tracefile locking from linux-tracefile.c to tracefile.c James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-28 22:39   ` Andreas Dilger
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 03/13] lustre: libcfs: move tcd locking across to tracefile.c James Simmons
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

From: NeilBrown <neilb@suse.com>

This function in used twice.
In libcfs_debug_init() the usage is pointless as
the only place that set libcfs_debug_mb ensures
that it does not exceed the maximum.  So checking
again can never help.

So open-code the small function into the only
other place that it is used - in cfs_trace_set_debug_mb(),
which is used to set libcfs_debug_mb.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/libcfs/debug.c           | 6 +++---
 drivers/staging/lustre/lnet/libcfs/linux-tracefile.c | 7 -------
 drivers/staging/lustre/lnet/libcfs/tracefile.c       | 3 ++-
 drivers/staging/lustre/lnet/libcfs/tracefile.h       | 1 -
 4 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/debug.c b/drivers/staging/lustre/lnet/libcfs/debug.c
index 06f694f..71effcf 100644
--- a/drivers/staging/lustre/lnet/libcfs/debug.c
+++ b/drivers/staging/lustre/lnet/libcfs/debug.c
@@ -411,10 +411,10 @@ int libcfs_debug_init(unsigned long bufsize)
 			sizeof(libcfs_debug_file_path_arr));
 	}
 
-	/* If libcfs_debug_mb is set to an invalid value or uninitialized
-	 * then just make the total buffers smp_num_cpus * TCD_MAX_PAGES
+	/* If libcfs_debug_mb is uninitialized then just make the
+	 * total buffers smp_num_cpus * TCD_MAX_PAGES
 	 */
-	if (max > cfs_trace_max_debug_mb() || max < num_possible_cpus()) {
+	if (max < num_possible_cpus()) {
 		max = TCD_MAX_PAGES;
 	} else {
 		max = max / num_possible_cpus();
diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
index 9e72220..64a5bc1 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
@@ -227,10 +227,3 @@ void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
 			hdr->ph_line_num, fn, len, buf);
 	}
 }
-
-int cfs_trace_max_debug_mb(void)
-{
-	int  total_mb = (totalram_pages >> (20 - PAGE_SHIFT));
-
-	return max(512, (total_mb * 80) / 100);
-}
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 5f31933..72321ce 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -933,7 +933,8 @@ int cfs_trace_set_debug_mb(int mb)
 	int i;
 	int j;
 	int pages;
-	int limit = cfs_trace_max_debug_mb();
+	int total_mb = (totalram_pages >> (20 - PAGE_SHIFT));
+	int limit = max(512, (total_mb * 80) / 100);
 	struct cfs_trace_cpu_data *tcd;
 
 	if (mb < num_possible_cpus()) {
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index 9f6b73d..bd1a1ef 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -88,7 +88,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 void libcfs_register_panic_notifier(void);
 void libcfs_unregister_panic_notifier(void);
 extern int libcfs_panic_in_progress;
-int cfs_trace_max_debug_mb(void);
 
 #define TCD_MAX_PAGES (5 << (20 - PAGE_SHIFT))
 #define TCD_STOCK_PAGES (TCD_MAX_PAGES)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 03/13] lustre: libcfs: move tcd locking across to tracefile.c
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 01/13] lustre: libcfs: move tracefile locking from linux-tracefile.c to tracefile.c James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb() James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console() James Simmons
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

From: NeilBrown <neilb@suse.com>

No need to have this in linux-tracefile.c

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 35 -------------
 drivers/staging/lustre/lnet/libcfs/tracefile.c     | 59 ++++++++++++++++++++++
 drivers/staging/lustre/lnet/libcfs/tracefile.h     | 28 ----------
 3 files changed, 59 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
index 64a5bc1..3af7722 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
@@ -116,41 +116,6 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
 	return CFS_TCD_TYPE_PROC;
 }
 
-/*
- * The walking argument indicates the locking comes from all tcd types
- * iterator and we must lock it and dissable local irqs to avoid deadlocks
- * with other interrupt locks that might be happening. See LU-1311
- * for details.
- */
-int cfs_trace_lock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
-	__acquires(&tcd->tc_lock)
-{
-	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_MAX);
-	if (tcd->tcd_type == CFS_TCD_TYPE_IRQ)
-		spin_lock_irqsave(&tcd->tcd_lock, tcd->tcd_lock_flags);
-	else if (tcd->tcd_type == CFS_TCD_TYPE_SOFTIRQ)
-		spin_lock_bh(&tcd->tcd_lock);
-	else if (unlikely(walking))
-		spin_lock_irq(&tcd->tcd_lock);
-	else
-		spin_lock(&tcd->tcd_lock);
-	return 1;
-}
-
-void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
-	__releases(&tcd->tcd_lock)
-{
-	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_MAX);
-	if (tcd->tcd_type == CFS_TCD_TYPE_IRQ)
-		spin_unlock_irqrestore(&tcd->tcd_lock, tcd->tcd_lock_flags);
-	else if (tcd->tcd_type == CFS_TCD_TYPE_SOFTIRQ)
-		spin_unlock_bh(&tcd->tcd_lock);
-	else if (unlikely(walking))
-		spin_unlock_irq(&tcd->tcd_lock);
-	else
-		spin_unlock(&tcd->tcd_lock);
-}
-
 void
 cfs_set_ptldebug_header(struct ptldebug_header *header,
 			struct libcfs_debug_msg_data *msgdata,
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 72321ce..6d567a9 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -109,6 +109,65 @@ struct cfs_trace_page {
 static void put_pages_on_tcd_daemon_list(struct page_collection *pc,
 					 struct cfs_trace_cpu_data *tcd);
 
+/* trace file lock routines */
+/*
+ * The walking argument indicates the locking comes from all tcd types
+ * iterator and we must lock it and dissable local irqs to avoid deadlocks
+ * with other interrupt locks that might be happening. See LU-1311
+ * for details.
+ */
+int cfs_trace_lock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
+	__acquires(&tcd->tc_lock)
+{
+	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_MAX);
+	if (tcd->tcd_type == CFS_TCD_TYPE_IRQ)
+		spin_lock_irqsave(&tcd->tcd_lock, tcd->tcd_lock_flags);
+	else if (tcd->tcd_type == CFS_TCD_TYPE_SOFTIRQ)
+		spin_lock_bh(&tcd->tcd_lock);
+	else if (unlikely(walking))
+		spin_lock_irq(&tcd->tcd_lock);
+	else
+		spin_lock(&tcd->tcd_lock);
+	return 1;
+}
+
+void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
+	__releases(&tcd->tcd_lock)
+{
+	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_MAX);
+	if (tcd->tcd_type == CFS_TCD_TYPE_IRQ)
+		spin_unlock_irqrestore(&tcd->tcd_lock, tcd->tcd_lock_flags);
+	else if (tcd->tcd_type == CFS_TCD_TYPE_SOFTIRQ)
+		spin_unlock_bh(&tcd->tcd_lock);
+	else if (unlikely(walking))
+		spin_unlock_irq(&tcd->tcd_lock);
+	else
+		spin_unlock(&tcd->tcd_lock);
+}
+
+#define cfs_tcd_for_each_type_lock(tcd, i, cpu)			   \
+	for (i = 0; cfs_trace_data[i] &&				\
+	     (tcd = &(*cfs_trace_data[i])[cpu].tcd) &&			\
+	     cfs_trace_lock_tcd(tcd, 1); cfs_trace_unlock_tcd(tcd, 1), i++)
+
+static inline struct cfs_trace_cpu_data *
+cfs_trace_get_tcd(void)
+{
+	struct cfs_trace_cpu_data *tcd =
+		&(*cfs_trace_data[cfs_trace_buf_idx_get()])[get_cpu()].tcd;
+
+	cfs_trace_lock_tcd(tcd, 0);
+
+	return tcd;
+}
+
+static inline void cfs_trace_put_tcd(struct cfs_trace_cpu_data *tcd)
+{
+	cfs_trace_unlock_tcd(tcd, 0);
+
+	put_cpu();
+}
+
 static inline struct cfs_trace_page *
 cfs_tage_from_list(struct list_head *list)
 {
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index bd1a1ef..91968cf 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -49,8 +49,6 @@ enum cfs_trace_buf_type {
 	CFS_TCD_TYPE_MAX
 };
 
-/* trace file lock routines */
-
 #define TRACEFILE_NAME_SIZE 1024
 extern char cfs_tracefile[TRACEFILE_NAME_SIZE];
 extern long long cfs_tracefile_size;
@@ -194,11 +192,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 		     j < num_possible_cpus();				 \
 		     j++, (tcd) = &(*cfs_trace_data[i])[j].tcd)
 
-#define cfs_tcd_for_each_type_lock(tcd, i, cpu)			   \
-	for (i = 0; cfs_trace_data[i] &&				\
-	     (tcd = &(*cfs_trace_data[i])[cpu].tcd) &&			\
-	     cfs_trace_lock_tcd(tcd, 1); cfs_trace_unlock_tcd(tcd, 1), i++)
-
 void cfs_set_ptldebug_header(struct ptldebug_header *header,
 			     struct libcfs_debug_msg_data *m,
 			     unsigned long stack);
@@ -206,9 +199,6 @@ void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
 			  const char *buf, int len, const char *file,
 			  const char *fn);
 
-int cfs_trace_lock_tcd(struct cfs_trace_cpu_data *tcd, int walking);
-void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking);
-
 extern char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
 enum cfs_trace_buf_type cfs_trace_buf_idx_get(void);
 
@@ -221,24 +211,6 @@ void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
 	return cfs_trace_console_buffers[i][j];
 }
 
-static inline struct cfs_trace_cpu_data *
-cfs_trace_get_tcd(void)
-{
-	struct cfs_trace_cpu_data *tcd =
-		&(*cfs_trace_data[cfs_trace_buf_idx_get()])[get_cpu()].tcd;
-
-	cfs_trace_lock_tcd(tcd, 0);
-
-	return tcd;
-}
-
-static inline void cfs_trace_put_tcd(struct cfs_trace_cpu_data *tcd)
-{
-	cfs_trace_unlock_tcd(tcd, 0);
-
-	put_cpu();
-}
-
 int cfs_trace_refill_stock(struct cfs_trace_cpu_data *tcd, gfp_t gfp,
 			   struct list_head *stock);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (2 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 03/13] lustre: libcfs: move tcd locking across to tracefile.c James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 21:27   ` NeilBrown
  2018-06-28 22:30   ` Andreas Dilger
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 05/13] lustre: libcfs: properly handle failure paths in cfs_tracefile_init_arch() James Simmons
                   ` (8 subsequent siblings)
  12 siblings, 2 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

The original code for cfs_print_to_console() used printk() and
used tricks to select which printk level to use. Later a cleanup
patch landed the just converted printk directly to pr_info which
is not exactly correct. Instead of converting back to printk lets
move everything to pr_* type functions which simplify the code.
This allows us to fold both dbghdr_to_err_string() and the
function dbghdr_to_info_string() into cfs_print_to_console().

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
 1 file changed, 14 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
index 3af7722..c1747c4 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
@@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
 	header->ph_extern_pid = 0;
 }
 
-static char *
-dbghdr_to_err_string(struct ptldebug_header *hdr)
-{
-	switch (hdr->ph_subsys) {
-	case S_LND:
-	case S_LNET:
-		return "LNetError";
-	default:
-		return "LustreError";
-	}
-}
-
-static char *
-dbghdr_to_info_string(struct ptldebug_header *hdr)
-{
-	switch (hdr->ph_subsys) {
-	case S_LND:
-	case S_LNET:
-		return "LNet";
-	default:
-		return "Lustre";
-	}
-}
-
 void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
 			  const char *buf, int len, const char *file,
 			  const char *fn)
 {
-	char *prefix = "Lustre", *ptype = NULL;
+	char *prefix = "Lustre";
+
+	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
+		prefix = "LNet";
 
-	if (mask & D_EMERG) {
-		prefix = dbghdr_to_err_string(hdr);
-		ptype = KERN_EMERG;
+	if (mask & (D_CONSOLE | libcfs_printk)) {
+		pr_info("%s: %.*s", prefix, len, buf);
+	} else if (mask & D_EMERG) {
+		pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
+			 hdr->ph_pid, hdr->ph_extern_pid, file,
+			 hdr->ph_line_num, fn, len, buf);
 	} else if (mask & D_ERROR) {
-		prefix = dbghdr_to_err_string(hdr);
-		ptype = KERN_ERR;
+		pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
+		       hdr->ph_pid, hdr->ph_extern_pid, file,
+		       hdr->ph_line_num, fn, len, buf);
 	} else if (mask & D_WARNING) {
-		prefix = dbghdr_to_info_string(hdr);
-		ptype = KERN_WARNING;
-	} else if (mask & (D_CONSOLE | libcfs_printk)) {
-		prefix = dbghdr_to_info_string(hdr);
-		ptype = KERN_INFO;
-	}
-
-	if (mask & D_CONSOLE) {
-		pr_info("%s%s: %.*s", ptype, prefix, len, buf);
-	} else {
-		pr_info("%s%s: %d:%d:(%s:%d:%s()) %.*s", ptype, prefix,
+		pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
 			hdr->ph_pid, hdr->ph_extern_pid, file,
 			hdr->ph_line_num, fn, len, buf);
 	}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 05/13] lustre: libcfs: properly handle failure paths in cfs_tracefile_init_arch()
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (3 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console() James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 06/13] lustre: libcfs: merge linux-tracefile.c into tracefile.c James Simmons
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

It is considered poor coding style in the linux kernel to call a
catch all for cleanup in the initialization function. Also their
is no reason to cleanup cfs_trace_console_buffers if they never
been allocated. Lets unroll the error handling.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/linux-tracefile.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
index f100cb9..a334b63 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
@@ -49,9 +49,9 @@
 
 int cfs_tracefile_init_arch(void)
 {
+	struct cfs_trace_cpu_data *tcd;
 	int i;
 	int j;
-	struct cfs_trace_cpu_data *tcd;
 
 	/* initialize trace_data */
 	memset(cfs_trace_data, 0, sizeof(cfs_trace_data));
@@ -61,7 +61,7 @@ int cfs_tracefile_init_arch(void)
 				      sizeof(union cfs_trace_data_union),
 				      GFP_KERNEL);
 		if (!cfs_trace_data[i])
-			goto out;
+			goto out_trace_data;
 	}
 
 	/* arch related info initialized */
@@ -79,13 +79,22 @@ int cfs_tracefile_init_arch(void)
 					GFP_KERNEL);
 
 			if (!cfs_trace_console_buffers[i][j])
-				goto out;
+				goto out_buffers;
 		}
 
 	return 0;
 
-out:
-	cfs_tracefile_fini_arch();
+out_buffers:
+	for (i = 0; i < num_possible_cpus(); i++)
+		for (j = 0; j < 3; j++) {
+			kfree(cfs_trace_console_buffers[i][j]);
+			cfs_trace_console_buffers[i][j] = NULL;
+		}
+out_trace_data:
+	for (i = 0; cfs_trace_data[i]; i++) {
+		kfree(cfs_trace_data[i]);
+		cfs_trace_data[i] = NULL;
+	}
 	pr_err("lnet: Not enough memory\n");
 	return -ENOMEM;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 06/13] lustre: libcfs: merge linux-tracefile.c into tracefile.c
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (4 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 05/13] lustre: libcfs: properly handle failure paths in cfs_tracefile_init_arch() James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 07/13] lustre: libcfs: remove cfs_trace_refill_stack() James Simmons
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

From: NeilBrown <neilb@suse.com>

It's good to keep related code together. The merger exposed the
flaws of cfs_print_to_console() so rework it to behavior properly.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/libcfs/Makefile        |   2 +-
 .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 176 ---------------------
 drivers/staging/lustre/lnet/libcfs/tracefile.c     | 135 ++++++++++++++++
 drivers/staging/lustre/lnet/libcfs/tracefile.h     |   7 -
 4 files changed, 136 insertions(+), 184 deletions(-)
 delete mode 100644 drivers/staging/lustre/lnet/libcfs/linux-tracefile.c

diff --git a/drivers/staging/lustre/lnet/libcfs/Makefile b/drivers/staging/lustre/lnet/libcfs/Makefile
index 5b13edc..cd96434 100644
--- a/drivers/staging/lustre/lnet/libcfs/Makefile
+++ b/drivers/staging/lustre/lnet/libcfs/Makefile
@@ -4,7 +4,7 @@ ccflags-y += -I$(srctree)/drivers/staging/lustre/lustre/include
 
 obj-$(CONFIG_LNET) += libcfs.o
 
-libcfs-obj-y += linux-tracefile.o linux-debug.o
+libcfs-obj-y += linux-debug.o
 libcfs-obj-y += linux-crypto.o
 libcfs-obj-y += linux-crypto-adler.o
 
diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
deleted file mode 100644
index becc773..0000000
--- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
+++ /dev/null
@@ -1,176 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * GPL HEADER START
- *
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 only,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License version 2 for more details (a copy is included
- * in the LICENSE file that accompanied this code).
- *
- * You should have received a copy of the GNU General Public License
- * version 2 along with this program; If not, see
- * http://www.gnu.org/licenses/gpl-2.0.html
- *
- * GPL HEADER END
- */
-/*
- * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
- * Use is subject to license terms.
- *
- * Copyright (c) 2012, Intel Corporation.
- */
-/*
- * This file is part of Lustre, http://www.lustre.org/
- * Lustre is a trademark of Sun Microsystems, Inc.
- */
-
-#define DEBUG_SUBSYSTEM S_LNET
-#define LUSTRE_TRACEFILE_PRIVATE
-
-#include <linux/slab.h>
-#include <linux/mm.h>
-#include "tracefile.h"
-
-/* percents to share the total debug memory for each type */
-static unsigned int pages_factor[CFS_TCD_TYPE_MAX] = {
-	80,  /* 80% pages for CFS_TCD_TYPE_PROC */
-	10,  /* 10% pages for CFS_TCD_TYPE_SOFTIRQ */
-	10   /* 10% pages for CFS_TCD_TYPE_IRQ */
-};
-
-char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
-
-int cfs_tracefile_init_arch(void)
-{
-	struct cfs_trace_cpu_data *tcd;
-	int i;
-	int j;
-
-	/* initialize trace_data */
-	memset(cfs_trace_data, 0, sizeof(cfs_trace_data));
-	for (i = 0; i < CFS_TCD_TYPE_MAX; i++) {
-		cfs_trace_data[i] =
-			kmalloc_array(num_possible_cpus(),
-				      sizeof(union cfs_trace_data_union),
-				      GFP_KERNEL);
-		if (!cfs_trace_data[i])
-			goto out_trace_data;
-	}
-
-	/* arch related info initialized */
-	cfs_tcd_for_each(tcd, i, j) {
-		spin_lock_init(&tcd->tcd_lock);
-		tcd->tcd_pages_factor = pages_factor[i];
-		tcd->tcd_type = i;
-		tcd->tcd_cpu = j;
-	}
-
-	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
-			cfs_trace_console_buffers[i][j] =
-				kmalloc(CFS_TRACE_CONSOLE_BUFFER_SIZE,
-					GFP_KERNEL);
-
-			if (!cfs_trace_console_buffers[i][j])
-				goto out_buffers;
-		}
-
-	return 0;
-
-out_buffers:
-	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
-			kfree(cfs_trace_console_buffers[i][j]);
-			cfs_trace_console_buffers[i][j] = NULL;
-		}
-out_trace_data:
-	for (i = 0; cfs_trace_data[i]; i++) {
-		kfree(cfs_trace_data[i]);
-		cfs_trace_data[i] = NULL;
-	}
-	pr_err("lnet: Not enough memory\n");
-	return -ENOMEM;
-}
-
-void cfs_tracefile_fini_arch(void)
-{
-	int i;
-	int j;
-
-	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
-			kfree(cfs_trace_console_buffers[i][j]);
-			cfs_trace_console_buffers[i][j] = NULL;
-		}
-
-	for (i = 0; cfs_trace_data[i]; i++) {
-		kfree(cfs_trace_data[i]);
-		cfs_trace_data[i] = NULL;
-	}
-}
-
-enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
-{
-	if (in_irq())
-		return CFS_TCD_TYPE_IRQ;
-	if (in_softirq())
-		return CFS_TCD_TYPE_SOFTIRQ;
-	return CFS_TCD_TYPE_PROC;
-}
-
-void
-cfs_set_ptldebug_header(struct ptldebug_header *header,
-			struct libcfs_debug_msg_data *msgdata,
-			unsigned long stack)
-{
-	struct timespec64 ts;
-
-	ktime_get_real_ts64(&ts);
-
-	header->ph_subsys = msgdata->msg_subsys;
-	header->ph_mask = msgdata->msg_mask;
-	header->ph_cpu_id = smp_processor_id();
-	header->ph_type = cfs_trace_buf_idx_get();
-	/* y2038 safe since all user space treats this as unsigned, but
-	 * will overflow in 2106
-	 */
-	header->ph_sec = (u32)ts.tv_sec;
-	header->ph_usec = ts.tv_nsec / NSEC_PER_USEC;
-	header->ph_stack = stack;
-	header->ph_pid = current->pid;
-	header->ph_line_num = msgdata->msg_line;
-	header->ph_extern_pid = 0;
-}
-
-void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
-			  const char *buf, int len, const char *file,
-			  const char *fn)
-{
-	char *prefix = "Lustre";
-
-	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
-		prefix = "LNet";
-
-	if (mask & (D_CONSOLE | libcfs_printk)) {
-		pr_info("%s: %.*s", prefix, len, buf);
-	} else if (mask & D_EMERG) {
-		pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
-			 hdr->ph_pid, hdr->ph_extern_pid, file,
-			 hdr->ph_line_num, fn, len, buf);
-	} else if (mask & D_ERROR) {
-		pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
-		       hdr->ph_pid, hdr->ph_extern_pid, file,
-		       hdr->ph_line_num, fn, len, buf);
-	} else if (mask & D_WARNING) {
-		pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
-			hdr->ph_pid, hdr->ph_extern_pid, file,
-			hdr->ph_line_num, fn, len, buf);
-	}
-}
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 6d567a9..fbf54d4 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -52,6 +52,7 @@
 /* XXX move things up to the top, comment */
 union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS] __cacheline_aligned;
 
+char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
 char cfs_tracefile[TRACEFILE_NAME_SIZE];
 long long cfs_tracefile_size = CFS_TRACEFILE_SIZE;
 static struct tracefiled_ctl trace_tctl;
@@ -150,6 +151,15 @@ void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
 	     (tcd = &(*cfs_trace_data[i])[cpu].tcd) &&			\
 	     cfs_trace_lock_tcd(tcd, 1); cfs_trace_unlock_tcd(tcd, 1), i++)
 
+enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
+{
+	if (in_irq())
+		return CFS_TCD_TYPE_IRQ;
+	if (in_softirq())
+		return CFS_TCD_TYPE_SOFTIRQ;
+	return CFS_TCD_TYPE_PROC;
+}
+
 static inline struct cfs_trace_cpu_data *
 cfs_trace_get_tcd(void)
 {
@@ -168,6 +178,82 @@ static inline void cfs_trace_put_tcd(struct cfs_trace_cpu_data *tcd)
 	put_cpu();
 }
 
+/* percents to share the total debug memory for each type */
+static unsigned int pages_factor[CFS_TCD_TYPE_MAX] = {
+	80,  /* 80% pages for CFS_TCD_TYPE_PROC */
+	10,  /* 10% pages for CFS_TCD_TYPE_SOFTIRQ */
+	10   /* 10% pages for CFS_TCD_TYPE_IRQ */
+};
+
+int cfs_tracefile_init_arch(void)
+{
+	struct cfs_trace_cpu_data *tcd;
+	int i;
+	int j;
+
+	/* initialize trace_data */
+	memset(cfs_trace_data, 0, sizeof(cfs_trace_data));
+	for (i = 0; i < CFS_TCD_TYPE_MAX; i++) {
+		cfs_trace_data[i] =
+			kmalloc_array(num_possible_cpus(),
+				      sizeof(union cfs_trace_data_union),
+				      GFP_KERNEL);
+		if (!cfs_trace_data[i])
+			goto out_trace_data;
+	}
+
+	/* arch related info initialized */
+	cfs_tcd_for_each(tcd, i, j) {
+		spin_lock_init(&tcd->tcd_lock);
+		tcd->tcd_pages_factor = pages_factor[i];
+		tcd->tcd_type = i;
+		tcd->tcd_cpu = j;
+	}
+
+	for (i = 0; i < num_possible_cpus(); i++)
+		for (j = 0; j < 3; j++) {
+			cfs_trace_console_buffers[i][j] =
+				kmalloc(CFS_TRACE_CONSOLE_BUFFER_SIZE,
+					GFP_KERNEL);
+
+			if (!cfs_trace_console_buffers[i][j])
+				goto out_buffers;
+		}
+
+	return 0;
+
+out_buffers:
+	for (i = 0; i < num_possible_cpus(); i++)
+		for (j = 0; j < 3; j++) {
+			kfree(cfs_trace_console_buffers[i][j]);
+			cfs_trace_console_buffers[i][j] = NULL;
+		}
+out_trace_data:
+	for (i = 0; cfs_trace_data[i]; i++) {
+		kfree(cfs_trace_data[i]);
+		cfs_trace_data[i] = NULL;
+	}
+	pr_err("lnet: Not enough memory\n");
+	return -ENOMEM;
+}
+
+void cfs_tracefile_fini_arch(void)
+{
+	int i;
+	int j;
+
+	for (i = 0; i < num_possible_cpus(); i++)
+		for (j = 0; j < 3; j++) {
+			kfree(cfs_trace_console_buffers[i][j]);
+			cfs_trace_console_buffers[i][j] = NULL;
+		}
+
+	for (i = 0; cfs_trace_data[i]; i++) {
+		kfree(cfs_trace_data[i]);
+		cfs_trace_data[i] = NULL;
+	}
+}
+
 static inline struct cfs_trace_page *
 cfs_tage_from_list(struct list_head *list)
 {
@@ -340,6 +426,55 @@ static struct cfs_trace_page *cfs_trace_get_tage(struct cfs_trace_cpu_data *tcd,
 	return tage;
 }
 
+static void cfs_set_ptldebug_header(struct ptldebug_header *header,
+				    struct libcfs_debug_msg_data *msgdata,
+				    unsigned long stack)
+{
+	struct timespec64 ts;
+
+	ktime_get_real_ts64(&ts);
+
+	header->ph_subsys = msgdata->msg_subsys;
+	header->ph_mask = msgdata->msg_mask;
+	header->ph_cpu_id = smp_processor_id();
+	header->ph_type = cfs_trace_buf_idx_get();
+	/* y2038 safe since all user space treats this as unsigned, but
+	 * will overflow in 2106
+	 */
+	header->ph_sec = (u32)ts.tv_sec;
+	header->ph_usec = ts.tv_nsec / NSEC_PER_USEC;
+	header->ph_stack = stack;
+	header->ph_pid = current->pid;
+	header->ph_line_num = msgdata->msg_line;
+	header->ph_extern_pid = 0;
+}
+
+static void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
+				 const char *buf, int len, const char *file,
+				 const char *fn)
+{
+	char *prefix = "Lustre";
+
+	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
+		prefix = "LNet";
+
+	if (mask & (D_CONSOLE | libcfs_printk)) {
+		pr_info("%s: %.*s", prefix, len, buf);
+	} else if (mask & D_EMERG) {
+		pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
+			 hdr->ph_pid, hdr->ph_extern_pid, file,
+			 hdr->ph_line_num, fn, len, buf);
+	} else if (mask & D_ERROR) {
+		pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
+		       hdr->ph_pid, hdr->ph_extern_pid, file,
+		       hdr->ph_line_num, fn, len, buf);
+	} else if (mask & D_WARNING) {
+		pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
+			hdr->ph_pid, hdr->ph_extern_pid, file,
+			hdr->ph_line_num, fn, len, buf);
+	}
+}
+
 int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata,
 		     const char *format, ...)
 {
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index 91968cf..399e0bf 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -192,13 +192,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 		     j < num_possible_cpus();				 \
 		     j++, (tcd) = &(*cfs_trace_data[i])[j].tcd)
 
-void cfs_set_ptldebug_header(struct ptldebug_header *header,
-			     struct libcfs_debug_msg_data *m,
-			     unsigned long stack);
-void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
-			  const char *buf, int len, const char *file,
-			  const char *fn);
-
 extern char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
 enum cfs_trace_buf_type cfs_trace_buf_idx_get(void);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 07/13] lustre: libcfs: remove cfs_trace_refill_stack()
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (5 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 06/13] lustre: libcfs: merge linux-tracefile.c into tracefile.c James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 08/13] lustre: libcfs: move cfs_trace_data data to tracefile.c James Simmons
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

The function cfs_trace_refill_stack() is not used anywhere so
remove it.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c | 21 ---------------------
 drivers/staging/lustre/lnet/libcfs/tracefile.h |  3 ---
 2 files changed, 24 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index a2d4ee9..6a42d7c 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -302,27 +302,6 @@ static void cfs_tage_to_tail(struct cfs_trace_page *tage,
 	list_move_tail(&tage->linkage, queue);
 }
 
-int cfs_trace_refill_stock(struct cfs_trace_cpu_data *tcd, gfp_t gfp,
-			   struct list_head *stock)
-{
-	int i;
-
-	/*
-	 * XXX nikita: do NOT call portals_debug_msg() (CDEBUG/ENTRY/EXIT)
-	 * from here: this will lead to infinite recursion.
-	 */
-
-	for (i = 0; i + tcd->tcd_cur_stock_pages < TCD_STOCK_PAGES ; ++i) {
-		struct cfs_trace_page *tage;
-
-		tage = cfs_tage_alloc(gfp);
-		if (!tage)
-			break;
-		list_add_tail(&tage->linkage, stock);
-	}
-	return i;
-}
-
 /* return a page that has 'len' bytes left at the end */
 static struct cfs_trace_page *
 cfs_trace_get_tage_try(struct cfs_trace_cpu_data *tcd, unsigned long len)
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index 399e0bf..b9e4a8e 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -204,9 +204,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 	return cfs_trace_console_buffers[i][j];
 }
 
-int cfs_trace_refill_stock(struct cfs_trace_cpu_data *tcd, gfp_t gfp,
-			   struct list_head *stock);
-
 void cfs_trace_assertion_failed(const char *str,
 				struct libcfs_debug_msg_data *m);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 08/13] lustre: libcfs: move cfs_trace_data data to tracefile.c
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (6 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 07/13] lustre: libcfs: remove cfs_trace_refill_stack() James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 09/13] lustre: libcfs: cleanup tracefile.h James Simmons
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

The macro cfs_tcd_for_each() is only used in tracefile.c so move
it from the header tracefile.h along with related material in
the header file.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c | 12 ++++++++++--
 drivers/staging/lustre/lnet/libcfs/tracefile.h |  9 ---------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 6a42d7c..914cd94 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -49,7 +49,8 @@
 #include <linux/uaccess.h>
 #include "tracefile.h"
 
-/* XXX move things up to the top, comment */
+#define TCD_MAX_TYPES			8
+
 union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS] __cacheline_aligned;
 
 char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
@@ -146,7 +147,14 @@ void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
 		spin_unlock(&tcd->tcd_lock);
 }
 
-#define cfs_tcd_for_each_type_lock(tcd, i, cpu)			   \
+#define cfs_tcd_for_each(tcd, i, j)					\
+	for (i = 0; cfs_trace_data[i]; i++)				\
+		for (j = 0, ((tcd) = &(*cfs_trace_data[i])[j].tcd);	\
+		     j < num_possible_cpus();				\
+		     j++, (tcd) = &(*cfs_trace_data[i])[j].tcd)
+
+
+#define cfs_tcd_for_each_type_lock(tcd, i, cpu)				\
 	for (i = 0; cfs_trace_data[i] &&				\
 	     (tcd = &(*cfs_trace_data[i])[cpu].tcd) &&			\
 	     cfs_trace_lock_tcd(tcd, 1); cfs_trace_unlock_tcd(tcd, 1), i++)
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index b9e4a8e..072c720 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -183,15 +183,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 	char __pad[L1_CACHE_ALIGN(sizeof(struct cfs_trace_cpu_data))];
 };
 
-#define TCD_MAX_TYPES      8
-extern union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS];
-
-#define cfs_tcd_for_each(tcd, i, j)				       \
-	for (i = 0; cfs_trace_data[i]; i++)				\
-		for (j = 0, ((tcd) = &(*cfs_trace_data[i])[j].tcd);	\
-		     j < num_possible_cpus();				 \
-		     j++, (tcd) = &(*cfs_trace_data[i])[j].tcd)
-
 extern char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
 enum cfs_trace_buf_type cfs_trace_buf_idx_get(void);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 09/13] lustre: libcfs: cleanup tracefile.h
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (7 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 08/13] lustre: libcfs: move cfs_trace_data data to tracefile.c James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 10/13] lustre: libcfs: fold cfs_tracefile_*_arch into their only callers James Simmons
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

With many things moved into tracefile.c we can cleanup a lot of
things in tracefile.h. Move some items that are only used in
tracefile.c from tracefile.h into tracefile.c. In tracefile.h
we have the ifdef LUSTRE_TRACEFILE_PRIVATE which has several
duplicate defines. We can remove those duplicates.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c | 18 +++++++++++--
 drivers/staging/lustre/lnet/libcfs/tracefile.h | 37 --------------------------
 2 files changed, 16 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 914cd94..5095e66 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -37,8 +37,6 @@
  */
 
 #define DEBUG_SUBSYSTEM S_LNET
-#define LUSTRE_TRACEFILE_PRIVATE
-#define pr_fmt(fmt) "Lustre: " fmt
 
 #include <linux/ratelimit.h>
 #include <linux/highmem.h>
@@ -49,8 +47,16 @@
 #include <linux/uaccess.h>
 #include "tracefile.h"
 
+#define CFS_TRACE_CONSOLE_BUFFER_SIZE	1024
 #define TCD_MAX_TYPES			8
 
+enum cfs_trace_buf_type {
+	CFS_TCD_TYPE_PROC = 0,
+	CFS_TCD_TYPE_SOFTIRQ,
+	CFS_TCD_TYPE_IRQ,
+	CFS_TCD_TYPE_MAX
+};
+
 union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS] __cacheline_aligned;
 
 char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
@@ -168,6 +174,14 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
 	return CFS_TCD_TYPE_PROC;
 }
 
+static inline char *cfs_trace_get_console_buffer(void)
+{
+	unsigned int i = get_cpu();
+	unsigned int j = cfs_trace_buf_idx_get();
+
+	return cfs_trace_console_buffers[i][j];
+}
+
 static inline struct cfs_trace_cpu_data *
 cfs_trace_get_tcd(void)
 {
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index 072c720..3e7b6fa 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -42,13 +42,6 @@
 #include <linux/smp.h>
 #include <linux/libcfs/libcfs.h>
 
-enum cfs_trace_buf_type {
-	CFS_TCD_TYPE_PROC = 0,
-	CFS_TCD_TYPE_SOFTIRQ,
-	CFS_TCD_TYPE_IRQ,
-	CFS_TCD_TYPE_MAX
-};
-
 #define TRACEFILE_NAME_SIZE 1024
 extern char cfs_tracefile[TRACEFILE_NAME_SIZE];
 extern long long cfs_tracefile_size;
@@ -91,22 +84,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 #define TCD_STOCK_PAGES (TCD_MAX_PAGES)
 #define CFS_TRACEFILE_SIZE (500 << 20)
 
-#ifdef LUSTRE_TRACEFILE_PRIVATE
-
-/*
- * Private declare for tracefile
- */
-#define TCD_MAX_PAGES (5 << (20 - PAGE_SHIFT))
-#define TCD_STOCK_PAGES (TCD_MAX_PAGES)
-
-#define CFS_TRACEFILE_SIZE (500 << 20)
-
-/*
- * Size of a buffer for sprinting console messages if we can't get a page
- * from system
- */
-#define CFS_TRACE_CONSOLE_BUFFER_SIZE   1024
-
 union cfs_trace_data_union {
 	struct cfs_trace_cpu_data {
 		/*
@@ -183,18 +160,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 	char __pad[L1_CACHE_ALIGN(sizeof(struct cfs_trace_cpu_data))];
 };
 
-extern char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
-enum cfs_trace_buf_type cfs_trace_buf_idx_get(void);
-
-static inline char *
-cfs_trace_get_console_buffer(void)
-{
-	unsigned int i = get_cpu();
-	unsigned int j = cfs_trace_buf_idx_get();
-
-	return cfs_trace_console_buffers[i][j];
-}
-
 void cfs_trace_assertion_failed(const char *str,
 				struct libcfs_debug_msg_data *m);
 
@@ -216,6 +181,4 @@ void cfs_trace_assertion_failed(const char *str,
 	__LASSERT(page_count(tage->page) > 0);		      \
 } while (0)
 
-#endif	/* LUSTRE_TRACEFILE_PRIVATE */
-
 #endif /* __LIBCFS_TRACEFILE_H__ */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 10/13] lustre: libcfs: fold cfs_tracefile_*_arch into their only callers.
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (8 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 09/13] lustre: libcfs: cleanup tracefile.h James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 11/13] lustre: libcfs: renamed CFS_TCD_TYPE_MAX to CFS_TCD_TYPE_CNT James Simmons
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

From: NeilBrown <neilb@suse.com>

There is no need to separate "arch" init/fini from
the rest, so fold it all in.
This requires some slightly subtle changes to clean-up
to make sure we don't walk lists before they are
initialized.

Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c | 147 +++++++++++--------------
 drivers/staging/lustre/lnet/libcfs/tracefile.h |   3 -
 2 files changed, 63 insertions(+), 87 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 5095e66..a2b5004 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -200,82 +200,6 @@ static inline void cfs_trace_put_tcd(struct cfs_trace_cpu_data *tcd)
 	put_cpu();
 }
 
-/* percents to share the total debug memory for each type */
-static unsigned int pages_factor[CFS_TCD_TYPE_MAX] = {
-	80,  /* 80% pages for CFS_TCD_TYPE_PROC */
-	10,  /* 10% pages for CFS_TCD_TYPE_SOFTIRQ */
-	10   /* 10% pages for CFS_TCD_TYPE_IRQ */
-};
-
-int cfs_tracefile_init_arch(void)
-{
-	struct cfs_trace_cpu_data *tcd;
-	int i;
-	int j;
-
-	/* initialize trace_data */
-	memset(cfs_trace_data, 0, sizeof(cfs_trace_data));
-	for (i = 0; i < CFS_TCD_TYPE_MAX; i++) {
-		cfs_trace_data[i] =
-			kmalloc_array(num_possible_cpus(),
-				      sizeof(union cfs_trace_data_union),
-				      GFP_KERNEL);
-		if (!cfs_trace_data[i])
-			goto out_trace_data;
-	}
-
-	/* arch related info initialized */
-	cfs_tcd_for_each(tcd, i, j) {
-		spin_lock_init(&tcd->tcd_lock);
-		tcd->tcd_pages_factor = pages_factor[i];
-		tcd->tcd_type = i;
-		tcd->tcd_cpu = j;
-	}
-
-	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
-			cfs_trace_console_buffers[i][j] =
-				kmalloc(CFS_TRACE_CONSOLE_BUFFER_SIZE,
-					GFP_KERNEL);
-
-			if (!cfs_trace_console_buffers[i][j])
-				goto out_buffers;
-		}
-
-	return 0;
-
-out_buffers:
-	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
-			kfree(cfs_trace_console_buffers[i][j]);
-			cfs_trace_console_buffers[i][j] = NULL;
-		}
-out_trace_data:
-	for (i = 0; cfs_trace_data[i]; i++) {
-		kfree(cfs_trace_data[i]);
-		cfs_trace_data[i] = NULL;
-	}
-	pr_err("lnet: Not enough memory\n");
-	return -ENOMEM;
-}
-
-void cfs_tracefile_fini_arch(void)
-{
-	int i;
-	int j;
-
-	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
-			kfree(cfs_trace_console_buffers[i][j]);
-			cfs_trace_console_buffers[i][j] = NULL;
-		}
-
-	for (i = 0; cfs_trace_data[i]; i++) {
-		kfree(cfs_trace_data[i]);
-		cfs_trace_data[i] = NULL;
-	}
-}
-
 static inline struct cfs_trace_page *
 cfs_tage_from_list(struct list_head *list)
 {
@@ -1324,21 +1248,38 @@ void cfs_trace_stop_thread(void)
 	mutex_unlock(&cfs_trace_thread_mutex);
 }
 
+/* percents to share the total debug memory for each type */
+static unsigned int pages_factor[CFS_TCD_TYPE_MAX] = {
+	80,  /* 80% pages for CFS_TCD_TYPE_PROC */
+	10,  /* 10% pages for CFS_TCD_TYPE_SOFTIRQ */
+	10   /* 10% pages for CFS_TCD_TYPE_IRQ */
+};
+
 int cfs_tracefile_init(int max_pages)
 {
 	struct cfs_trace_cpu_data *tcd;
 	int i;
 	int j;
-	int rc;
-	int factor;
 
-	rc = cfs_tracefile_init_arch();
-	if (rc)
-		return rc;
+	/* initialize trace_data */
+	memset(cfs_trace_data, 0, sizeof(cfs_trace_data));
+	for (i = 0; i < CFS_TCD_TYPE_MAX; i++) {
+		cfs_trace_data[i] =
+			kmalloc_array(num_possible_cpus(),
+				      sizeof(union cfs_trace_data_union),
+				      GFP_KERNEL);
+		if (!cfs_trace_data[i])
+			goto out_trace_data;
+	}
 
+	/* arch related info initialized */
 	cfs_tcd_for_each(tcd, i, j) {
-		/* tcd_pages_factor is initialized int tracefile_init_arch. */
-		factor = tcd->tcd_pages_factor;
+		int factor = pages_factor[i];
+
+		spin_lock_init(&tcd->tcd_lock);
+		tcd->tcd_pages_factor = factor;
+		tcd->tcd_type = i;
+		tcd->tcd_cpu = j;
 		INIT_LIST_HEAD(&tcd->tcd_pages);
 		INIT_LIST_HEAD(&tcd->tcd_stock_pages);
 		INIT_LIST_HEAD(&tcd->tcd_daemon_pages);
@@ -1350,7 +1291,31 @@ int cfs_tracefile_init(int max_pages)
 		tcd->tcd_shutting_down = 0;
 	}
 
+	for (i = 0; i < num_possible_cpus(); i++)
+		for (j = 0; j < 3; j++) {
+			cfs_trace_console_buffers[i][j] =
+				kmalloc(CFS_TRACE_CONSOLE_BUFFER_SIZE,
+					GFP_KERNEL);
+
+			if (!cfs_trace_console_buffers[i][j])
+				goto out_buffers;
+		}
+
 	return 0;
+
+out_buffers:
+	for (i = 0; i < num_possible_cpus(); i++)
+		for (j = 0; j < 3; j++) {
+			kfree(cfs_trace_console_buffers[i][j]);
+			cfs_trace_console_buffers[i][j] = NULL;
+		}
+out_trace_data:
+	for (i = 0; cfs_trace_data[i]; i++) {
+		kfree(cfs_trace_data[i]);
+		cfs_trace_data[i] = NULL;
+	}
+	pr_err("lnet: Not enough memory\n");
+	return -ENOMEM;
 }
 
 static void trace_cleanup_on_all_cpus(void)
@@ -1362,6 +1327,9 @@ static void trace_cleanup_on_all_cpus(void)
 
 	for_each_possible_cpu(cpu) {
 		cfs_tcd_for_each_type_lock(tcd, i, cpu) {
+			if (!tcd->tcd_pages_factor)
+				/* Not initialised */
+				continue;
 			tcd->tcd_shutting_down = 1;
 
 			list_for_each_entry_safe(tage, tmp, &tcd->tcd_pages,
@@ -1380,12 +1348,23 @@ static void trace_cleanup_on_all_cpus(void)
 static void cfs_trace_cleanup(void)
 {
 	struct page_collection pc;
+	int i;
+	int j;
 
 	INIT_LIST_HEAD(&pc.pc_pages);
 
 	trace_cleanup_on_all_cpus();
 
-	cfs_tracefile_fini_arch();
+	for (i = 0; i < num_possible_cpus(); i++)
+		for (j = 0; j < 3; j++) {
+			kfree(cfs_trace_console_buffers[i][j]);
+			cfs_trace_console_buffers[i][j] = NULL;
+		}
+
+	for (i = 0; cfs_trace_data[i]; i++) {
+		kfree(cfs_trace_data[i]);
+		cfs_trace_data[i] = NULL;
+	}
 }
 
 void cfs_tracefile_exit(void)
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index 3e7b6fa..dc782a6 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -53,9 +53,6 @@
 
 void libcfs_run_debug_log_upcall(char *file);
 
-int  cfs_tracefile_init_arch(void);
-void cfs_tracefile_fini_arch(void);
-
 int cfs_tracefile_dump_all_pages(char *filename);
 void cfs_trace_debug_print(void);
 void cfs_trace_flush_pages(void);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 11/13] lustre: libcfs: renamed CFS_TCD_TYPE_MAX to CFS_TCD_TYPE_CNT
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (9 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 10/13] lustre: libcfs: fold cfs_tracefile_*_arch into their only callers James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 12/13] lustre: libcfs: discard TCD_MAX_TYPES James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 13/13] lustre: libcfs: format macros in tracefile.h James Simmons
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

From: NeilBrown <neilb@suse.com>

The possible TCD types are 0, 1, 2.
So the MAX is 2.
The count of the number of types is 3.

CFS_TCD_TYPE_MAX is 3 - obviously wrong.

So rename it to CFS_TCD_TYPE_CNT.

Also there are 2 places where "3" is used rather
than the macro - fix them.

Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index a2b5004..b102465 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -54,12 +54,12 @@ enum cfs_trace_buf_type {
 	CFS_TCD_TYPE_PROC = 0,
 	CFS_TCD_TYPE_SOFTIRQ,
 	CFS_TCD_TYPE_IRQ,
-	CFS_TCD_TYPE_MAX
+	CFS_TCD_TYPE_CNT
 };
 
 union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS] __cacheline_aligned;
 
-char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
+char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_CNT];
 char cfs_tracefile[TRACEFILE_NAME_SIZE];
 long long cfs_tracefile_size = CFS_TRACEFILE_SIZE;
 static struct tracefiled_ctl trace_tctl;
@@ -127,7 +127,7 @@ static void put_pages_on_tcd_daemon_list(struct page_collection *pc,
 int cfs_trace_lock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
 	__acquires(&tcd->tc_lock)
 {
-	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_MAX);
+	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_CNT);
 	if (tcd->tcd_type == CFS_TCD_TYPE_IRQ)
 		spin_lock_irqsave(&tcd->tcd_lock, tcd->tcd_lock_flags);
 	else if (tcd->tcd_type == CFS_TCD_TYPE_SOFTIRQ)
@@ -142,7 +142,7 @@ int cfs_trace_lock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
 void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
 	__releases(&tcd->tcd_lock)
 {
-	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_MAX);
+	__LASSERT(tcd->tcd_type < CFS_TCD_TYPE_CNT);
 	if (tcd->tcd_type == CFS_TCD_TYPE_IRQ)
 		spin_unlock_irqrestore(&tcd->tcd_lock, tcd->tcd_lock_flags);
 	else if (tcd->tcd_type == CFS_TCD_TYPE_SOFTIRQ)
@@ -1249,7 +1249,7 @@ void cfs_trace_stop_thread(void)
 }
 
 /* percents to share the total debug memory for each type */
-static unsigned int pages_factor[CFS_TCD_TYPE_MAX] = {
+static unsigned int pages_factor[CFS_TCD_TYPE_CNT] = {
 	80,  /* 80% pages for CFS_TCD_TYPE_PROC */
 	10,  /* 10% pages for CFS_TCD_TYPE_SOFTIRQ */
 	10   /* 10% pages for CFS_TCD_TYPE_IRQ */
@@ -1263,7 +1263,7 @@ int cfs_tracefile_init(int max_pages)
 
 	/* initialize trace_data */
 	memset(cfs_trace_data, 0, sizeof(cfs_trace_data));
-	for (i = 0; i < CFS_TCD_TYPE_MAX; i++) {
+	for (i = 0; i < CFS_TCD_TYPE_CNT; i++) {
 		cfs_trace_data[i] =
 			kmalloc_array(num_possible_cpus(),
 				      sizeof(union cfs_trace_data_union),
@@ -1292,7 +1292,7 @@ int cfs_tracefile_init(int max_pages)
 	}
 
 	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
+		for (j = 0; j < CFS_TCD_TYPE_CNT; j++) {
 			cfs_trace_console_buffers[i][j] =
 				kmalloc(CFS_TRACE_CONSOLE_BUFFER_SIZE,
 					GFP_KERNEL);
@@ -1356,7 +1356,7 @@ static void cfs_trace_cleanup(void)
 	trace_cleanup_on_all_cpus();
 
 	for (i = 0; i < num_possible_cpus(); i++)
-		for (j = 0; j < 3; j++) {
+		for (j = 0; j < CFS_TCD_TYPE_CNT; j++) {
 			kfree(cfs_trace_console_buffers[i][j]);
 			cfs_trace_console_buffers[i][j] = NULL;
 		}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 12/13] lustre: libcfs: discard TCD_MAX_TYPES
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (10 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 11/13] lustre: libcfs: renamed CFS_TCD_TYPE_MAX to CFS_TCD_TYPE_CNT James Simmons
@ 2018-06-27 19:38 ` James Simmons
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 13/13] lustre: libcfs: format macros in tracefile.h James Simmons
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

From: NeilBrown <neilb@suse.com>

As well as CFS_TCD_TYPE_CNT we have TCD_MAX_TYPES which has a larger
value but a similar meaning.  Discard it and just use
CFS_TCD_TYPE_CNT.

Two places relied on the fact that TCD_MAX_TYPES was larger and so
there would be NULLs at the end of the array.  Change
them to check the array size properly.

Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index b102465..0f64fa2 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -48,7 +48,6 @@
 #include "tracefile.h"
 
 #define CFS_TRACE_CONSOLE_BUFFER_SIZE	1024
-#define TCD_MAX_TYPES			8
 
 enum cfs_trace_buf_type {
 	CFS_TCD_TYPE_PROC = 0,
@@ -57,7 +56,7 @@ enum cfs_trace_buf_type {
 	CFS_TCD_TYPE_CNT
 };
 
-union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS] __cacheline_aligned;
+union cfs_trace_data_union (*cfs_trace_data[CFS_TCD_TYPE_CNT])[NR_CPUS] __cacheline_aligned;
 
 char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_CNT];
 char cfs_tracefile[TRACEFILE_NAME_SIZE];
@@ -154,14 +153,14 @@ void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
 }
 
 #define cfs_tcd_for_each(tcd, i, j)					\
-	for (i = 0; cfs_trace_data[i]; i++)				\
+	for (i = 0; i < CFS_TCD_TYPE_CNT && cfs_trace_data[i]; i++)	\
 		for (j = 0, ((tcd) = &(*cfs_trace_data[i])[j].tcd);	\
 		     j < num_possible_cpus();				\
 		     j++, (tcd) = &(*cfs_trace_data[i])[j].tcd)
 
 
 #define cfs_tcd_for_each_type_lock(tcd, i, cpu)				\
-	for (i = 0; cfs_trace_data[i] &&				\
+	for (i = 0;  i < CFS_TCD_TYPE_CNT && cfs_trace_data[i] &&	\
 	     (tcd = &(*cfs_trace_data[i])[cpu].tcd) &&			\
 	     cfs_trace_lock_tcd(tcd, 1); cfs_trace_unlock_tcd(tcd, 1), i++)
 
@@ -1361,7 +1360,7 @@ static void cfs_trace_cleanup(void)
 			cfs_trace_console_buffers[i][j] = NULL;
 		}
 
-	for (i = 0; cfs_trace_data[i]; i++) {
+	for (i = 0; i < CFS_TCD_TYPE_CNT && cfs_trace_data[i]; i++) {
 		kfree(cfs_trace_data[i]);
 		cfs_trace_data[i] = NULL;
 	}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 13/13] lustre: libcfs: format macros in tracefile.h
  2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
                   ` (11 preceding siblings ...)
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 12/13] lustre: libcfs: discard TCD_MAX_TYPES James Simmons
@ 2018-06-27 19:38 ` James Simmons
  12 siblings, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-06-27 19:38 UTC (permalink / raw)
  To: lustre-devel

The __LASSERT_* macros are ugly and hard to read. This patch
beautifies them.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
index dc782a6..67107b1 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
@@ -161,21 +161,21 @@ void cfs_trace_assertion_failed(const char *str,
 				struct libcfs_debug_msg_data *m);
 
 /* ASSERTION that is safe to use within the debug system */
-#define __LASSERT(cond)						 \
-do {								    \
+#define __LASSERT(cond)							\
+do {									\
 	if (unlikely(!(cond))) {					\
-		LIBCFS_DEBUG_MSG_DATA_DECL(msgdata, D_EMERG, NULL);     \
-		cfs_trace_assertion_failed("ASSERTION("#cond") failed", \
-					   &msgdata);		   \
-	}							       \
+		LIBCFS_DEBUG_MSG_DATA_DECL(msgdata, D_EMERG, NULL);	\
+		cfs_trace_assertion_failed("ASSERTION("#cond") failed",	\
+					   &msgdata);			\
+	}								\
 } while (0)
 
-#define __LASSERT_TAGE_INVARIANT(tage)				  \
-do {								    \
-	__LASSERT(tage);					\
-	__LASSERT(tage->page);				  \
-	__LASSERT(tage->used <= PAGE_SIZE);			 \
-	__LASSERT(page_count(tage->page) > 0);		      \
+#define __LASSERT_TAGE_INVARIANT(tage)			\
+do {							\
+	__LASSERT(tage);				\
+	__LASSERT(tage->page);				\
+	__LASSERT(tage->used <= PAGE_SIZE);		\
+	__LASSERT(page_count(tage->page) > 0);		\
 } while (0)
 
 #endif /* __LIBCFS_TRACEFILE_H__ */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console() James Simmons
@ 2018-06-27 21:27   ` NeilBrown
  2018-06-28 22:30   ` Andreas Dilger
  1 sibling, 0 replies; 25+ messages in thread
From: NeilBrown @ 2018-06-27 21:27 UTC (permalink / raw)
  To: lustre-devel

On Wed, Jun 27 2018, James Simmons wrote:

> The original code for cfs_print_to_console() used printk() and
> used tricks to select which printk level to use. Later a cleanup
> patch landed the just converted printk directly to pr_info which
> is not exactly correct. Instead of converting back to printk lets
> move everything to pr_* type functions which simplify the code.
> This allows us to fold both dbghdr_to_err_string() and the
> function dbghdr_to_info_string() into cfs_print_to_console().
>
> Signed-off-by: James Simmons <jsimmons@infradead.org>

Thanks!
This code is much nicer now, and the whole series is much easier to
review now that you've broken it up.

It'll all appear in my lustre-testing shortly.

Thanks,
NeilBrown


> ---
>  .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
>  1 file changed, 14 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> index 3af7722..c1747c4 100644
> --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> @@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
>  	header->ph_extern_pid = 0;
>  }
>  
> -static char *
> -dbghdr_to_err_string(struct ptldebug_header *hdr)
> -{
> -	switch (hdr->ph_subsys) {
> -	case S_LND:
> -	case S_LNET:
> -		return "LNetError";
> -	default:
> -		return "LustreError";
> -	}
> -}
> -
> -static char *
> -dbghdr_to_info_string(struct ptldebug_header *hdr)
> -{
> -	switch (hdr->ph_subsys) {
> -	case S_LND:
> -	case S_LNET:
> -		return "LNet";
> -	default:
> -		return "Lustre";
> -	}
> -}
> -
>  void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
>  			  const char *buf, int len, const char *file,
>  			  const char *fn)
>  {
> -	char *prefix = "Lustre", *ptype = NULL;
> +	char *prefix = "Lustre";
> +
> +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
> +		prefix = "LNet";
>  
> -	if (mask & D_EMERG) {
> -		prefix = dbghdr_to_err_string(hdr);
> -		ptype = KERN_EMERG;
> +	if (mask & (D_CONSOLE | libcfs_printk)) {
> +		pr_info("%s: %.*s", prefix, len, buf);
> +	} else if (mask & D_EMERG) {
> +		pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> +			 hdr->ph_pid, hdr->ph_extern_pid, file,
> +			 hdr->ph_line_num, fn, len, buf);
>  	} else if (mask & D_ERROR) {
> -		prefix = dbghdr_to_err_string(hdr);
> -		ptype = KERN_ERR;
> +		pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> +		       hdr->ph_pid, hdr->ph_extern_pid, file,
> +		       hdr->ph_line_num, fn, len, buf);
>  	} else if (mask & D_WARNING) {
> -		prefix = dbghdr_to_info_string(hdr);
> -		ptype = KERN_WARNING;
> -	} else if (mask & (D_CONSOLE | libcfs_printk)) {
> -		prefix = dbghdr_to_info_string(hdr);
> -		ptype = KERN_INFO;
> -	}
> -
> -	if (mask & D_CONSOLE) {
> -		pr_info("%s%s: %.*s", ptype, prefix, len, buf);
> -	} else {
> -		pr_info("%s%s: %d:%d:(%s:%d:%s()) %.*s", ptype, prefix,
> +		pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
>  			hdr->ph_pid, hdr->ph_extern_pid, file,
>  			hdr->ph_line_num, fn, len, buf);
>  	}
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180628/510b8380/attachment.sig>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console() James Simmons
  2018-06-27 21:27   ` NeilBrown
@ 2018-06-28 22:30   ` Andreas Dilger
  2018-06-29 16:17     ` James Simmons
  1 sibling, 1 reply; 25+ messages in thread
From: Andreas Dilger @ 2018-06-28 22:30 UTC (permalink / raw)
  To: lustre-devel

On Jun 27, 2018, at 13:38, James Simmons <jsimmons@infradead.org> wrote:
> 
> The original code for cfs_print_to_console() used printk() and
> used tricks to select which printk level to use. Later a cleanup
> patch landed the just converted printk directly to pr_info which
> is not exactly correct. Instead of converting back to printk lets
> move everything to pr_* type functions which simplify the code.
> This allows us to fold both dbghdr_to_err_string() and the
> function dbghdr_to_info_string() into cfs_print_to_console().
> 
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
> .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
> 1 file changed, 14 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> index 3af7722..c1747c4 100644
> --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> @@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
> void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
> 			  const char *buf, int len, const char *file,
> 			  const char *fn)
> {
> +	char *prefix = "Lustre";
> +
> +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
> +		prefix = "LNet";
> 
> -	if (mask & D_EMERG) {
> -		prefix = dbghdr_to_err_string(hdr);
> -		ptype = KERN_EMERG;
> +	if (mask & (D_CONSOLE | libcfs_printk)) {

This check is broken.  The default value of libcfs_printk (the mask
that controls which messages should be printed to the console, and
which ones should only be logged into the internal buffer) is:

    #define D_CANTMASK (D_ERROR | D_EMERG | D_WARNING | D_CONSOLE)

so that means virtually every console message will be printed with
pr_info() because this is matched first, instead of pr_emerg() or
pr_err() below.

That is why the previous code was matching D_EMERG and D_ERROR first,
then D_WARNING, and (D_CONSOLE | libcfs_printk) at the end.

Cheers, Andreas

> +		pr_info("%s: %.*s", prefix, len, buf);
> +	} else if (mask & D_EMERG) {
> +		pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> +			 hdr->ph_pid, hdr->ph_extern_pid, file,
> +			 hdr->ph_line_num, fn, len, buf);
> 	} else if (mask & D_ERROR) {
> -		prefix = dbghdr_to_err_string(hdr);
> -		ptype = KERN_ERR;
> +		pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> +		       hdr->ph_pid, hdr->ph_extern_pid, file,
> +		       hdr->ph_line_num, fn, len, buf);
> 	} else if (mask & D_WARNING) {
> -		prefix = dbghdr_to_info_string(hdr);
> -		ptype = KERN_WARNING;
> -	} else if (mask & (D_CONSOLE | libcfs_printk)) {
> -		prefix = dbghdr_to_info_string(hdr);
> -		ptype = KERN_INFO;
> -	}
> -
> -	if (mask & D_CONSOLE) {
> -		pr_info("%s%s: %.*s", ptype, prefix, len, buf);
> -	} else {
> -		pr_info("%s%s: %d:%d:(%s:%d:%s()) %.*s", ptype, prefix,
> +		pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
> 			hdr->ph_pid, hdr->ph_extern_pid, file,
> 			hdr->ph_line_num, fn, len, buf);
> 	}
> --
> 1.8.3.1
> 

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud







-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 235 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180628/b49c7062/attachment.sig>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb()
  2018-06-27 19:38 ` [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb() James Simmons
@ 2018-06-28 22:39   ` Andreas Dilger
  2018-06-29  3:55     ` NeilBrown
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Dilger @ 2018-06-28 22:39 UTC (permalink / raw)
  To: lustre-devel

On Jun 27, 2018, at 13:38, James Simmons <jsimmons@infradead.org> wrote:
> 
> From: NeilBrown <neilb@suse.com>
> 
> This function in used twice.
> In libcfs_debug_init() the usage is pointless as
> the only place that set libcfs_debug_mb ensures
> that it does not exceed the maximum.  So checking
> again can never help.
> 
> So open-code the small function into the only
> other place that it is used - in cfs_trace_set_debug_mb(),
> which is used to set libcfs_debug_mb.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
> drivers/staging/lustre/lnet/libcfs/debug.c           | 6 +++---
> drivers/staging/lustre/lnet/libcfs/linux-tracefile.c | 7 -------
> drivers/staging/lustre/lnet/libcfs/tracefile.c       | 3 ++-
> drivers/staging/lustre/lnet/libcfs/tracefile.h       | 1 -
> 4 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/libcfs/debug.c b/drivers/staging/lustre/lnet/libcfs/debug.c
> index 06f694f..71effcf 100644
> --- a/drivers/staging/lustre/lnet/libcfs/debug.c
> +++ b/drivers/staging/lustre/lnet/libcfs/debug.c
> @@ -411,10 +411,10 @@ int libcfs_debug_init(unsigned long bufsize)
> 			sizeof(libcfs_debug_file_path_arr));
> 	}
> 
> -	/* If libcfs_debug_mb is set to an invalid value or uninitialized
> -	 * then just make the total buffers smp_num_cpus * TCD_MAX_PAGES
> +	/* If libcfs_debug_mb is uninitialized then just make the
> +	 * total buffers smp_num_cpus * TCD_MAX_PAGES
> 	 */
> -	if (max > cfs_trace_max_debug_mb() || max < num_possible_cpus()) {
> +	if (max < num_possible_cpus()) {

The libcfs_debug_mb value may be set as a module option, so that the debug
buffer can be sized before any debugging messages are logged (in case of
problems early on in module loading and such).  This code validates that
the value set via module parameter is sane.

It may be that there is a mechanism to limit the value set by module option
in newer kernels, but that wasn't the case in the past.

Cheers, Andreas

> 		max = TCD_MAX_PAGES;
> 	} else {
> 		max = max / num_possible_cpus();
> diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> index 9e72220..64a5bc1 100644
> --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> @@ -227,10 +227,3 @@ void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
> 			hdr->ph_line_num, fn, len, buf);
> 	}
> }
> -
> -int cfs_trace_max_debug_mb(void)
> -{
> -	int  total_mb = (totalram_pages >> (20 - PAGE_SHIFT));
> -
> -	return max(512, (total_mb * 80) / 100);
> -}
> diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
> index 5f31933..72321ce 100644
> --- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
> +++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
> @@ -933,7 +933,8 @@ int cfs_trace_set_debug_mb(int mb)
> 	int i;
> 	int j;
> 	int pages;
> -	int limit = cfs_trace_max_debug_mb();
> +	int total_mb = (totalram_pages >> (20 - PAGE_SHIFT));
> +	int limit = max(512, (total_mb * 80) / 100);
> 	struct cfs_trace_cpu_data *tcd;
> 
> 	if (mb < num_possible_cpus()) {
> diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
> index 9f6b73d..bd1a1ef 100644
> --- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
> +++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
> @@ -88,7 +88,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
> void libcfs_register_panic_notifier(void);
> void libcfs_unregister_panic_notifier(void);
> extern int libcfs_panic_in_progress;
> -int cfs_trace_max_debug_mb(void);
> 
> #define TCD_MAX_PAGES (5 << (20 - PAGE_SHIFT))
> #define TCD_STOCK_PAGES (TCD_MAX_PAGES)
> --
> 1.8.3.1
> 

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud







-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 235 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180628/70844d26/attachment.sig>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb()
  2018-06-28 22:39   ` Andreas Dilger
@ 2018-06-29  3:55     ` NeilBrown
  2018-07-02  2:17       ` NeilBrown
  0 siblings, 1 reply; 25+ messages in thread
From: NeilBrown @ 2018-06-29  3:55 UTC (permalink / raw)
  To: lustre-devel

On Thu, Jun 28 2018, Andreas Dilger wrote:

> On Jun 27, 2018, at 13:38, James Simmons <jsimmons@infradead.org> wrote:
>> 
>> From: NeilBrown <neilb@suse.com>
>> 
>> This function in used twice.
>> In libcfs_debug_init() the usage is pointless as
>> the only place that set libcfs_debug_mb ensures
>> that it does not exceed the maximum.  So checking
>> again can never help.
>> 
>> So open-code the small function into the only
>> other place that it is used - in cfs_trace_set_debug_mb(),
>> which is used to set libcfs_debug_mb.
>> 
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>> drivers/staging/lustre/lnet/libcfs/debug.c           | 6 +++---
>> drivers/staging/lustre/lnet/libcfs/linux-tracefile.c | 7 -------
>> drivers/staging/lustre/lnet/libcfs/tracefile.c       | 3 ++-
>> drivers/staging/lustre/lnet/libcfs/tracefile.h       | 1 -
>> 4 files changed, 5 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lnet/libcfs/debug.c b/drivers/staging/lustre/lnet/libcfs/debug.c
>> index 06f694f..71effcf 100644
>> --- a/drivers/staging/lustre/lnet/libcfs/debug.c
>> +++ b/drivers/staging/lustre/lnet/libcfs/debug.c
>> @@ -411,10 +411,10 @@ int libcfs_debug_init(unsigned long bufsize)
>> 			sizeof(libcfs_debug_file_path_arr));
>> 	}
>> 
>> -	/* If libcfs_debug_mb is set to an invalid value or uninitialized
>> -	 * then just make the total buffers smp_num_cpus * TCD_MAX_PAGES
>> +	/* If libcfs_debug_mb is uninitialized then just make the
>> +	 * total buffers smp_num_cpus * TCD_MAX_PAGES
>> 	 */
>> -	if (max > cfs_trace_max_debug_mb() || max < num_possible_cpus()) {
>> +	if (max < num_possible_cpus()) {
>
> The libcfs_debug_mb value may be set as a module option, so that the debug
> buffer can be sized before any debugging messages are logged (in case of
> problems early on in module loading and such).  This code validates that
> the value set via module parameter is sane.
>
> It may be that there is a mechanism to limit the value set by module option
> in newer kernels, but that wasn't the case in the past.

There is such a mechanism now.
When the libcfs_debug_mb module parameter is detected,
libcfs_param_debug_mb_set() is called to parse it.
This function uses kstrtouint() to convert the string
to unsigned int, and then...... oh, that's odd.
If the current value of libcfs_debug_mb is zero, then
then the number is used without further checking.  I hadn't noticed
that.
If that current value is non-zero, then the parsed number
is passed to cfs_trace_set_debug_mb().  I had assumed that
always happens, and that function imposes the required limit.

I'll fix that up - make sure it always imposes the
appropriate limit.

Thanks for the review!

NeilBrown


>
> Cheers, Andreas
>
>> 		max = TCD_MAX_PAGES;
>> 	} else {
>> 		max = max / num_possible_cpus();
>> diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> index 9e72220..64a5bc1 100644
>> --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> @@ -227,10 +227,3 @@ void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
>> 			hdr->ph_line_num, fn, len, buf);
>> 	}
>> }
>> -
>> -int cfs_trace_max_debug_mb(void)
>> -{
>> -	int  total_mb = (totalram_pages >> (20 - PAGE_SHIFT));
>> -
>> -	return max(512, (total_mb * 80) / 100);
>> -}
>> diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
>> index 5f31933..72321ce 100644
>> --- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
>> +++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
>> @@ -933,7 +933,8 @@ int cfs_trace_set_debug_mb(int mb)
>> 	int i;
>> 	int j;
>> 	int pages;
>> -	int limit = cfs_trace_max_debug_mb();
>> +	int total_mb = (totalram_pages >> (20 - PAGE_SHIFT));
>> +	int limit = max(512, (total_mb * 80) / 100);
>> 	struct cfs_trace_cpu_data *tcd;
>> 
>> 	if (mb < num_possible_cpus()) {
>> diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
>> index 9f6b73d..bd1a1ef 100644
>> --- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
>> +++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
>> @@ -88,7 +88,6 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
>> void libcfs_register_panic_notifier(void);
>> void libcfs_unregister_panic_notifier(void);
>> extern int libcfs_panic_in_progress;
>> -int cfs_trace_max_debug_mb(void);
>> 
>> #define TCD_MAX_PAGES (5 << (20 - PAGE_SHIFT))
>> #define TCD_STOCK_PAGES (TCD_MAX_PAGES)
>> --
>> 1.8.3.1
>> 
>
> Cheers, Andreas
> ---
> Andreas Dilger
> Principal Lustre Architect
> Whamcloud
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180629/dad555dd/attachment.sig>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-06-28 22:30   ` Andreas Dilger
@ 2018-06-29 16:17     ` James Simmons
  2018-07-02  2:27       ` NeilBrown
  0 siblings, 1 reply; 25+ messages in thread
From: James Simmons @ 2018-06-29 16:17 UTC (permalink / raw)
  To: lustre-devel


> > The original code for cfs_print_to_console() used printk() and
> > used tricks to select which printk level to use. Later a cleanup
> > patch landed the just converted printk directly to pr_info which
> > is not exactly correct. Instead of converting back to printk lets
> > move everything to pr_* type functions which simplify the code.
> > This allows us to fold both dbghdr_to_err_string() and the
> > function dbghdr_to_info_string() into cfs_print_to_console().
> > 
> > Signed-off-by: James Simmons <jsimmons@infradead.org>
> > ---
> > .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
> > 1 file changed, 14 insertions(+), 41 deletions(-)
> > 
> > diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> > index 3af7722..c1747c4 100644
> > --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> > +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> > @@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
> > void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
> > 			  const char *buf, int len, const char *file,
> > 			  const char *fn)
> > {
> > +	char *prefix = "Lustre";
> > +
> > +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
> > +		prefix = "LNet";
> > 
> > -	if (mask & D_EMERG) {
> > -		prefix = dbghdr_to_err_string(hdr);
> > -		ptype = KERN_EMERG;
> > +	if (mask & (D_CONSOLE | libcfs_printk)) {
> 
> This check is broken.  The default value of libcfs_printk (the mask
> that controls which messages should be printed to the console, and
> which ones should only be logged into the internal buffer) is:
> 
>     #define D_CANTMASK (D_ERROR | D_EMERG | D_WARNING | D_CONSOLE)
> 
> so that means virtually every console message will be printed with
> pr_info() because this is matched first, instead of pr_emerg() or
> pr_err() below.
> 
> That is why the previous code was matching D_EMERG and D_ERROR first,
> then D_WARNING, and (D_CONSOLE | libcfs_printk) at the end.

So to do this right we need:

static void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
                                 const char *buf, int len, const char 
*file,
                                 const char *fn)
{
        char *prefix = "Lustre";

        if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
                prefix = "LNet";

        if (mask & D_CONSOLE) {
                if (mask & D_EMERG) {
                        pr_emerg("%sError: %.*s", prefix, len, buf);
                } else if (mask & D_ERROR) {
                        pr_err("%sError: %.*s", prefix, len, buf);
                } else if (mask & D_WARNING) {
                        pr_warn("%s: %.*s", prefix, len, buf);
                } else if (mask & libcfs_printk) {
                        pr_info("%s: %.*s", prefix, len, buf);
                }
        } else {
                if (mask & D_EMERG) {
                        pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
                                 hdr->ph_pid, hdr->ph_extern_pid, file,
                                 hdr->ph_line_num, fn, len, buf);
                } else if (mask & D_ERROR) {
                        pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
                               hdr->ph_pid, hdr->ph_extern_pid, file,
                               hdr->ph_line_num, fn, len, buf);
                } else if (mask & D_WARNING) {
                        pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
                                hdr->ph_pid, hdr->ph_extern_pid, file,
                                hdr->ph_line_num, fn, len, buf);
                } else if (mask & libcfs_printk) {
                        pr_info("%s: %.*s", prefix, len, buf);
                }
        }
}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb()
  2018-06-29  3:55     ` NeilBrown
@ 2018-07-02  2:17       ` NeilBrown
  0 siblings, 0 replies; 25+ messages in thread
From: NeilBrown @ 2018-07-02  2:17 UTC (permalink / raw)
  To: lustre-devel

On Fri, Jun 29 2018, NeilBrown wrote:
>> It may be that there is a mechanism to limit the value set by module option
>> in newer kernels, but that wasn't the case in the past.
>
> There is such a mechanism now.
> When the libcfs_debug_mb module parameter is detected,
> libcfs_param_debug_mb_set() is called to parse it.
> This function uses kstrtouint() to convert the string
> to unsigned int, and then...... oh, that's odd.
> If the current value of libcfs_debug_mb is zero, then
> then the number is used without further checking.  I hadn't noticed
> that.
> If that current value is non-zero, then the parsed number
> is passed to cfs_trace_set_debug_mb().  I had assumed that
> always happens, and that function imposes the required limit.
>
> I'll fix that up - make sure it always imposes the
> appropriate limit.

I've inserted the following patch before this one.
That makes it correct :-)

Thanks,
NeilBrown

From: NeilBrown <neilb@suse.com>
Date: Mon, 2 Jul 2018 12:14:15 +1000
Subject: [PATCH] lustre: always range-check libcfs_debug_mb setting.

When the libcfs_debug_mb module parameter is set
at module-load time it isn't range-checked.  When
it is set via sysfs it is.  This inconsistency
makes the code harder to follow.

It is quite safe to call cfs_trace_set_debug_mb()
and cfs_trace_get_debug_mb() before the module
is initialized as cfs_tcd_for_each() does nothing
before initializtion.

So change cfs_trace_set_debug_mb() - which does
range checking - to returned the ranged checked number (it currently
always returns zero) and use that as the new value, unless
cfs_trace_get_debug_mb() now returns a non-zero value.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/libcfs/debug.c     | 16 +++++++---------
 drivers/staging/lustre/lnet/libcfs/tracefile.c |  2 +-
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/debug.c b/drivers/staging/lustre/lnet/libcfs/debug.c
index 06f694f6a28f..50c2995c1c99 100644
--- a/drivers/staging/lustre/lnet/libcfs/debug.c
+++ b/drivers/staging/lustre/lnet/libcfs/debug.c
@@ -67,17 +67,15 @@ static int libcfs_param_debug_mb_set(const char *val,
 	if (rc < 0)
 		return rc;
 
-	if (!*((unsigned int *)kp->arg)) {
-		*((unsigned int *)kp->arg) = num;
-		return 0;
-	}
-
-	rc = cfs_trace_set_debug_mb(num);
+	num = cfs_trace_set_debug_mb(num);
 
-	if (!rc)
-		*((unsigned int *)kp->arg) = cfs_trace_get_debug_mb();
+	*((unsigned int *)kp->arg) = num;
+	num = cfs_trace_get_debug_mb();
+	if (num)
+		/* This value is more precise */
+		*((unsigned int *)kp->arg) = num;
 
-	return rc;
+	return 0;
 }
 
 /* While debug_mb setting look like unsigned int, in fact
diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 5f319332f60b..3b92fd7d3182 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -958,7 +958,7 @@ int cfs_trace_set_debug_mb(int mb)
 
 	up_write(&cfs_tracefile_sem);
 
-	return 0;
+	return mb;
 }
 
 int cfs_trace_get_debug_mb(void)
-- 
2.14.0.rc0.dirty

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180702/dc2265db/attachment.sig>

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-06-29 16:17     ` James Simmons
@ 2018-07-02  2:27       ` NeilBrown
  2018-07-05 23:34         ` James Simmons
  2018-07-05 23:35         ` James Simmons
  0 siblings, 2 replies; 25+ messages in thread
From: NeilBrown @ 2018-07-02  2:27 UTC (permalink / raw)
  To: lustre-devel

On Fri, Jun 29 2018, James Simmons wrote:

>> > The original code for cfs_print_to_console() used printk() and
>> > used tricks to select which printk level to use. Later a cleanup
>> > patch landed the just converted printk directly to pr_info which
>> > is not exactly correct. Instead of converting back to printk lets
>> > move everything to pr_* type functions which simplify the code.
>> > This allows us to fold both dbghdr_to_err_string() and the
>> > function dbghdr_to_info_string() into cfs_print_to_console().
>> > 
>> > Signed-off-by: James Simmons <jsimmons@infradead.org>
>> > ---
>> > .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
>> > 1 file changed, 14 insertions(+), 41 deletions(-)
>> > 
>> > diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> > index 3af7722..c1747c4 100644
>> > --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> > +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> > @@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
>> > void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
>> > 			  const char *buf, int len, const char *file,
>> > 			  const char *fn)
>> > {
>> > +	char *prefix = "Lustre";
>> > +
>> > +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
>> > +		prefix = "LNet";
>> > 
>> > -	if (mask & D_EMERG) {
>> > -		prefix = dbghdr_to_err_string(hdr);
>> > -		ptype = KERN_EMERG;
>> > +	if (mask & (D_CONSOLE | libcfs_printk)) {
>> 
>> This check is broken.  The default value of libcfs_printk (the mask
>> that controls which messages should be printed to the console, and
>> which ones should only be logged into the internal buffer) is:
>> 
>>     #define D_CANTMASK (D_ERROR | D_EMERG | D_WARNING | D_CONSOLE)
>> 
>> so that means virtually every console message will be printed with
>> pr_info() because this is matched first, instead of pr_emerg() or
>> pr_err() below.
>> 
>> That is why the previous code was matching D_EMERG and D_ERROR first,
>> then D_WARNING, and (D_CONSOLE | libcfs_printk) at the end.
>
> So to do this right we need:
>
> static void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
>                                  const char *buf, int len, const char 
> *file,
>                                  const char *fn)
> {
>         char *prefix = "Lustre";
>
>         if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
>                 prefix = "LNet";
>
>         if (mask & D_CONSOLE) {
>                 if (mask & D_EMERG) {
>                         pr_emerg("%sError: %.*s", prefix, len, buf);
>                 } else if (mask & D_ERROR) {
>                         pr_err("%sError: %.*s", prefix, len, buf);
>                 } else if (mask & D_WARNING) {
>                         pr_warn("%s: %.*s", prefix, len, buf);
>                 } else if (mask & libcfs_printk) {
>                         pr_info("%s: %.*s", prefix, len, buf);
>                 }
>         } else {
>                 if (mask & D_EMERG) {
>                         pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
>                                  hdr->ph_pid, hdr->ph_extern_pid, file,
>                                  hdr->ph_line_num, fn, len, buf);
>                 } else if (mask & D_ERROR) {
>                         pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
>                                hdr->ph_pid, hdr->ph_extern_pid, file,
>                                hdr->ph_line_num, fn, len, buf);
>                 } else if (mask & D_WARNING) {
>                         pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
>                                 hdr->ph_pid, hdr->ph_extern_pid, file,
>                                 hdr->ph_line_num, fn, len, buf);
>                 } else if (mask & libcfs_printk) {
>                         pr_info("%s: %.*s", prefix, len, buf);
>                 }
>         }
> }

That doesn't look right either.
The original code would *always* print something.
This code looks like it might not (e.g. for mask == 0).

What is "D_CONSOLE" suppose to mean?  It seems to me "make the messages
less verbose" and it isn't clear to me that what is called "D_CONSOLE".

Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180702/de30cd2c/attachment.sig>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-07-02  2:27       ` NeilBrown
@ 2018-07-05 23:34         ` James Simmons
  2018-07-05 23:35         ` James Simmons
  1 sibling, 0 replies; 25+ messages in thread
From: James Simmons @ 2018-07-05 23:34 UTC (permalink / raw)
  To: lustre-devel


> >> > The original code for cfs_print_to_console() used printk() and
> >> > used tricks to select which printk level to use. Later a cleanup
> >> > patch landed the just converted printk directly to pr_info which
> >> > is not exactly correct. Instead of converting back to printk lets
> >> > move everything to pr_* type functions which simplify the code.
> >> > This allows us to fold both dbghdr_to_err_string() and the
> >> > function dbghdr_to_info_string() into cfs_print_to_console().
> >> > 
> >> > Signed-off-by: James Simmons <jsimmons@infradead.org>
> >> > ---
> >> > .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
> >> > 1 file changed, 14 insertions(+), 41 deletions(-)
> >> > 
> >> > diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> >> > index 3af7722..c1747c4 100644
> >> > --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> >> > +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> >> > @@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
> >> > void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
> >> > 			  const char *buf, int len, const char *file,
> >> > 			  const char *fn)
> >> > {
> >> > +	char *prefix = "Lustre";
> >> > +
> >> > +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
> >> > +		prefix = "LNet";
> >> > 
> >> > -	if (mask & D_EMERG) {
> >> > -		prefix = dbghdr_to_err_string(hdr);
> >> > -		ptype = KERN_EMERG;
> >> > +	if (mask & (D_CONSOLE | libcfs_printk)) {
> >> 
> >> This check is broken.  The default value of libcfs_printk (the mask
> >> that controls which messages should be printed to the console, and
> >> which ones should only be logged into the internal buffer) is:
> >> 
> >>     #define D_CANTMASK (D_ERROR | D_EMERG | D_WARNING | D_CONSOLE)
> >> 
> >> so that means virtually every console message will be printed with
> >> pr_info() because this is matched first, instead of pr_emerg() or
> >> pr_err() below.
> >> 
> >> That is why the previous code was matching D_EMERG and D_ERROR first,
> >> then D_WARNING, and (D_CONSOLE | libcfs_printk) at the end.
> >
> > So to do this right we need:
> >
> > static void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
> >                                  const char *buf, int len, const char 
> > *file,
> >                                  const char *fn)
> > {
> >         char *prefix = "Lustre";
> >
> >         if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
> >                 prefix = "LNet";
> >
> >         if (mask & D_CONSOLE) {
> >                 if (mask & D_EMERG) {
> >                         pr_emerg("%sError: %.*s", prefix, len, buf);
> >                 } else if (mask & D_ERROR) {
> >                         pr_err("%sError: %.*s", prefix, len, buf);
> >                 } else if (mask & D_WARNING) {
> >                         pr_warn("%s: %.*s", prefix, len, buf);
> >                 } else if (mask & libcfs_printk) {
> >                         pr_info("%s: %.*s", prefix, len, buf);
> >                 }
> >         } else {
> >                 if (mask & D_EMERG) {
> >                         pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> >                                  hdr->ph_pid, hdr->ph_extern_pid, file,
> >                                  hdr->ph_line_num, fn, len, buf);
> >                 } else if (mask & D_ERROR) {
> >                         pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> >                                hdr->ph_pid, hdr->ph_extern_pid, file,
> >                                hdr->ph_line_num, fn, len, buf);
> >                 } else if (mask & D_WARNING) {
> >                         pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
> >                                 hdr->ph_pid, hdr->ph_extern_pid, file,
> >                                 hdr->ph_line_num, fn, len, buf);
> >                 } else if (mask & libcfs_printk) {
> >                         pr_info("%s: %.*s", prefix, len, buf);
> >                 }
> >         }
> > }
> 
> That doesn't look right either.
> The original code would *always* print something.
> This code looks like it might not (e.g. for mask == 0).

Is that correct behavior? A mask of zero means don't do anything.
Also if you look at the original in the OpenSFS branch you can see
if mask was to be 0 then ptype would be NULl :-( 

Note a D_CANT_MASK prevents some fields in the mask from being disabled.

> What is "D_CONSOLE" suppose to mean?  It seems to me "make the messages
> less verbose" and it isn't clear to me that what is called "D_CONSOLE".

It means two things. If by itself then it means use pr_info. If it is one
field in the mask then it means less detail. Confusing no :-( That is my
understanding of it. Maybe someone else can clarify if I'm wrong.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-07-02  2:27       ` NeilBrown
  2018-07-05 23:34         ` James Simmons
@ 2018-07-05 23:35         ` James Simmons
  2018-07-06  4:19           ` NeilBrown
  2018-07-06 17:09           ` Andreas Dilger
  1 sibling, 2 replies; 25+ messages in thread
From: James Simmons @ 2018-07-05 23:35 UTC (permalink / raw)
  To: lustre-devel


> >> > The original code for cfs_print_to_console() used printk() and
> >> > used tricks to select which printk level to use. Later a cleanup
> >> > patch landed the just converted printk directly to pr_info which
> >> > is not exactly correct. Instead of converting back to printk lets
> >> > move everything to pr_* type functions which simplify the code.
> >> > This allows us to fold both dbghdr_to_err_string() and the
> >> > function dbghdr_to_info_string() into cfs_print_to_console().
> >> > 
> >> > Signed-off-by: James Simmons <jsimmons@infradead.org>
> >> > ---
> >> > .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
> >> > 1 file changed, 14 insertions(+), 41 deletions(-)
> >> > 
> >> > diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> >> > index 3af7722..c1747c4 100644
> >> > --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> >> > +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
> >> > @@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
> >> > void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
> >> > 			  const char *buf, int len, const char *file,
> >> > 			  const char *fn)
> >> > {
> >> > +	char *prefix = "Lustre";
> >> > +
> >> > +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
> >> > +		prefix = "LNet";
> >> > 
> >> > -	if (mask & D_EMERG) {
> >> > -		prefix = dbghdr_to_err_string(hdr);
> >> > -		ptype = KERN_EMERG;
> >> > +	if (mask & (D_CONSOLE | libcfs_printk)) {
> >> 
> >> This check is broken.  The default value of libcfs_printk (the mask
> >> that controls which messages should be printed to the console, and
> >> which ones should only be logged into the internal buffer) is:
> >> 
> >>     #define D_CANTMASK (D_ERROR | D_EMERG | D_WARNING | D_CONSOLE)
> >> 
> >> so that means virtually every console message will be printed with
> >> pr_info() because this is matched first, instead of pr_emerg() or
> >> pr_err() below.
> >> 
> >> That is why the previous code was matching D_EMERG and D_ERROR first,
> >> then D_WARNING, and (D_CONSOLE | libcfs_printk) at the end.
> >
> > So to do this right we need:
> >
> > static void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
> >                                  const char *buf, int len, const char 
> > *file,
> >                                  const char *fn)
> > {
> >         char *prefix = "Lustre";
> >
> >         if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
> >                 prefix = "LNet";
> >
> >         if (mask & D_CONSOLE) {
> >                 if (mask & D_EMERG) {
> >                         pr_emerg("%sError: %.*s", prefix, len, buf);
> >                 } else if (mask & D_ERROR) {
> >                         pr_err("%sError: %.*s", prefix, len, buf);
> >                 } else if (mask & D_WARNING) {
> >                         pr_warn("%s: %.*s", prefix, len, buf);
> >                 } else if (mask & libcfs_printk) {
> >                         pr_info("%s: %.*s", prefix, len, buf);
> >                 }
> >         } else {
> >                 if (mask & D_EMERG) {
> >                         pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> >                                  hdr->ph_pid, hdr->ph_extern_pid, file,
> >                                  hdr->ph_line_num, fn, len, buf);
> >                 } else if (mask & D_ERROR) {
> >                         pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
> >                                hdr->ph_pid, hdr->ph_extern_pid, file,
> >                                hdr->ph_line_num, fn, len, buf);
> >                 } else if (mask & D_WARNING) {
> >                         pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
> >                                 hdr->ph_pid, hdr->ph_extern_pid, file,
> >                                 hdr->ph_line_num, fn, len, buf);
> >                 } else if (mask & libcfs_printk) {
> >                         pr_info("%s: %.*s", prefix, len, buf);
> >                 }
> >         }
> > }
> 
> That doesn't look right either.
> The original code would *always* print something.
> This code looks like it might not (e.g. for mask == 0).

Is that correct behavior? A mask of zero means don't do anything.
Also if you look at the original in the OpenSFS branch you can see
if mask was to be 0 then ptype would be NULl :-( 

Note a D_CANT_MASK prevents some fields in the mask from being disabled.

> What is "D_CONSOLE" suppose to mean?  It seems to me "make the messages
> less verbose" and it isn't clear to me that what is called "D_CONSOLE".

It means two things. If by itself then it means use pr_info. If it is one
field in the mask then it means less detail. Confusing no :-( That is my
understanding of it. Maybe someone else can clarify if I'm wrong.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-07-05 23:35         ` James Simmons
@ 2018-07-06  4:19           ` NeilBrown
  2018-07-06 17:09           ` Andreas Dilger
  1 sibling, 0 replies; 25+ messages in thread
From: NeilBrown @ 2018-07-06  4:19 UTC (permalink / raw)
  To: lustre-devel

On Fri, Jul 06 2018, James Simmons wrote:

>> >> > The original code for cfs_print_to_console() used printk() and
>> >> > used tricks to select which printk level to use. Later a cleanup
>> >> > patch landed the just converted printk directly to pr_info which
>> >> > is not exactly correct. Instead of converting back to printk lets
>> >> > move everything to pr_* type functions which simplify the code.
>> >> > This allows us to fold both dbghdr_to_err_string() and the
>> >> > function dbghdr_to_info_string() into cfs_print_to_console().
>> >> > 
>> >> > Signed-off-by: James Simmons <jsimmons@infradead.org>
>> >> > ---
>> >> > .../staging/lustre/lnet/libcfs/linux-tracefile.c   | 55 ++++++----------------
>> >> > 1 file changed, 14 insertions(+), 41 deletions(-)
>> >> > 
>> >> > diff --git a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> >> > index 3af7722..c1747c4 100644
>> >> > --- a/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> >> > +++ b/drivers/staging/lustre/lnet/libcfs/linux-tracefile.c
>> >> > @@ -140,54 +140,27 @@ enum cfs_trace_buf_type cfs_trace_buf_idx_get(void)
>> >> > void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
>> >> > 			  const char *buf, int len, const char *file,
>> >> > 			  const char *fn)
>> >> > {
>> >> > +	char *prefix = "Lustre";
>> >> > +
>> >> > +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
>> >> > +		prefix = "LNet";
>> >> > 
>> >> > -	if (mask & D_EMERG) {
>> >> > -		prefix = dbghdr_to_err_string(hdr);
>> >> > -		ptype = KERN_EMERG;
>> >> > +	if (mask & (D_CONSOLE | libcfs_printk)) {
>> >> 
>> >> This check is broken.  The default value of libcfs_printk (the mask
>> >> that controls which messages should be printed to the console, and
>> >> which ones should only be logged into the internal buffer) is:
>> >> 
>> >>     #define D_CANTMASK (D_ERROR | D_EMERG | D_WARNING | D_CONSOLE)
>> >> 
>> >> so that means virtually every console message will be printed with
>> >> pr_info() because this is matched first, instead of pr_emerg() or
>> >> pr_err() below.
>> >> 
>> >> That is why the previous code was matching D_EMERG and D_ERROR first,
>> >> then D_WARNING, and (D_CONSOLE | libcfs_printk) at the end.
>> >
>> > So to do this right we need:
>> >
>> > static void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
>> >                                  const char *buf, int len, const char 
>> > *file,
>> >                                  const char *fn)
>> > {
>> >         char *prefix = "Lustre";
>> >
>> >         if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
>> >                 prefix = "LNet";
>> >
>> >         if (mask & D_CONSOLE) {
>> >                 if (mask & D_EMERG) {
>> >                         pr_emerg("%sError: %.*s", prefix, len, buf);
>> >                 } else if (mask & D_ERROR) {
>> >                         pr_err("%sError: %.*s", prefix, len, buf);
>> >                 } else if (mask & D_WARNING) {
>> >                         pr_warn("%s: %.*s", prefix, len, buf);
>> >                 } else if (mask & libcfs_printk) {
>> >                         pr_info("%s: %.*s", prefix, len, buf);
>> >                 }
>> >         } else {
>> >                 if (mask & D_EMERG) {
>> >                         pr_emerg("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
>> >                                  hdr->ph_pid, hdr->ph_extern_pid, file,
>> >                                  hdr->ph_line_num, fn, len, buf);
>> >                 } else if (mask & D_ERROR) {
>> >                         pr_err("%sError: %d:%d:(%s:%d:%s()) %.*s", prefix,
>> >                                hdr->ph_pid, hdr->ph_extern_pid, file,
>> >                                hdr->ph_line_num, fn, len, buf);
>> >                 } else if (mask & D_WARNING) {
>> >                         pr_warn("%s: %d:%d:(%s:%d:%s()) %.*s", prefix,
>> >                                 hdr->ph_pid, hdr->ph_extern_pid, file,
>> >                                 hdr->ph_line_num, fn, len, buf);
>> >                 } else if (mask & libcfs_printk) {
>> >                         pr_info("%s: %.*s", prefix, len, buf);
>> >                 }
>> >         }
>> > }
>> 
>> That doesn't look right either.
>> The original code would *always* print something.
>> This code looks like it might not (e.g. for mask == 0).
>
> Is that correct behavior? A mask of zero means don't do anything.
> Also if you look at the original in the OpenSFS branch you can see
> if mask was to be 0 then ptype would be NULl :-( 

True... so it would print something, but the something would have (null)
in it.  So it probably never happens.

>
> Note a D_CANT_MASK prevents some fields in the mask from being disabled.
>
>> What is "D_CONSOLE" suppose to mean?  It seems to me "make the messages
>> less verbose" and it isn't clear to me that what is called "D_CONSOLE".
>
> It means two things. If by itself then it means use pr_info. If it is one
> field in the mask then it means less detail. Confusing no :-( That is my
> understanding of it. Maybe someone else can clarify if I'm wrong.

Yes, "confusing" seems an accurate description.  But if that is what it
is, then that is what it is.
If you send you new version as a patch, either against the original or
against the buggy version, I'll apply it.

Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180706/2cadd2aa/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console()
  2018-07-05 23:35         ` James Simmons
  2018-07-06  4:19           ` NeilBrown
@ 2018-07-06 17:09           ` Andreas Dilger
  1 sibling, 0 replies; 25+ messages in thread
From: Andreas Dilger @ 2018-07-06 17:09 UTC (permalink / raw)
  To: lustre-devel

On Jul 5, 2018, at 17:35, James Simmons <jsimmons@infradead.org> wrote:
> 
>>>>> void cfs_print_to_console(struct ptldebug_header *hdr, int mask,
>>>>> 			  const char *buf, int len, const char *file,
>>>>> 			  const char *fn)
>>>>> {
>>>>> +	char *prefix = "Lustre";
>>>>> +
>>>>> +	if (hdr->ph_subsys == S_LND || hdr->ph_subsys == S_LNET)
>>>>> +		prefix = "LNet";
>>>>> 
>>>>> -	if (mask & D_EMERG) {
>>>>> -		prefix = dbghdr_to_err_string(hdr);
>>>>> -		ptype = KERN_EMERG;
>>>>> +	if (mask & (D_CONSOLE | libcfs_printk)) {
>>>> 
>>>> This check is broken.  The default value of libcfs_printk (the mask
>>>> that controls which messages should be printed to the console, and
>>>> which ones should only be logged into the internal buffer) is:
>>>> 
>>>>    #define D_CANTMASK (D_ERROR | D_EMERG | D_WARNING | D_CONSOLE)
>>>> 
>>>> so that means virtually every console message will be printed with
>>>> pr_info() because this is matched first, instead of pr_emerg() or
>>>> pr_err() below.
>>>> 
>>>> That is why the previous code was matching D_EMERG and D_ERROR first,
>>>> then D_WARNING, and (D_CONSOLE | libcfs_printk) at the end.
>>> 
>>> 
>> 
>> The original code would *always* print something.
>> This code looks like it might not (e.g. for mask == 0).
> 
> Is that correct behavior? A mask of zero means don't do anything.
> Also if you look at the original in the OpenSFS branch you can see
> if mask was to be 0 then ptype would be NULl :-(
> 
> Note a D_CANT_MASK prevents some fields in the mask from being disabled.

Typically, users will set "lctl set_param debug=0" to disable debugging
messages for performance, but we still want critical error messages to
be printed to the console, so D_CANT_MASK is always checked for messages
to be printed to the console.

>> What is "D_CONSOLE" suppose to mean?  It seems to me "make the messages
>> less verbose" and it isn't clear to me that what is called "D_CONSOLE".
> 
> It means two things. If by itself then it means use pr_info. If it is one
> field in the mask then it means less detail. Confusing no :-( That is my
> understanding of it. Maybe someone else can clarify if I'm wrong.

The D_CONSOLE mask is used for "pretty" messages on the console, that will
not print out the file/function/line/timestamp and other debugging fields.
The D_CONSOLE flag doesn't necessarily mean "more" or "less" information,
but is intended more for "ease of understanding".

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud







-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 235 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180706/7b8454f0/attachment.sig>

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-07-06 17:09 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-27 19:38 [lustre-devel] [PATCH v3 00/13] lustre: libcfs: tracefile cleanups James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 01/13] lustre: libcfs: move tracefile locking from linux-tracefile.c to tracefile.c James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 02/13] lustre: libcfs: open code cfs_trace_max_debug_mb() into cfs_trace_set_debug_mb() James Simmons
2018-06-28 22:39   ` Andreas Dilger
2018-06-29  3:55     ` NeilBrown
2018-07-02  2:17       ` NeilBrown
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 03/13] lustre: libcfs: move tcd locking across to tracefile.c James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 04/13] lustre: libcfs: fix cfs_print_to_console() James Simmons
2018-06-27 21:27   ` NeilBrown
2018-06-28 22:30   ` Andreas Dilger
2018-06-29 16:17     ` James Simmons
2018-07-02  2:27       ` NeilBrown
2018-07-05 23:34         ` James Simmons
2018-07-05 23:35         ` James Simmons
2018-07-06  4:19           ` NeilBrown
2018-07-06 17:09           ` Andreas Dilger
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 05/13] lustre: libcfs: properly handle failure paths in cfs_tracefile_init_arch() James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 06/13] lustre: libcfs: merge linux-tracefile.c into tracefile.c James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 07/13] lustre: libcfs: remove cfs_trace_refill_stack() James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 08/13] lustre: libcfs: move cfs_trace_data data to tracefile.c James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 09/13] lustre: libcfs: cleanup tracefile.h James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 10/13] lustre: libcfs: fold cfs_tracefile_*_arch into their only callers James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 11/13] lustre: libcfs: renamed CFS_TCD_TYPE_MAX to CFS_TCD_TYPE_CNT James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 12/13] lustre: libcfs: discard TCD_MAX_TYPES James Simmons
2018-06-27 19:38 ` [lustre-devel] [PATCH v3 13/13] lustre: libcfs: format macros in tracefile.h James Simmons

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.