Page MenuHomeFreeBSD

sys/param: increase MAXCOMLEN to 23 for 64-bit alignment
AbandonedPublic

Authored by ehem_freebsd_m5p.com on Jan 19 2023, 4:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Sep 18, 8:33 AM
Unknown Object (File)
Aug 7 2024, 6:16 PM
Unknown Object (File)
Aug 7 2024, 11:22 AM
Unknown Object (File)
Aug 5 2024, 9:06 PM
Unknown Object (File)
Jul 29 2024, 3:06 AM
Unknown Object (File)
Jul 10 2024, 8:57 AM
Unknown Object (File)
Jun 24 2024, 11:25 AM
Unknown Object (File)
Apr 11 2024, 3:12 PM
Subscribers

Details

Reviewers
markj
jhb
Summary

A score of years ago MAXCOMLEN was increased to 19 to take advantage of
most values being 32-bit aligned. Since c37c2d03b87 64-bit machines
have become common, take advantage of the larger alignment this causes.
Also allows longer strings which increasing machine size have made
common.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 49167
Build 46056: arc lint + arc unit

Event Timeline

Rather a long time since rG:c37c2d03b87 when it was increased to 19. Now 23 makes more sense. I'm tempted to suggest 31 since machines are so large now.

Appears struct kinfo_proc.ki_moretdname is defined in terms of MAXCOMLEN. This in turn triggers the KASSERT() on line 2477 of sys/kern/imgact_elf.c. Guessing this means __FreeBSD_version would need to be bumped, but also need to look into the KASSERT().

Question is whether any of the things tied to MAXCOMLEN should be separated. All the interrupt names are tied to MAXCOMLEN, but should they be?

MAXCOMLEN is used in userspace and as is this change breaks some ABIs like the layout of struct kinfo_proc. The change only adds four bytes to the size. What real benefits does it bring? That is, what are some examples of structures where the use of MAXCOMLEN with 64-bit alignment is creating holes?

Appears struct kinfo_proc.ki_moretdname is defined in terms of MAXCOMLEN. This in turn triggers the KASSERT() on line 2477 of sys/kern/imgact_elf.c. Guessing this means __FreeBSD_version would need to be bumped, but also need to look into the KASSERT().

The static assertion is there because the layout of struct kinfo_proc has changed as a result of your diff. At the very least, the size of ki_sparestrings[] needs to be reduced to compensate, but more analysis needs to be done to give confidence that changing this constant won't break anything. This doesn't seem worth the stated benefits of the change.

Question is whether any of the things tied to MAXCOMLEN should be separated. All the interrupt names are tied to MAXCOMLEN, but should they be?

MAXCOMLEN-sized buffers are usually used to store the names of commands, so I suspect there's no real reason to use it for interrupt names, it just happens to be convenient.

Changing this breaks a lot of things: many, many things. We store kinfo_proc in core dump notes for example, but it's also in places like ptrace_lwpinfo (which is both in the syscall ABI and also in core dump notes, and so external tools like procstat and GDB hardcode the layout of the structures in those notes). If you want a larger name field, you will have to instead add a new field in various structures with the longer length, or possibly have the new field hold the additional characters only. (I think we already do a split thread name field in at least one place as a similar trick now.)

The reason interrupt names are this length is because they are also used as the name of the associated interrupt threads and hit the MAXCOMLEN limit due to that.