[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-mips
Subject:    Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
From:       David Daney <ddaney () caviumnetworks ! com>
Date:       2014-07-31 1:13:56
Message-ID: 53D99854.8090109 () caviumnetworks ! com
[Download RAW message or body]

On 07/30/2014 05:48 PM, Huacai Chen wrote:
> Hi, David,
>
> For non-Octeon CPU, htlb_info.huge_pte is equal to K0, but I don't
> know much about Octeon. So I think you know whether we should use K0
>  or htlb_info.huge_pte here, since you are the original author.
>

This is why I requested that somebody show me a disassembly of the
faulty handler.  I cannot tell where the problem is unless I see that.

Really I think the problem is in build_is_huge_pte(), where we are
clobbering 'tmp' which is K0.

So you could reload tmp/K0 in build_is_huge_pte().

> On Thu, Jul 31, 2014 at 5:44 AM, David Daney
> <ddaney@caviumnetworks.com> wrote:
>> On 07/30/2014 02:41 PM, James Hogan wrote:
>>>
>>> Hi Huacai,
>>>
>>> On Tuesday 29 July 2014 14:54:40 Huacai Chen wrote:
>>>>
>>>> In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon
>>>> CPUs) build_r4000_tlb_refill_handler() is modified. But it
>>>> doesn't compatible with the original code in HUGETLB case.
>>>> Because there is a copy & paste error and one line of code is
>>>> missing. It is very easy to produce a bug with LTP's
>>>> hugemmap05 test.
>>>>
>>>> Signed-off-by: Huacai Chen <chenhc@lemote.com> Signed-off-by:
>>>> Binbin Zhou <zhoubb@lemote.com> Cc: <stable@vger.kernel.org>
>>>> --- arch/mips/mm/tlbex.c | 1 + 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c index
>>>> e80e10b..343fe0f 100644 --- a/arch/mips/mm/tlbex.c +++
>>>> b/arch/mips/mm/tlbex.c @@ -1299,6 +1299,7 @@ static void
>>>> build_r4000_tlb_refill_handler(void) } #ifdef
>>>> CONFIG_MIPS_HUGE_TLB_SUPPORT uasm_l_tlb_huge_update(&l, p); +
>>>> UASM_i_LW(&p, K0, 0, K1); build_huge_update_entries(&p,
>>>> htlb_info.huge_pte, K1); build_huge_tlb_write_entry(&p, &l,
>>>> &r, K0, tlb_random, htlb_info.restore_scratch);
>>>
>>>
>>> build_huge_tlb_write_entry only uses K0 as a temp and clobbers
>>> without using the value, so the K0 must be being used by the
>>> code generated by build_huge_update_entires, but the patch you
>>> mentioned changed the second argument from K0 to
>>> htlb_info.huge_pte.
>>>
>>> So should the K0 in the new UASM_i_LW call be changed to
>>> htlb_info.huge_pte too?
>>>
>>
>> I don't know.  You have to dump out the generated handler (by
>> #define DEBUG at the top of the file), then assemble/disassemble
>> it.  Looking at the disassembly, we could make sensible statements
>> about what is happening.
>>
>>
>>
>>> (David Daney on Cc)
>>>
>>> Thanks James
>>>
>>
>>


[prev in list] [next in list] [prev in thread] [next in thread]