Page MenuHomeFreeBSD

bhyve: reset event after blockif resize
ClosedPublic

Authored by rew on Dec 7 2021, 8:14 PM.
Tags
None
Referenced Files
F103060297: D33326.id99774.diff
Wed, Nov 20, 10:10 AM
Unknown Object (File)
Oct 3 2024, 11:26 AM
Unknown Object (File)
Oct 3 2024, 11:15 AM
Unknown Object (File)
Oct 3 2024, 9:13 AM
Unknown Object (File)
Sep 24 2024, 3:35 AM
Unknown Object (File)
Sep 23 2024, 6:49 PM
Unknown Object (File)
Sep 17 2024, 4:53 PM
Unknown Object (File)
Sep 16 2024, 11:48 PM

Details

Summary

After a resize event is triggered, reset it; this prevents the resize
(NOTE_ATTRIB) event from being triggered more than once.

Currently, the blockif resize functionality is the only user of
mevent_add_flags() - so this change won't affect other mevent users.

I noticed this when bhyve was consuming a 100% of the cpu after a resize
event was triggered.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rew requested review of this revision.Dec 7 2021, 8:14 PM

I don't know that mevent_add_flags is really the right layer to fix this vs providing some way to explicitly mark an event as EV_CLEAR. It could be that all EVFILT_VNODE should be EV_CLEAR? That is, perhaps have mevent_kq_flags set EV_CLEAR in the flags it returns if me_type == EVF_VNODE. (I looked at the kevent(2) manpage and I do think we would want EV_CLEAR for any EVFILT_VNODE events.)

In D33326#753476, @jhb wrote:

I don't know that mevent_add_flags is really the right layer to fix this

Agreed - blockif_register_resize_callback() could call mevent_add_state(), but then sys/event.h would have to be included in block_if.c

It could be that all EVFILT_VNODE should be EV_CLEAR?

maybe, most uses of EVFILT_VNODE in base would agree with that.

The man page stated that some filters (e.g., EVFILT_SIGNAL and EVFILT_TIMER) automatically set EV_CLEAR, which led me to believe that event filters that do not set EV_CLEAR must have use-cases where the event filter doesn't want to clear the event.

Do you think it would be preferable to set EV_CLEAR in mevent_kq_flags as opposed to using mevent_add_state() directly from blockif_register_resize_callback()?

In D33326#753714, @rew wrote:

Do you think it would be preferable to set EV_CLEAR in mevent_kq_flags as opposed to using mevent_add_state() directly from blockif_register_resize_callback()?

So in theory mevent is a somewhat portable layer that isn't kevent specific. bhyve on illumos doesn't use kevent but is able to just replace mevent.c. Exposing mevent_add_state would beak that I think, so I would prefer changing mevent_kq_fflags and avoiding changing block_if.c in this case (so that the EV_CLEAR is a property of EVF_VNODE that is provided by mevent)

set EV_CLEAR flag when EVFILT_VNODE filter is used

usr.sbin/bhyve/mevent.c
178

I would perhaps make this stateless (not modifying mevp) and instead do something like:

int state;

state = mevp->me_state;
if (mevp->me_type == EVF_VNODE)
    state |= EV_CLEAR;
return (state);

don't modify mevp, just return the translated flag value

use the name retval and format this function to be consistent with the
function above and below it.

Thanks for fixing this!

This revision is now accepted and ready to land.Dec 9 2021, 9:47 PM
This revision was automatically updated to reflect the committed changes.

I have this problem in STABLE-13 with disk image located on NFS share.

This patch fixes the problem.

@rew , are you planning on making an MFC to 13-STABLE?

I have this problem in STABLE-13 with disk image located on NFS share.

This patch fixes the problem.

@rew , are you planning on making an MFC to 13-STABLE?

done, thanks for the reminder