Re: [PATCH for-4.18 v5] xen/pdx: Make CONFIG_PDX_COMPRESSION a common Kconfig option

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.10.2023 17:27, Roger Pau Monné wrote:
> On Fri, Oct 06, 2023 at 04:09:19PM +0100, Julien Grall wrote:
>> On 06/10/2023 15:44, Andrew Cooper wrote:
>>> From: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
>>>
>>> Adds a new compile-time flag to allow disabling PDX compression and
>>> compiles out compression-related code/data. It also shorts the pdx<->pfn
>>> conversion macros and creates stubs for masking functions.
>>>
>>> While at it, removes the old arch-defined CONFIG_HAS_PDX flag.  Despite the
>>> illusion of choice, it was not optional.
>>>
>>> There are ARM and PPC platforms with sparse RAM banks - leave compression
>>> active by default there.  OTOH, there are no known production x86 systems 
>>> with
>>> sparse RAM banks, so disable compression.  This decision can be revisited if
>>> such a platform comes along.
>>
>> (Process remarks rather than the code itself)
>>
>> Jan is away this week so I want to make sure this doesn't go in without him
>> having a say.
>>
>> While I don't particularly care about the approach taken for x86, Jan voiced
>> concerned with this approach and so far I didn't see any conclusion. If
>> there is any, then please point me to them.
>>
>> For the record, the objections from Jan are in [1]. If we want to ignore
>> them, then I think we need a vote. Possibly only from the x86 folks (?).
> 
> I would be fine in leaving the option to be selected if we knew that
> such x86 systems might be feasible, but so far we have seen 0 x86
> systems with sparse RAM.  That said, I don't have a strong opinion, but
> the hiding on x86 seems fine to me.  Interested parties can always
> forcefully select the option, and a case can be made to make it
> available again on Kconfig.

I find it odd to demand people to change source code for aspects like
this. The very least I'd expect is that BIGMEM configurations (which
I've never seen any production use of) can actually also engage PDX.

Jan