So a general question I have is, why don't we simply always lazily do this processing. Just leave it on the active list. With no flag or anything. During the active scan if the holdcnt is zero move it to the free list. All of this trylock and flags is a lot of complexity to save the cost of scanning and requeuing later when it can be done more efficiently in bulk.

You can look at being on the active queue as having a single activation count. If you want to optimize the LRU you can add an atomic flag for vnodes that are 'reactivated'. So vnodes with a single ref are moved out of active but those that are regularly reactivated never leave.

Since this is bool it can/should be !refcount_release

Shouldn't these be inverted? We're now skipping the work if these conditions are true. We should only skip it if they are false.

I see I failed to mention in the original text that the issue at hand is a significant bottleneck on modest contemporary machines. Thus I wanted the simplest approach which is legible for MFC to stable/12.

I did note I don't consider this to be a good long term fix.

The proposed modification of letting more vnodes stay on the active list is what I considered originally. That is, syncer would check for a "used" bit -- if present it would remove it and requeue the vnode to the end, if not present it would return the vnode to the free list. It's not hard to extend this code to do it, but I don't think it a worthwhile addition given the temporary nature of this patch.

I think a hard requirement for long run solution is numa-awareness. I don't have it completely thought out, but a rough sketch involves removal of the global free list and per-mount point active lists. Instead, all aging state would be managed per-domain (may be with clock or a different algorithm).

This was copied from vdrop, can adjust both.

This is the original condition but inverted (note the "!").

So I took a stab at making this second chance-y and that made me even more confident it's not the right approach if doing anything remotely more involved that the patch as proposed here.

Key for me is that some filesystems sometimes want to traverse a list of vnodes which can have outstanding i/o to perform. Currently there is no dedicated list of the sort and instead the "active" list is filtered. Imo we should add new list to mount point/vnode relationship and add there vnodes where it is likely they will match this criterion. It should be easy to be strict about addition (opens O_RDWR and similar), while we can be very lax about removing them from the list. Possibly in the very form as used in this patch when it comes to active list.

With these out of the way we can have completely unrelated linkage used for aging. That in particular can be made per-domain instead of per-fs (or we can start with just keeping it global). It would be easy to make it into a variant of clock or whatever else which proves adequate.

Additional feature of said list can be handling of deferred inactive. vdrop sees VI_OWEINACT and leaves a 0-held vnode on the active list which I consider to be a total hack. With the list at hand we can easily keep the ref and denote the vnode with a flag meaning syncer is resposible for inactive and vdrop. Finally there may be places which want to drop usecount but can't afford risking inactive. They could also use this facililty (vput_async?)

Opinions?

This could all be placed in the iterator instead of using msync for something unrelated. You would still need to guarantee that someone iterated at some point though.

Alternatively if you implemented my idea of the active iterator grabbing a hold count this happens as a side effect.

This is super hard to read. You should invert the individual tests rather than the whole block.

This would change semantics of the loop making it more problematic to MFC so I don't think it's a good idea.

tests are the same so that's it's easy to see it's the same stuff. Instead of inverting like this I can "== 0" or better yet move all the conditions into an inline ("vfs_want_msync"?) and call it in both places.