[v3] mm: mmap_lock: remove "\n" in TP_printk and use DECLARE_EVENT_CLASS

Commit Message

Gang Li Oct. 11, 2021, 2:11 a.m. UTC

Ftrace core will add "\n" automatically on print. "\n" in TP_printk
will create blank line, so remove it.

By using DECLARE_EVENT_CLASS and TRACE_EVENT_FN, we can save a lot
of space from duplicate code.

Signed-off-by: Gang Li <[e-mail]>
---
 include/trace/events/mmap_lock.h | 48 ++++++++++----------------------
 1 file changed, 14 insertions(+), 34 deletions(-)

Comments

Steven Rostedt Oct. 11, 2021, 2:59 a.m. UTC | #1

On Mon, 11 Oct 2021 10:11:24 +0800
Gang Li <[e-mail]> wrote:

> Ftrace core will add "\n" automatically on print. "\n" in TP_printk
> will create blank line, so remove it.
> 
> By using DECLARE_EVENT_CLASS and TRACE_EVENT_FN, we can save a lot
> of space from duplicate code.

Why did you send this? It should be two patches, not one. The rule is,
every commit does one thing. Now you made this patch do two.

Andrew already pulled in the other two patches. I don't think this one
is appropriate, and should be discarded.

-- Steve


> 
> Signed-off-by: Gang Li <[e-mail]>

Gang Li Oct. 11, 2021, 5:53 a.m. UTC | #2

On 10/11/21 10:59 AM, Steven Rostedt wrote:

 > Why did you send this? It should be two patches, not one. The rule is,
 > every commit does one thing. Now you made this patch do two.
 >
 > Andrew already pulled in the other two patches. I don't think this one
 > is appropriate, and should be discarded.
 >
 > -- Steve

My fault. Obviously you are talking about events, not patches. I 
misunderstood your previous email.


 > Thanks for doing this. Looking at the events, they can certainly be
 > merged into one.


This patch v3 should be discarded.

Patch

diff --git a/include/trace/events/mmap_lock.h b/include/trace/events/mmap_lock.h
index 0abff67b96f0..14db8044c1ff 100644
--- a/include/trace/events/mmap_lock.h
+++ b/include/trace/events/mmap_lock.h
@@ -13,7 +13,7 @@  struct mm_struct;
 extern int trace_mmap_lock_reg(void);
 extern void trace_mmap_lock_unreg(void);
 
-TRACE_EVENT_FN(mmap_lock_start_locking,
+DECLARE_EVENT_CLASS(mmap_lock,
 
 	TP_PROTO(struct mm_struct *mm, const char *memcg_path, bool write),
 
@@ -32,15 +32,23 @@  TRACE_EVENT_FN(mmap_lock_start_locking,
 	),
 
 	TP_printk(
-		"mm=%p memcg_path=%s write=%s\n",
+		"mm=%p memcg_path=%s write=%s",
 		__entry->mm,
 		__get_str(memcg_path),
 		__entry->write ? "true" : "false"
-	),
-
-	trace_mmap_lock_reg, trace_mmap_lock_unreg
+	)
 );
 
+#define DEFINE_MMAP_LOCK_EVENT(name)                                    \
+	DEFINE_EVENT_FN(mmap_lock, name,                                \
+		TP_PROTO(struct mm_struct *mm, const char *memcg_path,  \
+			bool write),                                    \
+		TP_ARGS(mm, memcg_path, write),                         \
+		trace_mmap_lock_reg, trace_mmap_lock_unreg)
+
+DEFINE_MMAP_LOCK_EVENT(mmap_lock_start_locking);
+DEFINE_MMAP_LOCK_EVENT(mmap_lock_released);
+
 TRACE_EVENT_FN(mmap_lock_acquire_returned,
 
 	TP_PROTO(struct mm_struct *mm, const char *memcg_path, bool write,
@@ -63,7 +71,7 @@  TRACE_EVENT_FN(mmap_lock_acquire_returned,
 	),
 
 	TP_printk(
-		"mm=%p memcg_path=%s write=%s success=%s\n",
+		"mm=%p memcg_path=%s write=%s success=%s",
 		__entry->mm,
 		__get_str(memcg_path),
 		__entry->write ? "true" : "false",
@@ -73,34 +81,6 @@  TRACE_EVENT_FN(mmap_lock_acquire_returned,
 	trace_mmap_lock_reg, trace_mmap_lock_unreg
 );
 
-TRACE_EVENT_FN(mmap_lock_released,
-
-	TP_PROTO(struct mm_struct *mm, const char *memcg_path, bool write),
-
-	TP_ARGS(mm, memcg_path, write),
-
-	TP_STRUCT__entry(
-		__field(struct mm_struct *, mm)
-		__string(memcg_path, memcg_path)
-		__field(bool, write)
-	),
-
-	TP_fast_assign(
-		__entry->mm = mm;
-		__assign_str(memcg_path, memcg_path);
-		__entry->write = write;
-	),
-
-	TP_printk(
-		"mm=%p memcg_path=%s write=%s\n",
-		__entry->mm,
-		__get_str(memcg_path),
-		__entry->write ? "true" : "false"
-	),
-
-	trace_mmap_lock_reg, trace_mmap_lock_unreg
-);
-
 #endif /* _TRACE_MMAP_LOCK_H */
 
 /* This part must be outside protection */