@kib reimplemented atop SFBSD_MNTPOINT and made into a stat(1) alias. Please re-review, thanks!

@kib can you please flush this down the pipe since everyone seems in agreement, so it unblocks the D59906? 🙏

@kib agreed, that was not proportional. The new revision replaces the ffs_syncvnode() call with a targeted flush, ffs_sync_alloc(), that only makes the pointers to the written range durable:

Thanks, all three addressed in the update. Every restart of the dependency selection now goes through the EJUSTRETURN return to flush_pagedep_deps(), and nothing is kept on the private unfinished list while the softdep lock is dropped for a restart.

@kib thanks, updated.

@kib Thanks, both done in the update:

Add missing src.opts.mk.

@kib updated as requested, thanks

Updated, relative to the previous revision (874f9dd09fea):

In D59356#1378462, @pho wrote:

Nice test scenario!
Reproduced: https://people.freebsd.org/~pho/stress/log/log0694.txt
As mkdir_blkreuse.sh is a shebang script, the file must have the x bit set.

In D59356#1375508, @kib wrote:

In D59356#1375502, @sobomax wrote:

@pho I tried to coerce ChatGPT Codex to write a reproducer, but it keep refusing claiming that's a security concern. I will try maybe to rephrase it as writing a "unit test to validate the fix."

Peter is exceptionally good in creating the test scenarios.

I suggest you to explain the workload in more details, at the low level. I.e. instead of saying 'we install set of packages on memory disk' say something like 'we create UFS on swap-backed(?) md(4) volume of that size, with SU (or SU+J?) enabled, then create that deep file hierarchy and create that many files, each of that size'.

Add a reproducer, worth $30 of Claude Fable credits.

P.S. Perhaps bump __FreeBSD_version? Since there is no other way to probe if the kernel supports this feature or not.

@pho I tried to coerce ChatGPT Codex to write a reproducer, but it keep refusing claiming that's a security concern. I will try maybe to rephrase it as writing a "unit test to validate the fix."

New flag needs to be documented in the stat(2).

Incorporate @kib suggestions.

In D59910#1374935, @kib wrote:

In D59910#1374910, @sobomax wrote:

something along those lines:

Sure, documenting st_bsdflags would be useful. I suggest you to create a review for the diff, I have some notes.

something along those lines:

Looks good, however maybe it's time to document both st_bsdflags member and its respective values in the stat(2) as non-portable interface?

In D59356#1363271, @pho wrote:

In D59356#1363214, @kib wrote:

From the first look, this is fine.
I believe the next step is to help Peter (pho@) to create the reproducer, and get the stress2 testing.
Also, please wait for Kirk' feedback.

That would be nice. I have been seeing the panic a few times over the years, but not after Feb 10 2021.

Rework to preserve dependencies as suggested by @kib

In D59356#1362256, @kib wrote:

The new 'find' logic essentially reorders the processing of the newblk dependencies, and I am not sure that this is fine in this case (it is generally not).

IMO the correct solution for the race in flush_pagedep_deps() as described in the summary is to memoize the vnode returned by get_parent_vp(), and then restart the global loop. When you see the MKDIR_BODY dependency, and the inode number matches the memoized result from get_parent_vp(), you can process it without relocking sd_fslock, which closes the race. If the vp is NULL or does not match the inode number, you vput() it, call get_parent_vp(), and restart the loop.

Some of the facts supporting this diagnosis:

• fix buildworld
• added KASSERT_PANIC_NOCONTEXT option allowing to avoid compiling extra location strings
• made format a bit closer to what MPASS4() would produce today
• improve wording on panic(9) and update KASSERT(9) to match.

In D56168#1284968, @glebius wrote:

The code placement is ad-hoc. We already have a case for IPPROTO_TCP in the main switch and the new option handling should go there. This will require some extension of linux_to_bsd_tcp_sockopt() but the result would be cleaner code.

Implement suggestions from @glebius

Here is updated savings table:

All weak symbols are now gone.

Refresh with the latest -CURRENT. Get rid of the --whole-archive, with the latest toolchain the linking now works properly in both LTO and non-LTO mode.

Use :ts

In D49677#1132641, @tuexen wrote:

If I understand the patch correctly, it contains three components:

• the fix by replacing in_delayed_cksum(m) with in_delayed_cksum_o(m, ipofs) in ng_nat.c.
• adding protection code which logs a warning.
• refactoring the code to remove indentation by using goto send.

It might be useful to separate these three when committing the code.

printf() -> log(). Fix type mismatch.

Fix error message.

Fix mismerge.

Mostly bogus. Sorry for the noise.