In D50315#1148924, @arrowd wrote:

I tried building it with make buildworld && make buildkernel and it failed with

--- libsys.so.7.full ---
Building shared library libsys.so.7
...
ld: error: version script assignment of 'FBSDprivate_1.0' to symbol '_inotify_add_watch' failed: symbol not defined
ld: error: version script assignment of 'FBSDprivate_1.0' to symbol '__sys_inotify_add_watch' failed: symbol not defined
ld: error: version script assignment of 'FBSDprivate_1.0' to symbol '_inotify_rm_watch' failed: symbol not defined
ld: error: version script assignment of 'FBSDprivate_1.0' to symbol '__sys_inotify_rm_watch' failed: symbol not defined
cc: error: linker command failed with exit code 1 (use -v to see invocation)

• Address some review comments.
• Rename INOTIFY_ND to INOTIFY_NAME and move those events to VOP hooks.
• When a watched file/dir itself triggers an event, the name stored in the event structure should be the empty string.

I'd rather not add new uses of the sticky bit.

In D50337#1148554, @christos wrote:

In D50337#1148551, @markj wrote:

In D50337#1148520, @christos wrote:

@markj If this fixes the issue mentioned in D48482, should we MFC it to releng/14.3 as well?

Yes, definitely.

Alright. Can I do it directly or do I need to get formal approval from re@?

In D50337#1148520, @christos wrote:

@markj If this fixes the issue mentioned in D48482, should we MFC it to releng/14.3 as well?

In D48482#1148395, @pkubaj wrote:

This change breaks booting for me on a Talos II board, both on powerpc64 and powerpc64le. Please revert, at least on releng/14.3.

Sort harder.

Remove some more unneeded includes.

In D50125#1147887, @kib wrote:

Being able to use the user thread context to help system thread to cope with load is beneficial. For instance, a lot of deadlocks in buffer cache were fixed by allowing the allocating thread to do some work for the buffer daemon. Compare the amount of tuning required for the page daemon, with the almost non-existent issues for the buffer cache.

In D50125#1147862, @kib wrote:

In D50125#1147853, @markj wrote:

Thinking about it some more, from my POV the bug is that we are applying a resource limit (maxvnodes) when there is no actual shortage (of memory).

Yes.

getnewvnode()/vn_alloc() seems to try very hard to avoid exceeding the desiredvnodes limit. It will even do direct reclaim and try to claim a vnode from the free list (this can have unpredictable latency that is quite large in the worst case, and we purposefully avoid this kind of mechanism for page reclamation for that reason) rather than exceed the limit. Meanwhile we have a dedicated thread whose sole responsibility is to reclaim unused vnodes.

This is not a small effort, but I would prefer to fix this by making maxvnodes a soft limit. That is, getnewvnode() should always allocate a vnode unless it's really unable to because of a memory shortage (or because we're getting "close" to a memory shortage by some metric). Let the vnlru thread handle the hard work of maintaining the vnode cache size, and keep the allocation path simple. Going further, if vnode allocation outpaces the vnlru thread in practices, then we can spin up additional threads to handle the load. It may also potentially be useful to use a PID controller (in subr_pidctrl.c) to regulate the vnlru thread. Even further, the vnode cache should react to pressure from the rest of the kernel and be able to dynamically shrink the cache in proportion to its total memory usage (the total number of vnodes plus auxiliary structures like v_pollinfo and namecache entries).

There are two big issues there:

1. getnewvnode() is not supposed to fail. It is claimed that the assumption is the bug in filesystems, but I think now that it is not. It should be similar to the pv allocator in pmap, always giving the resource on request.

In D50125#1147853, @markj wrote:

It will even do direct reclaim and try to claim a vnode from the free list (this can have unpredictable latency that is quite large in the worst case, and we purposefully avoid this kind of mechanism for page reclamation for that reason) rather than exceed the limit.

In D50125#1144527, @olce wrote:

In D50125#1143642, @markj wrote:

I tend to agree. I don't think this change really fixes the problem either: even with maxfiles larger than maxvnodes you're not safe, since multiple processes may open the same vnode, which counts multiple times against maxfiles but only once against maxvnodes.

I think you're seeing it backwards. Ensuring that kern.maxfiles is lower than kern.maxvnodes does fix the problem (modulo the leeway, see below), because then no process is able to continue to open file descriptors referencing not-yet opened files until kern.maxvnodes is exhausted, which is necessary to reach the deadlock point.

If your leeway is 1000, and you have 333 processes that hold /dev/null open over fds 0, 1, 2, then any process can still exhaust the limit.

No, because the limit we are talking about is the vnode limit, not the file descriptor limits (kern.maxfiles/kern.maxfilesperproc). In your example, it's the opposite: All vnodes are shared, so that limit is never reached and there can be no deadlocks; the file descriptor limits still apply though (without causing any problem).

Section 9.5 should be removed too, I think.

A couple more man page nits, otherwise this seems fine to me, modulo the other comment. I didn't carefully review the code changes.

I've been running this patch on my desktop. I'll commit it tomorrow if there are no objections.

This seems fine, sure.

In D50271#1146815, @emaste wrote:

Maybe "entire" build process isn't the way to express it. "Build process step" perhaps -- e.g., as if makefs was run at some given SOURCE_DATE_EPOCH.

Maybe "Behave as if the step in the build process was started at the time given by SOURCE_DATE_EPOCH and completed instantaneously."

In D50271#1146812, @emaste wrote:

Supporting clamping in makefs will require a bit more effort.

I'm not sure that's worth the effort. I've been discussing this in the reproducible-builds IRC channel and there seems to be a consensus:

• SOURCE_DATE_EPOCH isn't intended to override filesystem timestamps
• It can be interpreted as "Behave as if the entire build process occurred at the time specified by SOURCE_DATE_EPOCH"

In D50271#1146801, @emaste wrote:

The SOURCE_DATE_EPOCH spec doesn't explicitly state this, but based on these points I think it is the intent:

• Build processes MUST use this variable for embedded timestamps in place of the "current" date and time.
• Where build processes embed timestamps that are not "current", but are nevertheless still specific to one execution of the build process, they MUST use a timestamp no later than the value of this variable. This is often called "timestamp clamping".

I think for "use this timestamp instead of the real one from the filesystem" we do want an explicit flag rather than using SOURCE_DATE_EPOCH

Remove an incorrectly dropped timestamp setting.

In D50271#1146754, @emaste wrote:

Seems reasonable, but also SOURCE_DATE_EPOCH is intended to override the notion of "now" so should the routine return SOURCE_DATE_EPOCH if set or time(3) if not?

Fix the copyright.

• Correct the man page (-T overrides SOURCE_DATE_EPOCH, not the other way around)
• Use libutil's source_date_epoch()

This is missing a man page, I'll write one once there's some consensus on the interface.

• Remove expected failure annotations (assuming that the makefs code changes will be committed first)

• Fix the man page.
• Update mtree_global_inode as well.
• Factor out some duplicated code.

In D50192#1146268, @dumbbell wrote:

Beside the conflict with D49808, this looks good to me.

Your version of the code that conflicts with mine looks more complete now that the timeout is a long. Perhaps you should commit this one and I will rebase my patch on top of this.

Push the conversion into linux_add_to_sleepqueue().

In D50192#1145759, @olce wrote:

Why not call linux_jiffies_timeout_to_ticks() only once from linux_add_to_sleepqueue(), in the concerned callee, instead of in all callers?

In D50192#1145248, @bz wrote:

Does __wait_event_common need similar treatment?

Deduplicate jiffies->ticks conversions, use a local variable instead of casting.

In D50104#1145336, @jhb wrote:

So maybe one commit to update the krb5 code to use crp_opaque = NULL and then you can just remove the flag entirely in a second commit? The changes look ok to me though (I'm just also fine with now removing the flag entirely)

Apply John's suggestion of splitting the diff into two.