Hi,
On 25/07/2023 07:51, Jan Beulich wrote:
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?
I was referring to the number of pages. I don't think it make sense to
have it in bytes given the start is a frame.
Especially in the former
case calculations at the call sites would be quite a bit more cumbersome.
I could agree with (mfn,nr) tuples
Ok. So your objection of my proposal is just about the name, right? If
so, I didn't put too much thought behind the naming when I sent my
original e-mail. I am open to any.
, 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).
The problem with range is that it can lead to confusion on whether the
end is inclusive or exclusive. We had one bug recently in the Arm PCI
code because of that.
So I would like to get rid of any use of range in new functions.
Cheers,
--
Julien Grall
