It is not legal to hold any vnode locks across a call to vn_fullpath. In particular having the vp locked threatens a deadlock against normal path lookup, as the reconstruction grabs one lock at a time the opposite way. So you can have the child locked and stall waiting for the parent, while the forward lookup just locked the parent and now wants the child.

In D60894#1397856, @glebius wrote:

In D60894#1397813, @mjg wrote:

I did not see any point in benchmarking this, it is strictly less work in the fast path.

This is definitely not the right attitude when submitting patches that add more code and are declared as a performance improvement.

sigh, use correct diff

I did not see any point in benchmarking this, it is strictly less work in the fast path.

• guard CTASSERT with _KERNEL. verified it fixes buildworld

While I agree the fence is missing, I completely disagree with the fix.

This greatly reduces the race window, but it does not fix the problem.

fixed in https://cgit.freebsd.org/src/commit/?id=d6138a65405f697715189363b2b18581e7abd982

I'm fine either way.

So the ranged bitmap proposal works, but it also has funny corner cases if fds are very sparse which perhaps are not worth it.

I did not co-autor the change, so please drop that tag.

these also need to go:

This looks legitimate.

I checked sx and rw locks and I only see the check in the slowpath.

Do you have stats how many classes can be realistically present? maybe this would be way faster single-threaded by iterating an array?

I'll be posting a different variant later today.

I think some work can be saved if the errno is ENOENT or EACCESS, but someone(tm) has to make sure errors of the sort are not returned from the stage where the syscall does the reverse lookup.

In D53070#1212811, @glebius wrote:

In D53070#1212661, @mjg wrote:

This *is* used by pf_rule_counter_u64_periodic. The goal is to sum up the 32 bit values frequently enough(tm) that they don't overflow.

Then probably better to rename the field from allrulelist to markerlist.

This *is* used by pf_rule_counter_u64_periodic. The goal is to sum up the 32 bit values frequently enough(tm) that they don't overflow.

landed

• remove goto labels

• sort includes
• fix a bug found by pho@ in the lock smr patch: after the lock is acquired, doomed handling has to be the same as with the interlock-protected routine

• patch up commentary
• remove a debug tunable

In D52819#1207028, @kib wrote:

This is not a complete patch, I think? The other bits except smr lock are missing.

• upload the entire thing

• also drop ref drip around unlock

• doomed asserts
• cosmetic fixups to null_hashget_locked
• null_lock using smr

I think this is turning into a bikeshed, so just do what you feel.

The above may be true and the pointer may be populated depending on when you got the the inode. I would just add something like "->i_din2 is NULL, some information omitted" or compatible.

maybe print that the pointer is NULL?

I don't have an opinion on the functionality, but the implementation looks avoidably slow.

• patch up some comments
• add the missing acq fence

vput et al are the primary consumers of vdrop.

Huh, so happens vtryrecycle unlocks and then vdrops, so it happens to not run into the problem after all. General remark stands though.