Page MenuHomeFreeBSD

NOTES: Update the HZ entry with latest advice.
ClosedPublic

Authored by imp on Jun 17 2021, 4:06 PM.

Details

Summary

Sponsored by: Netflix

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

imp requested review of this revision.Jun 17 2021, 4:06 PM
imp created this revision.
pstef added inline comments.
sys/conf/NOTES
1050

As a non-native speaker I'm undecided how to parse this. Is it "in order to get smooth response, you should not lower HZ" or "lowering HZ will get you smooth response but you shouldn't do it (possibly because of undesirable side effects)"?

I see it is explained later, but maybe this part here could be improved in any case.

allanjude added inline comments.
sys/conf/NOTES
1050

The default inside a VM is 100, not sure if it makes sense to mention that here, but, the default may be lower than 1000

1235

Not your change, but there seems to be a strange character here.

1245

It looks like HZ isn't defined in GENERIC etc anymore. sys/kern/subr_param.c has:

#ifndef HZ
#  if defined(__mips__) || defined(__arm__)
#    define     HZ 100
#  else
#    define     HZ 1000
#  endif
#  ifndef HZ_VM
#    define     HZ_VM 100
#  endif
#else
#  ifndef HZ_VM
#    define     HZ_VM HZ
#  endif
#endif

So NOTES should probably say 1000 here.

gnn added a subscriber: gnn.

LGTM

This revision is now accepted and ready to land.Jun 17 2021, 5:00 PM
imp marked an inline comment as done.Jun 17 2021, 5:04 PM
imp added inline comments.
sys/conf/NOTES
1050

ACK on both these comments. will see if there's some way to make this clearer and more complete.

1235

This is a form feed.
It was used, in by gone eras, to eject paper when people still listed these things on paper.
Later, it was used to separate sections of the file.
There's a bunch of them in this file, and I'm not going to change that just yet.

1245

No. This is wrong two ways.
First, subr_param.c no longer has that.
Second, it's 100 here because the default is 1000.

imp marked an inline comment as done.

Rewrite again to address review suggestions and concerns.

This revision now requires review to proceed.Jun 17 2021, 7:51 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 18 2021, 2:44 PM
This revision was automatically updated to reflect the committed changes.