Convert bunch of macros from sys/mount.h into static inlines

[kib]

Summary

This gives type safety without loosing compiler optimizations

vfs: convert vfs_op_thread_* macros to static inlines


sys/vnode.h: convert VN_KNOTE() to function


vfs: convert VFS_OPs from macros to static inlines


sys/vnode.h: remove stale comment

The source sweep is not going to happen.

[markj]

sys/sys/mount.h

859

sys/sys/vnode.h

229

The list-is-empty check is no longer inlined, now it requires an extra function call in the common case where there are no knotes present.

I'd actually like to add a new VIRF flag which gets set when the knote list is non-empty, similar to VIRF_INOTIFY. Then, instead of checking v->pollinfo != NULL && !KNLIST_EMPTY(&...) and vp->v_irflag & VIRF_INOTIFY separately in various VOP _post implementations, they can look like this:

void
vop_open_post(void *ap, int rc)
{
    struct vop_open_args *a = ap;

    if (rc == 0 && __predict_false((a->a_vp->v_irflag & (VIRF_INOTIFY | VIRF_KNOTE)))) {
        <slow path>
    }
}

[kib]

Coded VIRF_KNOTE.

The issue I see is that for vn_pollrecord() must interlock the enqueuing of of a knote with setting the VIRF_KNOTE flag, which creates the order between the vnode interlock and pollinfo vpi_lock mutex. I do not think there was an order before, then it should be fine for now. But even if it is, it is somewhat limiting the future code modifications.

[markj]

In D56611#1296692, @kib wrote:

Coded VIRF_KNOTE.

The issue I see is that for vn_pollrecord() must interlock the enqueuing of of a knote with setting the VIRF_KNOTE flag, which creates the order between the vnode interlock and pollinfo vpi_lock mutex. I do not think there was an order before, then it should be fine for now. But even if it is, it is somewhat limiting the future code modifications.

Hmm but the knote list is locked by the vnode lock, not by the pollinfo lock. See vfs_knllock().

I'm not sure why it was done this way. Maybe it's an optimization, since many VOPs already hold the vnode lock when they scan the list?

sys/kern/vfs_subr.c

5329

Shouldn't we set this in vfs_kqfilter(), not here?

5821

Maybe vop_check_pollinfo()? And I would annotate this with __predict_false.

[kib]

In D56611#1297270, @markj wrote:

In D56611#1296692, @kib wrote:

Coded VIRF_KNOTE.

The issue I see is that for vn_pollrecord() must interlock the enqueuing of of a knote with setting the VIRF_KNOTE flag, which creates the order between the vnode interlock and pollinfo vpi_lock mutex. I do not think there was an order before, then it should be fine for now. But even if it is, it is somewhat limiting the future code modifications.

Hmm but the knote list is locked by the vnode lock, not by the pollinfo lock. See vfs_knllock().

Right. And with the correction of setting the flag in vfs_kqfilter, the flag is naturally protected by the vnode lock. This makes it more natural to have it in v_vflag than in v_irflag (but I have same opinion about VIRF_INOTIFY).

I'm not sure why it was done this way. Maybe it's an optimization, since many VOPs already hold the vnode lock when they scan the list?

[kib]

Set VIRF_KNOTE in vfs_kqfilter(), where it should be, on inserting knote into the knlist.
Rename vop_check_knote() to vop_check_pollinfo().

[kib]

Do not set VIRF_KNOTE in knlist unlock, vfs_kqfilter() should be enough.

[markj]

In D56611#1297281, @kib wrote:

In D56611#1297270, @markj wrote:

In D56611#1296692, @kib wrote:

Coded VIRF_KNOTE.

The issue I see is that for vn_pollrecord() must interlock the enqueuing of of a knote with setting the VIRF_KNOTE flag, which creates the order between the vnode interlock and pollinfo vpi_lock mutex. I do not think there was an order before, then it should be fine for now. But even if it is, it is somewhat limiting the future code modifications.

Hmm but the knote list is locked by the vnode lock, not by the pollinfo lock. See vfs_knllock().

Right. And with the correction of setting the flag in vfs_kqfilter, the flag is naturally protected by the vnode lock. This makes it more natural to have it in v_vflag than in v_irflag (but I have same opinion about VIRF_INOTIFY).

My main reason for putting it in v_irflag was because the flag is checked "frequently." If we had a frequently read flag field which is synchronized by the vnode lock, then that would be fine too.

[kib]

In D56611#1297289, @markj wrote:

In D56611#1297281, @kib wrote:

Right. And with the correction of setting the flag in vfs_kqfilter, the flag is naturally protected by the vnode lock. This makes it more natural to have it in v_vflag than in v_irflag (but I have same opinion about VIRF_INOTIFY).

My main reason for putting it in v_irflag was because the flag is checked "frequently." If we had a frequently read flag field which is synchronized by the vnode lock, then that would be fine too.

We have 7 bytes unused in the v_rl rangelock member of struct vnode. I will propose something after this batch is landed.

[markj]

sys/kern/vfs_subr.c

6725–6729

Why vholdl()?

When I run the regression test suite, the sys/kern/path_test tests trigger a panic:

VNASSERT failed: mtx_owned(VI_MTX(vp)) not true at /home/markj/sb/main/src/sys/kern/vfs_subr.c:5761 (assert_vi_locked)                                         
0xfffffe019af43dc0: type VREG state VSTATE_CONSTRUCTED op 0xffffffff830c6380                                                                                   
    usecount 1, writecount 0, refcount 3 seqc users 0                                                                                                          
    hold count flags ()                                                                                                                                        
    flags (VIRF_PGREAD|VMP_LAZYLIST)                                                                                                                           
    v_object 0xfffffe01991cec30 ref 0 pages 1 cleanbuf 0 dirtybuf 1                                                                                            
    lock type ufs: UNLOCKED                                                                                                                                    
        nlink=1, effnlink=1, size=1024, extsize=0                                                                                                              
        generation=604727da, uid=0, gid=0, flags=0x0                                                                                                                                                                                                                                                                          
        ino 886830, on dev gpt/rootfs                                                                                                                          
panic: vfs_kqfilter: vnode interlock is not locked but should be                                                                                               
cpuid = 11                                                                                                                                                     
time = 1777327121                                                                                                                                              
KDB: stack backtrace:                                                                                                                                          
db_trace_self_wrapper() at db_trace_self_wrapper+0xa5/frame 0xfffffe00f17cdfd0                                                                                 
kdb_backtrace() at kdb_backtrace+0xc6/frame 0xfffffe00f17ce130                                                                                                                                                                                                                                                                
vpanic() at vpanic+0x214/frame 0xfffffe00f17ce2d0                                                                                                                                                                                                                                                                             
panic() at panic+0xb5/frame 0xfffffe00f17ce390                                                                                                                 
vfs_kqfilter() at vfs_kqfilter+0x2a7/frame 0xfffffe00f17ce440                                                                                                  
VOP_KQFILTER_APV() at VOP_KQFILTER_APV+0x7d/frame 0xfffffe00f17ce470                                                                                           
vn_kqfilter_opath() at vn_kqfilter_opath+0xe1/frame 0xfffffe00f17ce530                                                                                         
kqueue_register() at kqueue_register+0xa24/frame 0xfffffe00f17ce690                                                                                            
kqueue_kevent() at kqueue_kevent+0x1b4/frame 0xfffffe00f17ce9c0                                                                                                
kern_kevent_fp() at kern_kevent_fp+0xce/frame 0xfffffe00f17cea10                                                                                                                                                                                                                                                              
kern_kevent() at kern_kevent+0x12f/frame 0xfffffe00f17ceaf0                                                                                                                                                                                                                                                                   
kern_kevent_generic() at kern_kevent_generic+0x135/frame 0xfffffe00f17cebd0                                                                                                                                                                                                                                                   
sys_kevent() at sys_kevent+0x232/frame 0xfffffe00f17ced10                                                                                                                                                                                                                                                                     
amd64_syscall() at amd64_syscall+0x3d5/frame 0xfffffe00f17cef30                                                                                                                                                                                                                                                               
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe00f17cef30                                                                                     
--- syscall (560, FreeBSD ELF64, kevent), rip = 0xdd0a4b1420a, rsp = 0xdd0a201fc88, rbp = 0xdd0a2020110 ---

[kib]

sys/kern/vfs_subr.c

6725–6729

Sorry for it. The change was a left-over from using the interlock for setting the flag, before I changed to vnode lock.

[kib]

vhold