Re: [PATCH 6/8] mm/pdx: Standardize region validation wrt pdx compression

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.07.2023 20:20, Julien Grall wrote:
> On 24/07/2023 13:18, Alejandro Vallejo wrote:
>> On Fri, Jul 21, 2023 at 06:05:51PM +0100, Julien Grall wrote:
>>> Hi Alejandro,
>>>
>>> On 17/07/2023 17:03, Alejandro Vallejo wrote:
>>>> +bool pdx_is_region_compressible(unsigned long smfn, unsigned long emfn)
>>>
>>> For newer interface, I would rather prefer if we use start + size. It is
>>> easier to reason (you don't have to wonder whether 'emfn' is inclusive or
>>> not) and avoid issue in the case you are trying to handle a region right at
>>> the end of the address space as emfn would be 0 in the non-inclusive case
>>> (not much a concern for MFNs as the last one should be invalid, but it makes
>>> harder to reason).
>> I could agree on this, but every single caller is based on (smfn, emfn),
>> so it needlessly forces every caller to perform conversions where the
>> callee can do it just once.
> 
> That's indeed one way to see it. The problem is that...
> 
>> That said, I think your point makes sense and
>> it ought to be done. Probably as as part of a bigger refactor where
>> (smfn, emfn)-based functions are turned into (base, len) variants.
> 
> ... clean-up tends to be put in the back-burner and we just continue to 
> add new use. This makes the task to remove every use a lot more 
> difficult. So there is a point when one has to say no more.
> 
> Therefore, I would strongly prefer if each callers are doing the 
> computation. The rest can be removed leisurely.
> 
> Let see what the opinion of the other maintainers.

I think [a,b] ranges are fine to pass, and may even be preferable over
passing a size. I'm specifically using that term that you also used:
"size" (or "length") is ambiguous when talking about page granular
items - is it in bytes or number of pages? Especially in the former
case calculations at the call sites would be quite a bit more cumbersome.
I could agree with (mfn,nr) tuples, but as said I think inclusive
ranges are also fine to use (and would be less of a problem at the call
sites here, afaics).

Jan