Thread (67 messages) 67 messages, 8 authors, 2022-03-10

Re: [PATCH 02/16] dept: Implement Dept(Dependency Tracker)

From: Byungchul Park <hidden>
Date: 2022-02-18 06:09:45
Also in: dri-devel, linux-block, linux-ext4, linux-fsdevel, linux-mm, lkml

On Thu, Feb 17, 2022 at 12:36:56PM -0500, Steven Rostedt wrote:
quoted
+struct dept_ecxt;
+struct dept_iecxt {
+	struct dept_ecxt *ecxt;
+	int enirq;
+	bool staled; /* for preventing to add a new ecxt */
+};
+
+struct dept_wait;
+struct dept_iwait {
+	struct dept_wait *wait;
+	int irq;
+	bool staled; /* for preventing to add a new wait */
+	bool touched;
+};
Nit. It makes it easier to read (and then review) if structures are spaced
where their fields are all lined up:

struct dept_iecxt {
	struct dept_ecxt		*ecxt;
	int				enirq;
	bool				staled;
};

struct dept_iwait {
	struct dept_wait		*wait;
	int				irq;
	bool				staled;
	bool				touched;
};

See, the fields stand out, and is nicer on the eyes. Especially for those
of us that are getting up in age, and our eyes do not work as well as they
use to ;-)
Sure! I will apply this.
quoted
+ * ---
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your ootion) any later version.
+ *
+ * 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 for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you can access it online at
+ * http://www.gnu.org/licenses/gpl-2.0.html.
The SPDX at the top of the file is all that is needed. Please remove this
boiler plate. We do not use GPL boiler plates in the kernel anymore. The
SPDX code supersedes that.
Thank you for informing it!
quoted
+/*
+ * Can use llist no matter whether CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG is
+ * enabled because DEPT never race with NMI by nesting control.
                         "never races with"
Good eyes!
Although, I'm confused by what you mean with "by nesting control".
I should've expressed it more clearly. It meant NMI and other contexts
never run inside of Dept concurrently in the same CPU by preventing
reentrance.
quoted
+static void initialize_class(struct dept_class *c)
+{
+	int i;
+
+	for (i = 0; i < DEPT_IRQS_NR; i++) {
+		struct dept_iecxt *ie = &c->iecxt[i];
+		struct dept_iwait *iw = &c->iwait[i];
+
+		ie->ecxt = NULL;
+		ie->enirq = i;
+		ie->staled = false;
+
+		iw->wait = NULL;
+		iw->irq = i;
+		iw->staled = false;
+		iw->touched = false;
+	}
+	c->bfs_gen = 0U;
Is the U really necessary?
I was just wondering if it's really harmful? I want to leave this if
it's harmless because U let us guess the data type of ->bfs_gen correctly
at a glance. Or am I missing some reason why I should fix this?

Thank you very much, Steven.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help