On 25.07.2023 16:27, Julien Grall wrote: > 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.
Something like "nr" would be fine with me, for example. > , 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. It's a matter of properly writing it down, I would say. > So I would like to get rid of any use of range in new functions. Hmm, seems to need further input from other maintainers / committers then. Jan
