Skip to contentSkip to conversation

tracing: Do not clean up hiter in mmiotrace read function

PATCH New Steven Rostedt 1 participant 3 messages Sep 10

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover tracing: Do not clean up hiter in mmiotrace read function

When the mmiotrace trace was first created, it allocated a descriptor in its pipe_open() method. Since there was no pipe_close() method when it was created (in May of 2008, and pipe_close() was added in December of 2009), it cleaned up the allocated descriptors in the read.

Now that the clean up is in the pipe_close() method that now exists, remove the clean up from the read as it is no longer needed.

Also simplify the code by inverting the early exit conditional into a conditional to perform the logic and get rid of the goto.

Changse since v1: https://lore.kernel.org/linux-trace-kernel/[email protected]/

- Broke it up into two patches. Nothing else changed.

Steven Rostedt (2):
tracing: Do not clean up hiter in mmiotrace read function
tracing: Invert the logic in mmio_read() to get rid of the goto

----

 kernel/trace/trace_mmiotrace.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)
#1 lore raw

PATCH v2 1/2 tracing: Do not clean up hiter in mmiotrace read function

From: Steven Rostedt <[email protected]>

When the mmiotrace trace was first created, it allocated a descriptor in its pipe_open() method. Since there was no pipe_close() method when it was created (in May of 2008, and pipe_close() was added in December of 2009), it cleaned up the allocated descriptors in the read.

Now that the clean up is in the pipe_close() method that now exists, remove the clean up from the read as it is no longer needed.

---
Changses since v1 : https://patch.msgid.link/[email protected]

- Broke it up into two patches

 kernel/trace/trace_mmiotrace.c | 5 -----
 1 file changed, 5 deletions(-)
kernel/trace/trace_mmiotrace.c +0 -5
index 77120d467e11..0034d6af3d9d 100644
@@ -152,11 +152,6 @@ static ssize_t mmio_read(struct trace_iterator *iter, struct file *filp,
 	mmio_print_pcidev(s, hiter->dev);
 	hiter->dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, hiter->dev);
 
-	if (!hiter->dev) {
-		destroy_header_iter(hiter);
-		iter->private = NULL;
-	}
-
 print_out:
 	ret = trace_seq_to_user(s, ubuf, cnt);
 	return (ret == -EBUSY) ? 0 : ret;
Signature · 1 line
2.53.0
#2 lore raw

PATCH v2 2/2 tracing: Invert the logic in mmio_read() to get rid of the goto

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Simplify the logic by removing the goto in the first condition and inverting the logic of the early return.

This changes the reading logic slightly. Before, the goto would exit the function right away with the output of missed events. The next read would then update print the saved hiter->dev and update it.

Now, in one iteration, if count_overruns() retruns !0, the lost events is printed as well as the next hiter->dev without needing a second loop.

---
Changes since v1: https://patch.msgid.link/[email protected]

- Broke it up into two patches

 kernel/trace/trace_mmiotrace.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
kernel/trace/trace_mmiotrace.c +4 -7
index 0034d6af3d9d..c49738cdb439 100644
@@ -143,16 +143,13 @@ static ssize_t mmio_read(struct trace_iterator *iter, struct file *filp,
 		if (!overrun_detected)
 			pr_warn("mmiotrace has lost events\n");
 		overrun_detected = true;
-		goto print_out;
 	}
 
-	if (!hiter || !hiter->dev)
-		return 0;
-
-	mmio_print_pcidev(s, hiter->dev);
-	hiter->dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, hiter->dev);
+	if (hiter && hiter->dev) {
+		mmio_print_pcidev(s, hiter->dev);
+		hiter->dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, hiter->dev);
+	}
 
-print_out:
 	ret = trace_seq_to_user(s, ubuf, cnt);
 	return (ret == -EBUSY) ? 0 : ret;
 }
Signature · 1 line
2.53.0