fifos: delegate unhandled kqueue filters to underlying filesystem
ClosedPublic
Actions

Authored by kevans on Oct 2 2021, 6:07 AM.

Details

Reviewers

bdrewery
markj
kib

Commits

rG7259ca31048e: fifos: delegate unhandled kqueue filters to underlying filesystem

Summary

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

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 41916
Build 38804: arc lint + arc unit

Event Timeline

kevans created this revision.Oct 2 2021, 6:07 AM

Herald added a subscriber: imp. · View Herald TranscriptOct 2 2021, 6:07 AM

kevans requested review of this revision.Oct 2 2021, 6:07 AM

kevans added a parent revision: D32270: vfs: remove dead fifoop VOP_KQFILTER implementations.

Harbormaster completed remote builds in B41915: Diff 96127.Oct 2 2021, 6:07 AM

vop_stdkqfilter is the default.

Harbormaster completed remote builds in B41916: Diff 96128.Oct 2 2021, 6:21 AM

kevans added inline comments.Oct 2 2021, 7:09 AM

sys/fs/fifofs/fifo_vnops.c
86	I had considered adding a small fifo_kqfilter that asserts we don't get any EVFILT_READ/EVFILT_WRITE since these should either be rejected by or handled by pipe_kqfilter, but I'm not sure.

kib accepted this revision.Oct 2 2021, 11:19 AM

This revision is now accepted and ready to land.Oct 2 2021, 11:19 AM

I wonder if it would be useful to support the same thing for bound unix sockets.

sys/fs/fifofs/fifo_vnops.c
86	Perhaps instead modify vfs_kqfilter() to assert `vp->v_type != VFIFO \|\| kn->kn_filter == EVFILT_VNODE`. I don't have any particular objection to adding an INVARIANTS-only VOP though.

kevans added inline comments.Oct 3 2021, 5:10 AM

sys/fs/fifofs/fifo_vnops.c
86	I guess that raises another question- should this have been scoped to just passing through `EVFILT_VNODE`? It doesn't seem like there's anything in particular preventing us from passing along a (bogus) `EVFILT_PROCDESC` or `EVFILT_EMPTY`

markj added inline comments.Oct 4 2021, 12:09 AM

sys/fs/fifofs/fifo_vnops.c
86	It seems a bit more maintainable to simply pass unknown filters through to vop_kqfilter(), rather than trying to second-guess the implementation, especially since VOP_KQFILTER could be overridden by a fs that implements custom filters. (OneFS does exactly this, in fact.) The assertion can simply verify that the filter is not EVFILT_READ or _WRITE.