From: Mike Kravetz <mike.kravetz@oracle.com>
To: Punit Agrawal <punit.agrawal@arm.com>, Michal Hocko <mhocko@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-arch@vger.kernel.org, steve.capper@arm.com,
will.deacon@arm.com, catalin.marinas@arm.com,
kirill.shutemov@linux.intel.com
Subject: Re: [PATCH 1/1] mm/hugetlb: Make huge_pte_offset() consistent and document behaviour
Date: Wed, 26 Jul 2017 20:16:31 -0700 [thread overview]
Message-ID: <9b3b3585-f984-e592-122c-ed23c8558069@oracle.com> (raw)
In-Reply-To: <8760efjp98.fsf@e105922-lin.cambridge.arm.com>
On 07/26/2017 06:34 AM, Punit Agrawal wrote:
> Michal Hocko <mhocko@kernel.org> 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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Punit Agrawal <punit.agrawal@arm.com>, Michal Hocko <mhocko@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-arch@vger.kernel.org, steve.capper@arm.com,
will.deacon@arm.com, catalin.marinas@arm.com,
kirill.shutemov@linux.intel.com
Subject: Re: [PATCH 1/1] mm/hugetlb: Make huge_pte_offset() consistent and document behaviour
Date: Wed, 26 Jul 2017 20:16:31 -0700 [thread overview]
Message-ID: <9b3b3585-f984-e592-122c-ed23c8558069@oracle.com> (raw)
Message-ID: <20170727031631.x6Lpnyx3V54TgbzPoU1TYJU6Fe-lXyCMhXNE5vBelEI@z> (raw)
In-Reply-To: <8760efjp98.fsf@e105922-lin.cambridge.arm.com>
On 07/26/2017 06:34 AM, Punit Agrawal wrote:
> Michal Hocko <mhocko@kernel.org> 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
next prev parent reply other threads:[~2017-07-27 3:16 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-25 15:41 [PATCH 0/1] Clarify huge_pte_offset() semantics Punit Agrawal
2017-07-25 15:41 ` [PATCH 1/1] mm/hugetlb: Make huge_pte_offset() consistent and document behaviour Punit Agrawal
2017-07-25 15:41 ` Punit Agrawal
2017-07-26 8:39 ` Catalin Marinas
2017-07-26 8:50 ` Michal Hocko
2017-07-26 8:53 ` Michal Hocko
2017-07-26 8:53 ` Michal Hocko
2017-07-26 12:11 ` Punit Agrawal
2017-07-26 12:11 ` Punit Agrawal
2017-07-26 12:33 ` Michal Hocko
2017-07-26 12:47 ` Michal Hocko
2017-07-26 12:47 ` Michal Hocko
2017-07-26 13:34 ` Punit Agrawal
2017-07-27 3:16 ` Mike Kravetz [this message]
2017-07-27 3:16 ` Mike Kravetz
2017-07-27 12:58 ` Punit Agrawal
2017-07-27 12:58 ` Punit Agrawal
2017-08-18 14:54 ` [PATCH v2] mm/hugetlb.c: make " Punit Agrawal
2017-08-18 14:54 ` Punit Agrawal
2017-08-18 21:29 ` Mike Kravetz
2017-08-18 21:29 ` Mike Kravetz
2017-08-21 18:07 ` Catalin Marinas
2017-08-21 21:30 ` Mike Kravetz
2017-08-21 21:30 ` Mike Kravetz
2017-08-22 15:32 ` Punit Agrawal
2017-08-22 15:32 ` Punit Agrawal
2017-08-22 10:11 ` Catalin Marinas
2017-08-30 7:49 ` Michal Hocko
2017-08-30 7:49 ` Michal Hocko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9b3b3585-f984-e592-122c-ed23c8558069@oracle.com \
--to=mike.kravetz@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=catalin.marinas@arm.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=n-horiguchi@ah.jp.nec.com \
--cc=punit.agrawal@arm.com \
--cc=steve.capper@arm.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).