Re: [PATCH v2 8/8] pdx: introduce a new compression algorithm based on region offsets

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.06.2025 16:51, Roger Pau Monné wrote:
> On Thu, Jun 26, 2025 at 09:35:04AM +0200, Jan Beulich wrote:
>> On 25.06.2025 18:24, Roger Pau Monné wrote:
>>> On Tue, Jun 24, 2025 at 06:16:15PM +0200, Jan Beulich wrote:
>>>> On 20.06.2025 13:11, Roger Pau Monne wrote:
>>>>> +bool pdx_is_region_compressible(paddr_t base, unsigned long npages)
>>>>> +{
>>>>> +    unsigned long pfn = PFN_DOWN(base);
>>>>> +
>>>>> +    return pdx_to_pfn(pfn_to_pdx(pfn) + npages - 1) == (pfn + npages - 
>>>>> 1);
>>>>
>>>> Aiui for this to be correct, there need to be gaps between the ranges
>>>> covered by individual lookup table slots. In the setup logic you have a
>>>> check commented "Avoid compression if there's no gain", but that doesn't
>>>> look to guarantee gaps everywhere (nor would pfn_offset_sanitize_ranges()
>>>> appear to)?
>>>
>>> But if there are no gaps, the full region is covered correctly, and
>>> hence it's compressible?
>>
>> If there's a guarantee that such ranges would be folded into a single one,
>> all would be fine.
>>
>>> Maybe I'm missing something, could you maybe provide an example that
>>> would exhibit this issue?
>>
>> My understanding is that when there's no gap between regions, and when
>> [base, base + npages) crosses as region boundary, then the expression
>> above will yield true when, because of crossing a region boundary, it
>> ought to be false. Or did I simply misunderstand the purpose of the
>> pdx_is_region_compressible() invocations?
> 
> If there's no gap between the regions it's IMO intended for
> pdx_is_region_compressible() to return true, as the whole region is
> continuous in both the PFN and PDX spaces, and hence compressible
> (even if it spans multiple regions).

My problem is that I can't make the connection between that function
returning true and regions getting concatenated. When the function is
invoked, concatenation (or not) has happened already, aiui.

> But maybe I'm not understanding your point correctly, could you maybe
> provide an example if you disagree with my reply above?  Sorry if I'm
> being dull, with this compression stuff it's sometimes hard for me to
> visualize the case you are trying to make without a concrete
> example.

What I think I didn't take into consideration is that from two pages
being contiguous in MFN space, it ought to follow they're also
contiguous in PDX space. Hence [base, base + npages) crossing a region
boundary (if, contrary to what you say, this was possible in the first
place) would still not be encountering a discontinuity. So overall not
an issue, irrespective of what pdx_is_region_compressible() means
towards (non-)contiguity.

Jan