All of lore.kernel.org
 help / color / mirror / Atom feed
From: thunder.leizhen@huawei.com (Leizhen (ThunderTown))
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/1] arm64/hugetlb: clear PG_dcache_clean if the page is dirty when munmap
Date: Thu, 25 Aug 2016 19:27:47 +0800	[thread overview]
Message-ID: <57BED633.1060802@huawei.com> (raw)
In-Reply-To: <20160825093054.GB20748@e104818-lin.cambridge.arm.com>



On 2016/8/25 17:30, Catalin Marinas wrote:
> On Thu, Aug 25, 2016 at 09:42:26AM +0800, Leizhen (ThunderTown) wrote:
>> On 2016/8/24 18:30, Catalin Marinas wrote:
>>>>>>>>>>>> On 2016/7/8 21:54, Catalin Marinas wrote:
>>>>>>>>>>>>> ------------8<----------------
>>>>>>>>>>>>> diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c
>>>>>>>>>>>>> index dbd12ea8ce68..c753fa804165 100644
>>>>>>>>>>>>> --- a/arch/arm64/mm/flush.c
>>>>>>>>>>>>> +++ b/arch/arm64/mm/flush.c
>>>>>>>>>>>>> @@ -75,7 +75,8 @@ void __sync_icache_dcache(pte_t pte, unsigned long addr)
>>>>>>>>>>>>>  	if (!page_mapping(page))
>>>>>>>>>>>>>  		return;
>>>>>>>>>>>>>  
>>>>>>>>>>>>> -	if (!test_and_set_bit(PG_dcache_clean, &page->flags))
>>>>>>>>>>>>> +	if (!test_and_set_bit(PG_dcache_clean, &page->flags) ||
>>>>>>>>>>>>> +	    PageDirty(page))
>>>>>>>>>>>>>  		sync_icache_aliases(page_address(page),
>>>>>>>>>>>>>  				    PAGE_SIZE << compound_order(page));
>>>>>>>>>>>>>  	else if (icache_is_aivivt())
>>>>>>>>>>>>> ----------------8<---------------------
> [...]
>>> While we indeed see failures on multiple filesystem types, I wonder
>>> whether this test case is actually expected to work. If I modify the
>>> test to pass O_TRUNC to open(), I can no longer see failures. So any
>>> standard tool that copies/creates executable files (gcc, dpkg, cp, rsync
>>> etc.) wouldn't encounter such issues since they truncate the original
>>> file and old page cache pages would be removed.
>>>
>>> Do you have a real use-case where a task mmap's an executable file,
>>> modifies it in place and expects another task to see the new
>>> instructions without user-space cache maintenance?
>>
>> No, it's just a test case created by testers.
> 
> In this case I propose we ignore this patch and you adjust the test to
> use O_TRUNC, at least until we find a real scenario where this would
> matter.
OK, thanks. We currently add __clear_cache in user space.

> 

WARNING: multiple messages have this Message-ID (diff)
From: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steve Capper <Steve.Capper@arm.com>,
	David Woods <dwoods@ezchip.com>,
	Tianhong Ding <dingtianhong@huawei.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Xinwei Hu <huxinwei@huawei.com>, Zefan Li <lizefan@huawei.com>,
	"fangwei (I)" <fangwei1@huawei.com>,
	"Hanjun Guo" <guohanjun@huawei.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	wangxuefeng 00195527 <wxf.wang@hisilicon.com>
Subject: Re: [PATCH 1/1] arm64/hugetlb: clear PG_dcache_clean if the page is dirty when munmap
Date: Thu, 25 Aug 2016 19:27:47 +0800	[thread overview]
Message-ID: <57BED633.1060802@huawei.com> (raw)
In-Reply-To: <20160825093054.GB20748@e104818-lin.cambridge.arm.com>



On 2016/8/25 17:30, Catalin Marinas wrote:
> On Thu, Aug 25, 2016 at 09:42:26AM +0800, Leizhen (ThunderTown) wrote:
>> On 2016/8/24 18:30, Catalin Marinas wrote:
>>>>>>>>>>>> On 2016/7/8 21:54, Catalin Marinas wrote:
>>>>>>>>>>>>> ------------8<----------------
>>>>>>>>>>>>> diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c
>>>>>>>>>>>>> index dbd12ea8ce68..c753fa804165 100644
>>>>>>>>>>>>> --- a/arch/arm64/mm/flush.c
>>>>>>>>>>>>> +++ b/arch/arm64/mm/flush.c
>>>>>>>>>>>>> @@ -75,7 +75,8 @@ void __sync_icache_dcache(pte_t pte, unsigned long addr)
>>>>>>>>>>>>>  	if (!page_mapping(page))
>>>>>>>>>>>>>  		return;
>>>>>>>>>>>>>  
>>>>>>>>>>>>> -	if (!test_and_set_bit(PG_dcache_clean, &page->flags))
>>>>>>>>>>>>> +	if (!test_and_set_bit(PG_dcache_clean, &page->flags) ||
>>>>>>>>>>>>> +	    PageDirty(page))
>>>>>>>>>>>>>  		sync_icache_aliases(page_address(page),
>>>>>>>>>>>>>  				    PAGE_SIZE << compound_order(page));
>>>>>>>>>>>>>  	else if (icache_is_aivivt())
>>>>>>>>>>>>> ----------------8<---------------------
> [...]
>>> While we indeed see failures on multiple filesystem types, I wonder
>>> whether this test case is actually expected to work. If I modify the
>>> test to pass O_TRUNC to open(), I can no longer see failures. So any
>>> standard tool that copies/creates executable files (gcc, dpkg, cp, rsync
>>> etc.) wouldn't encounter such issues since they truncate the original
>>> file and old page cache pages would be removed.
>>>
>>> Do you have a real use-case where a task mmap's an executable file,
>>> modifies it in place and expects another task to see the new
>>> instructions without user-space cache maintenance?
>>
>> No, it's just a test case created by testers.
> 
> In this case I propose we ignore this patch and you adjust the test to
> use O_TRUNC, at least until we find a real scenario where this would
> matter.
OK, thanks. We currently add __clear_cache in user space.

> 

  reply	other threads:[~2016-08-25 11:27 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-07 12:09 [PATCH 1/1] arm64/hugetlb: clear PG_dcache_clean if the page is dirty when munmap Zhen Lei
2016-07-07 12:09 ` Zhen Lei
2016-07-07 15:37 ` Catalin Marinas
2016-07-07 15:37   ` Catalin Marinas
2016-07-08  3:36   ` Leizhen (ThunderTown)
2016-07-08  3:36     ` Leizhen (ThunderTown)
2016-07-08 13:54     ` Catalin Marinas
2016-07-08 13:54       ` Catalin Marinas
2016-07-08 15:24       ` Leizhen (ThunderTown)
2016-07-08 15:24         ` Leizhen (ThunderTown)
2016-07-08 16:13         ` Catalin Marinas
2016-07-08 16:13           ` Catalin Marinas
2016-07-11 12:43           ` Leizhen (ThunderTown)
2016-07-11 12:43             ` Leizhen (ThunderTown)
2016-07-12 15:35             ` Catalin Marinas
2016-07-12 15:35               ` Catalin Marinas
2016-07-20  2:46               ` Leizhen (ThunderTown)
2016-07-20  2:46                 ` Leizhen (ThunderTown)
2016-07-20  9:19                 ` Catalin Marinas
2016-07-20  9:19                   ` Catalin Marinas
2016-08-22  4:19                   ` Leizhen (ThunderTown)
2016-08-22  4:19                     ` Leizhen (ThunderTown)
2016-08-23 17:28                     ` Catalin Marinas
2016-08-23 17:28                       ` Catalin Marinas
2016-08-24  1:30                       ` Leizhen (ThunderTown)
2016-08-24  1:30                         ` Leizhen (ThunderTown)
2016-08-24  9:00                       ` Leizhen (ThunderTown)
2016-08-24  9:00                         ` Leizhen (ThunderTown)
2016-08-24 10:30                         ` Catalin Marinas
2016-08-24 10:30                           ` Catalin Marinas
2016-08-25  1:42                           ` Leizhen (ThunderTown)
2016-08-25  1:42                             ` Leizhen (ThunderTown)
2016-08-25  9:30                             ` Catalin Marinas
2016-08-25  9:30                               ` Catalin Marinas
2016-08-25 11:27                               ` Leizhen (ThunderTown) [this message]
2016-08-25 11:27                                 ` Leizhen (ThunderTown)

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=57BED633.1060802@huawei.com \
    --to=thunder.leizhen@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.