Re: [PATCH 0/8] Make PDX compression optional

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On Tue, Jul 18, 2023 at 11:33:03AM +0200, Jan Beulich wrote:
> On 17.07.2023 18:03, Alejandro Vallejo wrote:
> > Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to
> > disable it because the whole codebase performs unconditional
> > compression/decompression operations on addresses. This has the
> > unfortunate side effect that systems without a need for compression still
> > have to pay the performance impact of juggling bits on every pfn<->pdx
> > conversion (this requires reading several global variables). This series
> > attempts to:
> > 
> >   * Leave the state of pdx and pdx compression documented
> >   * Factor out compression so it _can_ be removed through Kconfig
> >   * Make it so compression is disabled on x86 and enabled on both Aarch32
> >     and Aarch64 by default.
> 
> I disagree with this choice of default for x86. To avoid surprising
> downstreams, this should at best be a two-step process: Keep the
> default as Y right now, and switch to N a couple of releases later.
I'm not particularly worried about the default, as that's easy to change
locally for our customers. That said, my .02 on the matter is that it would
be easier to accept (leaving it as Y) if there was some documented case of
the feature being triggered at present on x86. I'd rather turn it off
unless we have strong evidence that it's actually used. Forcing Xen users
to pay the cost of a feature they don't benefit from just feels wrong.

> 
> But that's only the simple / mechanical side. Considering my earlier
> effort to reduce / remove the involved overhead dynamically at
> runtime (which you may or may not be aware of; see [2]),
I wasn't. I'll have a look at the end of the day[3]. Regardless, most of this
series is about containment of compression into known helpers and hopefully
that should be undeniably positive irrespective of how the overhead
reduction is achieved.

> I view a
> compile time choice as less desirable. At the very least I would
> expect some justification towards this build time choice being
> acceptable / reasonable despite the earlier effort towards greater
> flexibility. Only such would be likely to have me merely defer to
> other x86 maintainers, rather than outright objecting.
> 
> Jan
> 
> [2] https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01616.html

I believe the burden of proof is reversed. Features bring complexity, and
complexity increases the chances of introducing bugs. It's the presence of
a feature that ought to be backed by a proof-of-requirement, not its
absence.

As far as data goes, we're aware of several ARM systems with compressible
memory banks, but no publicly available x86 ones. I'm purposefully leaving
RISC-V and PPC out of this equation, as they may or may not require this,
it's something the maintainers of those ports will have to assess in due
time.

Alejandro

[3] Note that I send this email before reading carefully your 2018 series.