Re: [PATCH 1/1] mm/hugetlb: Make huge_pte_offset() consistent and document behaviour

[Mike Kravetz]

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?

-- 
Mike Kravetz