r150074 had a relevant comment here: +/* + * Because fifos are now a file descriptor layer object, EVFILT_VNODE is not + * implemented. Likely, fifo_kqfilter() should be removed, and + * fifo_kqfilter_f() should know how to forward the request to the underling + * vnode using f_vnode in the file descriptor here. + */ My main interest is in having NOTE_DELETE work. This impacts tail -F as it tries to use NOTE_DELETE|NOTE_RENAME but gets EINVAL and then falls into a sleep+stat+read loop. kevent(4,{ 3,EVFILT_VNODE,EV_ADD|EV_ENABLE|EV_CLEAR,NOTE_DELETE|NOTE_RENAME,0x0,0x0 3,EVFILT_READ,EV_ADD|EV_ENABLE|EV_CLEAR,0x0,0x0,0x0 },2,0x0,0,{ 0.000000000 }) ERR#22 'Invalid argument'
Indeed, at the end of fifo_open() we set the file's ops table to pipeops. pipe_kqfilter() has no way of attaching a knote to the fifo vnode. We could perhaps handle it with a new file ops table for fifos: every operation except kqfilter would just be the corresponding operation in pipeops. fifo_kqfilter() would attempt to handle EVFILT_VNODE by calling vfs_kqfilter() and otherwise would call pipe_kqfilter(). Another option might be to maintain a backpointer from the pipe structure to the vnode for named pipes. BTW there is some dead code here, I can't see how ufsfifo_kqfilter() can ever get called.
I've seen some places in sys_pipe.c where operations are delegated to vnode operations (in the fifo case). Could something like the following work? --- a/sys/kern/sys_pipe.c +++ b/sys/kern/sys_pipe.c @@ -1707,6 +1707,10 @@ pipe_kqfilter(struct file *fp, struct knote *kn) cpipe = PIPE_PEER(cpipe); break; default: + if (cpipe->pipe_state & PIPE_NAMED) { + PIPE_UNLOCK(cpipe); + return (vnops.fo_kqfilter(fp, kn)); + } PIPE_UNLOCK(cpipe); return (EINVAL); } Maybe vop_kqfilter/VOP_KQFILTER could be removed completely in favor of vfs_kqfilter() as the only implementation? It looks like this mechanism was only needed to forward *_kqfilter calls to fifo's vop_kqfilter. But now it's done the other way around in a sense.
(In reply to Jan Kokemüller from comment #2) I think that would work, and is better than what I had in mind. I think we should at least remove the VOP_KQFILTER implementations for fifos across the various filesystems, they're just dead code. I know of at least one out-of-tree filesystem that implements VOP_KQFILTER though, so I'd be inclined to keep the VOP itself.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6b88668f0bfcca09035549e25d0f3c181cd537c8 commit 6b88668f0bfcca09035549e25d0f3c181cd537c8 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2021-10-02 05:17:57 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-10-03 06:02:51 +0000 vfs: remove dead fifoop VOP_KQFILTER implementations These began to become obsolete in d6d64f0f2c26 (r137739) and the deal was later sealed in 003e18aef4c4 (r137801) when vfs.fifofs.fops was dropped and vop-bypass for pipes became mandatory. PR: 225934 Suggested by: markj Reviewe by: kib, markj Differential Revision: https://reviews.freebsd.org/D32270 sys/fs/ext2fs/ext2_vnops.c | 18 ------------------ sys/ufs/ufs/ufs_vnops.c | 19 ------------------- 2 files changed, 37 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=48e109cf31e1e710328f5750fc683daf058ccc19 commit 48e109cf31e1e710328f5750fc683daf058ccc19 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2021-10-02 05:17:57 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-10-09 00:56:46 +0000 vfs: remove dead fifoop VOP_KQFILTER implementations These began to become obsolete in d6d64f0f2c26 (r137739) and the deal was later sealed in 003e18aef4c4 (r137801) when vfs.fifofs.fops was dropped and vop-bypass for pipes became mandatory. PR: 225934 (cherry picked from commit 6b88668f0bfcca09035549e25d0f3c181cd537c8) sys/fs/ext2fs/ext2_vnops.c | 18 ------------------ sys/ufs/ufs/ufs_vnops.c | 19 ------------------- 2 files changed, 37 deletions(-)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=7259ca31048e5ced8e7f90657a3d7084aeafdf51 commit 7259ca31048e5ced8e7f90657a3d7084aeafdf51 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2021-10-02 05:23:03 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-10-12 07:43:07 +0000 fifos: delegate unhandled kqueue filters to underlying filesystem This gives the vfs layer a chance to provide handling for EVFILT_VNODE, for instance. Change pipe_specops to use the default vop_kqfilter to accommodate fifoops that don't specify the method (i.e. all in-tree). Based on a patch by Jan Kokemüller. PR: 225934 Reviewed by: kib, markj (both pre-KASSERT) Differential Revision: https://reviews.freebsd.org/D32271 sys/fs/fifofs/fifo_vnops.c | 1 - sys/kern/sys_pipe.c | 4 +++ sys/kern/vfs_subr.c | 3 ++ tests/sys/kqueue/libkqueue/common.h | 2 ++ tests/sys/kqueue/libkqueue/vnode.c | 59 +++++++++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 1 deletion(-)
^Triage: assignment to a committer of related commits. (In reply to commit-hook from comment #6) For 7259ca31048e5ced8e7f90657a3d7084aeafdf51 (D32271), are merges from current applicable?
^Triage: the 12 branch is now out of support.