@kib both comments were right, and the second one pointed at a real gap.

@kib thanks for the approval. While organizing the patch set into logically independent commits, I noticed a potential minor issue with the version you approved, though.

@kib right, it is not needed either. A successful vn_start_write() keeps the mount reference that VOP_GETWRITEMOUNT() took until vn_finished_write() drops it. All three callers are between the two:

@kib OK, done. ffs_sync_alloc() is gone, and an allocating O_SYNC/O_DSYNC write finishes with ffs_syncvnode() again, as in the revision before last (DATA_ONLY for O_DSYNC, still only when the write changed i_blocks).

In D59356#1382368, @pho wrote:

In D59356#1382342, @sobomax wrote:

In D59356#1381929, @kib wrote:

I think this is fine. I suggest asking Peter to do the full stress2 run with the patch applied.

Hey @pho can you do this? Thanks!

Sure. I have been running tests for a day with D59356.188224.patch added. A full test usually takes 2 1/2 days on my test HW.

In D59356#1382368, @pho wrote:

In D59356#1382342, @sobomax wrote:

In D59356#1381929, @kib wrote:

I think this is fine. I suggest asking Peter to do the full stress2 run with the patch applied.

Hey @pho can you do this? Thanks!

Sure. I have been running tests for a day with D59356.188224.patch added. A full test usually takes 2 1/2 days on my test HW.

In D59356#1381929, @kib wrote:

I think this is fine. I suggest asking Peter to do the full stress2 run with the patch applied.

• usr.bin/stat/stat.1:86 ("'as reported ...' is an implementation detail") ---

@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:

@kib 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.