Page MenuHomeFreeBSD

libsysdecode: Drop dead __amd64__ && COMPAT_32BIT code
ClosedPublic

Authored by jrtc27 on Jul 7 2023, 11:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jun 3 2024, 7:26 AM
Unknown Object (File)
May 10 2024, 12:20 AM
Unknown Object (File)
Apr 26 2024, 9:18 AM
Unknown Object (File)
Apr 26 2024, 9:18 AM
Unknown Object (File)
Apr 26 2024, 9:18 AM
Unknown Object (File)
Apr 26 2024, 2:16 AM
Unknown Object (File)
Apr 21 2024, 10:25 PM
Unknown Object (File)
Apr 21 2024, 9:50 PM
Subscribers

Details

Summary

Under COMPAT_32BIT we are compiling 32-bit code and so amd64 is not
defined, i386 is, and we use the real i386 headers.

Diff Detail

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

Event Timeline

This revision is now accepted and ready to land.Jul 8 2023, 12:12 AM

hmm, so when the i386 kernel bits will be retired?

hmm, so when the i386 kernel bits will be retired?

I don't understand the question

hmm, so when the i386 kernel bits will be retired?

I don't understand the question

#include <i386/linux/linux.h> is the kernel bits, so my question is what would you suggests when the i386 kernel will be removed.
Also, I do not plan to support i386 Linuxulator due to ^^

In my POV 32-bit libsysdecode on 64-bit platforms should use linux32 headers to be built

hmm, so when the i386 kernel bits will be retired?

I don't understand the question

#include <i386/linux/linux.h> is the kernel bits, so my question is what would you suggests when the i386 kernel will be removed.
Also, I do not plan to support i386 Linuxulator due to ^^

In my POV 32-bit libsysdecode on 64-bit platforms should use linux32 headers to be built

Either all i386 headers will stay around, because lib32 needs them, or they will all be reorganised. This is not a libsysdecode-specific concern; it will just follow whatever happens for all the other i386 headers you need to build lib32 (and applications on top of that).

hmm, so when the i386 kernel bits will be retired?

I don't understand the question

#include <i386/linux/linux.h> is the kernel bits, so my question is what would you suggests when the i386 kernel will be removed.
Also, I do not plan to support i386 Linuxulator due to ^^

In my POV 32-bit libsysdecode on 64-bit platforms should use linux32 headers to be built

And no, they should not. We do not use the various freebsd32 files to build lib32, so we should not use the various linux32 files to build lib32. We always want the real i386 definitions of things so we match the real i386 build as closely as possible. Your belief here goes against the whole approach of lib32 across the entire tree.

hmm, so when the i386 kernel bits will be retired?

I don't understand the question

#include <i386/linux/linux.h> is the kernel bits, so my question is what would you suggests when the i386 kernel will be removed.
Also, I do not plan to support i386 Linuxulator due to ^^

In my POV 32-bit libsysdecode on 64-bit platforms should use linux32 headers to be built

And no, they should not. We do not use the various freebsd32 files to build lib32, so we should not use the various linux32 files to build lib32. We always want the real i386 definitions of things so we match the real i386 build as closely as possible. Your belief here goes against the whole approach of lib32 across the entire tree.

So, someone broke libsysdecode and you want to fix it by a wrong way. I.e., WE, who really do the JOB, should maintain i386 Linuxulator only to build libsysdecode for 32-bit kdump on amd64. Which is intended to decode traces from amd64/linux32, strange at least

hmm, so when the i386 kernel bits will be retired?

I don't understand the question

#include <i386/linux/linux.h> is the kernel bits, so my question is what would you suggests when the i386 kernel will be removed.
Also, I do not plan to support i386 Linuxulator due to ^^

In my POV 32-bit libsysdecode on 64-bit platforms should use linux32 headers to be built

And no, they should not. We do not use the various freebsd32 files to build lib32, so we should not use the various linux32 files to build lib32. We always want the real i386 definitions of things so we match the real i386 build as closely as possible. Your belief here goes against the whole approach of lib32 across the entire tree.

So, someone broke libsysdecode and you want to fix it by a wrong way. I.e., WE, who really do the JOB, should maintain i386 Linuxulator only to build libsysdecode for 32-bit kdump on amd64. Which is intended to decode traces from amd64/linux32, strange at least

No, this code was always wrong, always dead, never did what you thought it did when you committed b69ae1a34c6f918118693490f18a81ecd7163f83. Nobody broke anything. It has never been the case that __amd64__ and COMPAT_32BIT are simultaneously defined, because that is a contradiction. I am deleting the dead code that achieves no point, and leaving libsysdecode to follow the existing approach for the rest of the tree in dealing with lib32.

If and when the i386 kernel goes away there can be a conversation about how to maintain lib32 libsysdecode linuxulator support. But that day is not today, and that conversation is part of a much bigger one about how to deal with i386 everything in lib32.

Please keep your attitude in check, and at least get your facts right if you're going to accuse people of breaking your code and fixing things the "wrong" way.

Let me reiterate: this is a completely non-functional change. The code I am deleting is 100% dead. You can prove it for yourself by putting a #error there and doing an amd64 buildworld.

Perhaps worth noting explicitly that during the COMPAT_32BIT build, __amd64__ is not defined, only __i386__, so this commit is a NOP.

In D40916#931786, @jhb wrote:

Perhaps worth noting explicitly that during the COMPAT_32BIT build, __amd64__ is not defined, only __i386__, so this commit is a NOP.

so, it should be like:
#ifdef aarch64
#include <arm64/linux/linux.h>
#elif i386
#ifdef COMPAT_32BIT
#include <amd64/linux32/linux.h>
#else
#include <i386/linux/linux.h>
#endif
#elif amd64
#include <amd64/linux/linux.h>
#else
#error "Unsupported Linux arch"
#endif

?

In D40916#931786, @jhb wrote:

Perhaps worth noting explicitly that during the COMPAT_32BIT build, __amd64__ is not defined, only __i386__, so this commit is a NOP.

so, it should be like:
#ifdef aarch64
#include <arm64/linux/linux.h>
#elif i386
#ifdef COMPAT_32BIT
#include <amd64/linux32/linux.h>
#else
#include <i386/linux/linux.h>
#endif
#elif amd64
#include <amd64/linux/linux.h>
#else
#error "Unsupported Linux arch"
#endif

?

That code is not dead, but it is not how we do this anywhere in the tree today. So that is not the right code to commit. Maybe in future if i386 dies, though likely i386/linux/linux.h would instead just become a forwarding header in that scenario.

In D40916#931786, @jhb wrote:

Perhaps worth noting explicitly that during the COMPAT_32BIT build, __amd64__ is not defined, only __i386__, so this commit is a NOP.

so, it should be like:
#ifdef aarch64
#include <arm64/linux/linux.h>
#elif i386
#ifdef COMPAT_32BIT
#include <amd64/linux32/linux.h>
#else
#include <i386/linux/linux.h>
#endif
#elif amd64
#include <amd64/linux/linux.h>
#else
#error "Unsupported Linux arch"
#endif

?

To do what you wanted to achieve, yes. However, that seems quite odd to me. We don't do that kind of duplication now for freebsd32 where we use the corresponding 32-bit headers directly instead. That is, we don't have sys/amd64/i386/foo.h, instead we use sys/i386/foo.h directly. Arguably linux32 on amd64 should be able to reuse headers (and ideally code) from sys/i386/linux instead. Probably that wasn't done initially for simplicity, but it would probably be less work going forward to have a single copy of those instead.

In D40916#931819, @jhb wrote:
In D40916#931786, @jhb wrote:

Perhaps worth noting explicitly that during the COMPAT_32BIT build, __amd64__ is not defined, only __i386__, so this commit is a NOP.

so, it should be like:
#ifdef aarch64
#include <arm64/linux/linux.h>
#elif i386
#ifdef COMPAT_32BIT
#include <amd64/linux32/linux.h>
#else
#include <i386/linux/linux.h>
#endif
#elif amd64
#include <amd64/linux/linux.h>
#else
#error "Unsupported Linux arch"
#endif

?

To do what you wanted to achieve, yes. However, that seems quite odd to me. We don't do that kind of duplication now for freebsd32 where we use the corresponding 32-bit headers directly instead. That is, we don't have sys/amd64/i386/foo.h, instead we use sys/i386/foo.h directly. Arguably linux32 on amd64 should be able to reuse headers (and ideally code) from sys/i386/linux instead. Probably that wasn't done initially for simplicity, but it would probably be less work going forward to have a single copy of those instead.

It's hard to find motivation to do this, i386 is dead. I do not object about the change at all, I hope Jess will finish that you suggests