geom: add kqfilter support for geom dev
ClosedPublic
Actions

Authored by rew on Dec 13 2021, 2:18 AM.

Details

Reviewers

kib
jhb

Commits

rGa01bf3940271: geom: add kqfilter support for geom dev
rGa50e92cc203c: geom: add kqfilter support for geom dev

Summary

Implement support for the EVFILT_VNODE filter to trigger the NOTE_ATTRIB
event.

This will allow bhyve to notify a guest when an underlying zvol is
resized from the host. This filter will catch events for zvols that are
being managed by geom (i.e., volmode=full|geom).

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rew created this revision.Dec 13 2021, 2:18 AM

Herald added a subscriber: imp. · View Herald TranscriptDec 13 2021, 2:18 AM

rew requested review of this revision.Dec 13 2021, 2:18 AM

Harbormaster completed remote builds in B43303: Diff 99905.Dec 13 2021, 2:18 AM

rew mentioned this in D33403: bhyve: get mediasize for character devices when resizing virtio-blk.Dec 13 2021, 2:44 AM

rew added a child revision: D33403: bhyve: get mediasize for character devices when resizing virtio-blk.Dec 13 2021, 2:44 AM

jhb added a subscriber: jhb.Dec 13 2021, 8:10 PM

jhb added inline comments.

sys/geom/geom_dev.c
946	Similar to the zvol request, perhaps you want to reject requests with any flags other than `NOTE_ATTRIB`?

don't add the knote to knlist if the event is not supported.

Harbormaster completed remote builds in B43527: Diff 100425.Dec 21 2021, 6:05 PM

rew added reviewers: kib, jhb.Dec 21 2021, 6:38 PM

I think NOTE_REVOKE is also quite useful for geom devs.

kib added inline comments.Dec 22 2021, 7:20 AM

sys/geom/geom_dev.c
327	I suspect this should be NOTE_EXTEND and not NOTE_ATTRIB?

In D33402#759444, @kib wrote:

I think NOTE_REVOKE is also quite useful for geom devs.

Noted, I will follow-up with separate reviews that implement other NOTE_* events, including NOTE_REVOKE.

I noticed geom access counters get leaked when a geom device is revoked - and then I stumbled on commit https://reviews.freebsd.org/rS286237. When a geom device is being revoked, can the access counters be zero'ed out so they don't get leaked when revoked?

sys/geom/geom_dev.c
327	I used NOTE_ATTRIB for consistency because that is the event that is triggered when the size of regular file changes. In my mind, NOTE_EXTEND was to communicate something being extended (size increase) as opposed to shrinking (size decrease). Maybe I misunderstand that. If it's acceptable to use NOTE_EXTEND when the size of a geom is extended/shrunk, then I'm good with using NOTE_EXTEND - let me know what you think.

In D33402#763772, @rew wrote:

I noticed geom access counters get leaked when a geom device is revoked - and then I stumbled on commit https://reviews.freebsd.org/rS286237. When a geom device is being revoked, can the access counters be zero'ed out so they don't get leaked when revoked?

I really do not know. An answer to this question can be obtained only by the experiment, try to do that and see what else get broken. From very high point of view, I doubt that it is possible, because there are other ways to keep a handle on geom providers than through devfs node. So despite devfs node is reclaimed, you might still have reference to provider and have associated rights (g_vfs_open() is about this, in fact).

sys/geom/geom_dev.c
327	Look for instance at ftruncate(2) that both extends and shrinks.

rew added inline comments.Jan 9 2022, 4:10 AM

sys/geom/geom_dev.c
327	That'll trigger the NOTE_ATTRIB event when extending or shrinking. For a regular file, NOTE_EXTEND will be triggered after a VOP_WRITE() if the size is larger than before.

kib added inline comments.Jan 9 2022, 5:47 AM

sys/geom/geom_dev.c
327	I must admit that this is quite weird and not matches the actual machinery behind the implementation. For instance, on UFS the last byte of file must be always backed by the block, i.e. hole can exist only in between start and EOF, so UFS does allocate and zero-out extending block (but not through VOP_WRITE()).

kib accepted this revision.Jan 9 2022, 5:58 AM

This revision is now accepted and ready to land.Jan 9 2022, 5:58 AM

jhb added inline comments.Jan 11 2022, 8:03 PM

sys/geom/geom_dev.c
327	It's kind of complicated. It should perhaps be NOTE_EXTEND if it grows (but not shrinks!), but NOTE_ATTRIB always? The problem is that EVFILT_VNODE is a bit "filesystem like UFS"-specific. That is, a VOP_WRITE() that grows a file raises NOTE_EXTEND, but using truncate(2) or ftruncate(2) to resize a file uses VOP_SETATTR which raises NOTE_ATTRIB. Right now bhyve only lists for NOTE_ATTRIB, though I've suggested to Rob that perhaps it wants to listen to both NOTE_ATTRIB and NOTE_EXTEND.