From: David Daney <ddaney@caviumnetworks.com>
To: James Hogan <james@albanarts.com>
Cc: <linux-mips@linux-mips.org>, Huacai Chen <chenhc@lemote.com>,
Ralf Baechle <ralf@linux-mips.org>,
John Crispin <john@phrozen.org>,
"Steven J. Hill" <Steven.Hill@imgtec.com>,
Fuxin Zhang <zhangfx@lemote.com>,
Zhangjin Wu <wuzhangjin@gmail.com>,
Binbin Zhou <zhoubb@lemote.com>, <stable@vger.kernel.org>
Subject: Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
Date: Wed, 30 Jul 2014 14:44:46 -0700 [thread overview]
Message-ID: <53D9674E.4000507@caviumnetworks.com> (raw)
In-Reply-To: <2357839.vPXx615ci5@radagast>
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
>
WARNING: multiple messages have this Message-ID (diff)
From: David Daney <ddaney@caviumnetworks.com>
To: James Hogan <james@albanarts.com>
Cc: linux-mips@linux-mips.org, Huacai Chen <chenhc@lemote.com>,
Ralf Baechle <ralf@linux-mips.org>,
John Crispin <john@phrozen.org>,
"Steven J. Hill" <Steven.Hill@imgtec.com>,
Fuxin Zhang <zhangfx@lemote.com>,
Zhangjin Wu <wuzhangjin@gmail.com>,
Binbin Zhou <zhoubb@lemote.com>,
stable@vger.kernel.org
Subject: Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
Date: Wed, 30 Jul 2014 14:44:46 -0700 [thread overview]
Message-ID: <53D9674E.4000507@caviumnetworks.com> (raw)
Message-ID: <20140730214446.1reXIk0cdJuxfNn7tWDqYQfOn5FRlgwdOnhcrNNP9f4@z> (raw)
In-Reply-To: <2357839.vPXx615ci5@radagast>
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
>
next prev parent reply other threads:[~2014-07-30 21:45 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-29 6:54 [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB Huacai Chen
2014-07-29 6:54 ` Huacai Chen
2014-07-30 16:37 ` Aurelien Jarno
2014-07-30 21:41 ` James Hogan
2014-07-30 21:44 ` David Daney [this message]
2014-07-30 21:44 ` David Daney
2014-07-31 0:48 ` Huacai Chen
2014-07-31 1:13 ` David Daney
2014-07-31 11:54 ` James Hogan
2014-07-31 17:33 ` David Daney
2014-08-02 21:35 ` Aurelien Jarno
2014-08-04 10:08 ` James Hogan
2014-08-04 13:05 ` Aurelien Jarno
2014-08-04 13:10 ` James Hogan
2014-08-05 7:09 ` Huacai Chen
2014-08-06 16:27 ` Ralf Baechle
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=53D9674E.4000507@caviumnetworks.com \
--to=ddaney@caviumnetworks.com \
--cc=Steven.Hill@imgtec.com \
--cc=chenhc@lemote.com \
--cc=james@albanarts.com \
--cc=john@phrozen.org \
--cc=linux-mips@linux-mips.org \
--cc=ralf@linux-mips.org \
--cc=stable@vger.kernel.org \
--cc=wuzhangjin@gmail.com \
--cc=zhangfx@lemote.com \
--cc=zhoubb@lemote.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.