Re: [PATCH net-next 0/2] can: publish tty->disc_data only after register_candev()

[Chaosheng Qu <chaosheng.qu.linux@gmail.com>]

Hi Max,

Thank you for the thorough review, and for taking the time to work out why the
patch felt wrong. You are right, and I am dropping this approach.

The point I missed is the first one you made: "ldisc first, then netdev" is an
implicit contract in these two drivers, and my patch interleaves it. Even if
nothing races today, the next person to touch these functions would have to
reason about SMT safety that was previously not theirs to reason about. That is
swapping one trap for another, which is exactly what I claimed to be removing.

So v3 will use your alternative instead of mine, and nothing else:

	err = register_candev(dev);
	if (err) {
+		tty->disc_data = NULL;
		free_candev(dev);
		pr_err("can't register candev\n");
		return err;
	}

Same in can327. The ordering stays as it is, so the implicit contract is
untouched, and the failure path no longer leaves tty->disc_data pointing at
memory that free_candev() has just released. The commit message will say that
plainly, and will state why no barrier or extra locking is needed: the pointer
is read exactly where it was read before, and the only write on this path
happens before the interface can be brought up.

The sashiko report on 1/2 (a lost tty_wakeup while ldisc_sem is held) also goes
away with your version, since nothing is reordered any more.

On your question: yes. I use an AI assistant when I write patches, and the
disclosure line required by Documentation/process/coding-assistants.rst will be
on v3. Your instinct about why this one looked too neat is right: the assistant
was good at exactly one thing here, finding the smallest diff that removes the
dangling pointer, and moving the assignment is a smaller diff than clearing it.
That is the move that quietly crossed the SMT boundary. Knowing when the
smaller diff is the wrong one is still my job, and I got it wrong.

v3 in a day or two, after I have re-read both drivers end to end rather than
trusting the diff.

Thanks,
Chaosheng