Page MenuHomeFreeBSD

sys: purge EOL release compatibility
ClosedPublic

Authored by ehem_freebsd_m5p.com on Jun 23 2022, 4:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 8, 2:55 AM
Unknown Object (File)
Wed, May 8, 2:44 AM
Unknown Object (File)
Wed, May 8, 2:43 AM
Unknown Object (File)
Wed, May 8, 2:43 AM
Unknown Object (File)
Wed, May 8, 2:43 AM
Unknown Object (File)
Wed, May 8, 2:41 AM
Unknown Object (File)
Wed, May 8, 2:40 AM
Unknown Object (File)
Wed, May 8, 12:28 AM

Details

Reviewers
None
Group Reviewers
bhyve
Commits
rG517ccb7c8061: mlx4: purge EOL release compatibility
rG417b350048f5: wmt: purge EOL release compatibility
rGd4a8b3f7cf10: usb: purge EOL release compatibility
rG026babd427e6: mlx4: purge EOL release compatibility
rG33f6a4e90fa1: acpica: purge EOL release compatibility
rG21e0b5079610: qlnxr: cleanup secondary effects from EOL purge
rGda6ae2cf1750: rtwm: purge EOL release compatibility
rG5fa183351459: wmt: purge EOL release compatibility
rG263261e5196c: rtsx: purge EOL release compatibility
rG1c5e68711760: mlx5: purge EOL release compatibility
rGb02a39778952: hyperv: purge EOL release compatibility
rG5d1c658b47e4: hptmv: purge EOL release compatibility
rGbbe35708ad03: qlnx: purge EOL release compatibility
rG097e192a9c20: iavf: purge EOL release compatibility
rG407912909a71: ixl: purge EOL release compatibility
rGbd78d17c333c: qlxgbe: purge EOL release compatibility
rGaf0e1ece373b: qlxge: purge EOL release compatibility
rGecfb9e4e126f: qlxgb: purge EOL release compatibility
rG862dd0d21400: mmc: purge EOL release compatibility
rG29a4a60fffac: ixgbe: purge EOL release compatibility
rG757cb555ae66: e1000: purge EOL release compatibility
rG6147584bc050: mxge: cleanup from purging EOL release compatibility
rGc98b837ad01d: mxge: purge EOL release compatibility
rG721d3e7e9a7d: mrsas: purge EOL release compatibility
rG24c0ba96f1d0: mpt: purge EOL release compatibility
rG336fbb23def7: usb: purge EOL release compatibility
rGaa41036eb7a3: nfsserver: purge EOL release compatibility
rGef2235ec65fd: altq: purge EOL release compatibility
Summary

Remove conditionals checking for End-of-Life releases. Some
pre-FreeBSD-13.0 conditionals remain, but remove everything before
FreeBSD 12.3. For version checks in headers, check for 12.3 and #error
otherwise.

Diff Detail

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

Event Timeline

Several configurations passed build testing, so I'm hopeful I didn't flip any conditionals (I don't know whether I tried everything though). I suspect some additional blank lines are worthy of removal. Possibly comments are worthy of adjustment.

Appears many portions became conditional before FreeBSD 7 and FreeBSD 11. Though that conditional for FreeBSD 3.0 in altq_rmclass.c is impressive.

sys/net/altq/altq_rmclass.c
1451–1453

FreeBSD 3.0, that is just a bit in the past. I think that is the oldest conditional here, but don't quote me on that. Since I just noticed this, I'll update this declaration in an update.

Upon checking appears no test configurations for enabling altq, even LINT. Bit odd...

Then yet another configuration and an unexpected spot shows up.

A few nits.
The ideal way forward is to fix the couple of nits, have me look at them then do a pull request against github (or you can use git's format-patch function) that I can land that has your name as you'd like it in the commit, ideally broken down into one commit per directory to allow for easier MFCing of this piecemeal should the need arise.
And thank you.

sys/dev/netmap/netmap_freebsd.c
441 ↗(On Diff #107273)

I'd delete 'New FreeBSD versions.' here

sys/dev/qlnx/qlnxe/qlnx_os.h
53–56 ↗(On Diff #107273)

I'd just kill these lines.

sys/dev/qlnx/qlnxr/qlnxr_verbs.c
5020 ↗(On Diff #107273)

There looks to be a nesting problem here, no?

sys/dev/qlxgb/qla_os.h
54–57 ↗(On Diff #107273)

I'd just flat out remove these as well.

sys/dev/qlxgbe/ql_os.h
55–58 ↗(On Diff #107273)

Ditto

sys/dev/qlxge/qls_os.h
55–58 ↗(On Diff #107273)

ditto

sys/net/altq/altq_rmclass.c
1451–1453

I'm surprised we had a FreeBSD 3 left in the tree... I killed all < 6 some time ago.

ehem_freebsd_m5p.com marked 5 inline comments as done.

Updating as per comments.

I believe the current nits are adjusted appropriately. Presently on GitLab, branch "D35560". Might be a bit before I'm on GitHub to do such.

The branch "D35560" has been broken apart as appropriate. The commits are roughly ordered by older compatibility bits are earlier, later compatibility bits are later.

sys/dev/qlnx/qlnxr/qlnxr_verbs.c
5020 ↗(On Diff #107273)

Looks like a Phabricator diff issue to me. Mainly it matched lines with same text, but differing indentation. Did 5297->5018, 5309->5019, and 5315->5020. Whereas it should have done 5320->5018, 5321->5019, and 5322->5020.

Looks like moving the blank line made Phabricator choose a better fit.

sys/net/altq/altq_rmclass.c
1451–1453

Looks like sys/net/altq was missed, plus the one in sys/fs. Happens.

This one of course comes complete with K&R declaration which was adjusted.

ehem_freebsd_m5p.com added inline comments.
sys/dev/ixl/ixl_pf_main.c
2269–2279 ↗(On Diff #107339)

Huh, taking a look for the heck of it... I'm guessing "rd64()" is read 64-bits. Should this perhaps instead be an atomic_load_64()? Certainly looks odd. Also noticing this driver is on AMD64 and PPC, so perhaps this should be unconditionally the first option?

I think the vendor-written drivers need to be checked with their respective vendors. It could be that they still want to support EOL-ed versions.

Ok from me for ALTQ.

sys/net/altq/altq_var.h
82

The empty line should go away, too.

ehem_freebsd_m5p.com marked an inline comment as done.

Removing the empty line. The mxge and qlnxr removals caused conditionals to always be true or false. Propogate the conditionals to remove yet more now dead pieces.

I think the vendor-written drivers need to be checked with their respective vendors. It could be that they still want to support EOL-ed versions.

Ick, I could believe it. Though hopefully pre-FreeBSD 11.0 is right out. That is the largest group of compatibility conditionals.

I imagine there are quite a few additional style issues here. I fixed some of the worst ones, but I imagine many more blank lines are worthy of removal or perhaps should be left in.

So rd64() is a wrapper calling bus_space_read_8(). I have a strong suspicion this would be available on PPC64 systems, but I'm not sure. If this is so with recent versions, then always using rd64() is correct.

The pull on GitHub has been created.

vmm and acpi_container are ok with me. I think aside from altq and usb the rest of these are all in vendor drivers for which it would be good to probably try to solicit review from the vendor (though with a timeout to move ahead if there is no response)

In D35560#808976, @jhb wrote:

vmm and acpi_container are ok with me. I think aside from altq and usb the rest of these are all in vendor drivers for which it would be good to probably try to solicit review from the vendor (though with a timeout to move ahead if there is no response)

I'd suggest that if we can't take the driver and build it on release X w/o changes, we should remove the ifdefs, even from the vendor drivers.

Stuff for FreeBSD 10 or 11 at this point likely is insufficient for the drivers to still build there.

Of course, it would be nice to ask, but I wouldn't wait too long if the #ifdef don't at least compile on those older versions.... Looking at qlnx, it seems there's a number of places where API changes were made w/o ifdefs for older systems (not picking on qlnx here, I just picked one at random).

Followup question: Do we have good info on who to contact for each of these drivers?

In D35560#809057, @imp wrote:

I'd suggest that if we can't take the driver and build it on release X w/o changes, we should remove the ifdefs, even from the vendor drivers.

That's a very nice criteria! Totally agreed.

Send me a diff to whack the hwpmc stuff (as in git format-patch) and I'll commit that bit. Commit message needs to start with "hwpmc: "

This revision was not accepted when it landed; it landed in state Needs Review.Feb 4 2023, 4:20 PM
This revision was automatically updated to reflect the committed changes.