On Feb 17, 2012, at 3:02 PM, Julio Merino wrote: > On 2/17/12 5:58 PM, Matt Thomas wrote: >> >> On Feb 17, 2012, at 2:54 PM, Julio Merino wrote: >> >>> On 2/17/12 5:45 PM, Matt Thomas wrote: >>>> >>>> On Feb 17, 2012, at 2:43 PM, Julio Merino wrote: >>>>> So the modules are broken on purpose? >>>> >>>> Yes. >>> >>> Interesting. If that's the case, shouldn't we break PAGE_SIZE for all >>> platforms and keep things consistent? >> >> For those with variable page sizes (like powerpc or mips), yes. > > I was asking about *all* platforms regardless of whether they have static or > variable page sizes. Keeping this inconsistent seems like a very easy way of > writing non-portable code...
Again they should use PAGE_SIZE which can be constant or not. No reason for a non-constant PAGE_SIZE on alpha or vax which has fixed sized pages. >>> The modules that are broken (see the referenced PR for a list) fail due to >>> a missing uvmexp_pagesize symbol (*not* PAGE_SIZE itself), which I assumed >>> was there to prevent depending on the PAGE_SIZE compile-time constant. I >>> understand this information not being statically-available because of >>> variable-page sizes in these platforms. >> >> properly should use uvmexp.pagesize instead. > > Aha, I see. Does it make sense to keep the test after renaming PAGE_SIZE to > uvmexp.pagesize? It's a public symbol after all and we really should have > tests for these, I think. > > Do we need to go over the broken modules one by one and replace PAGE_SIZE > with uvmexp.pagesize? (I'm expecting this won't be as easy as it sounds due > to preprocessor conditionals et. al.) They should still use PAGE_SIZE which should evaluate to uvmexp.pagesize