Mike Kravetz <mike.kravetz@xxxxxxxxxx> writes: > On 07/26/2017 06:34 AM, Punit Agrawal wrote: >> Michal Hocko <mhocko@xxxxxxxxxx> writes: >> >>> On Wed 26-07-17 14:33:57, Michal Hocko wrote: >>>> On Wed 26-07-17 13:11:46, Punit Agrawal wrote: >>> [...] >>>>> I've been running tests from mce-test suite and libhugetlbfs for similar >>>>> changes we did on arm64. There could be assumptions that were not >>>>> exercised but I'm not sure how to check for all the possible usages. >>>>> >>>>> Do you have any other suggestions that can help improve confidence in >>>>> the patch? >>>> >>>> Unfortunatelly I don't. I just know there were many subtle assumptions >>>> all over the place so I am rather careful to not touch the code unless >>>> really necessary. >>>> >>>> That being said, I am not opposing your patch. >>> >>> Let me be more specific. I am not opposing your patch but we should >>> definitely need more reviewers to have a look. I am not seeing any >>> immediate problems with it but I do not see a large improvements either >>> (slightly less nightmare doesn't make me sleep all that well ;)). So I >>> will leave the decisions to others. >> >> I hear you - I'd definitely appreciate more eyes on the code change and >> description. > > I like the change in semantics for the routine. Like you, I examined all > callers of huge_pte_offset() and it appears that they will not be impacted > by your change. > > My only concern is that arch specific versions of huge_pte_offset, may > not (yet) follow the new semantic. Someone could potentially introduce > a new huge_pte_offset call and depend on the new 'documented' semantics. > Yet, an unmodified arch specific version of huge_pte_offset might have > different semantics. I have not reviewed all the arch specific instances > of the routine to know if this is even possible. Just curious if you > examined these, or perhaps you think this is not an issue? >From checking through the implementations of huge_pte_offset() architectures, the change shouldn't break anything. (I also cc'd the posting to linux-arch for architecture maintainers to take more notice). This is because existing users actively deal with the different returned values (NULL, huge pte_t*, swap pte_t*) and are not checking explicitly for pmd or pud. Guarding against future users is more tricky - it would definitely help to align all the implementations.
