diff mbox series

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

Message ID 20211011021124.86248-1-ligang.bdlg@bytedance.com (mailing list archive)
State New
Headers show
Series [v3] mm: mmap_lock: remove "\n" in TP_printk and use DECLARE_EVENT_CLASS | expand

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 <ligang.bdlg@bytedance.com>
---
 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 <ligang.bdlg@bytedance.com> 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 <ligang.bdlg@bytedance.com>
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.
diff mbox series

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 */