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> Subject: Re: [PATCH 1/1] arm64/hugetlb: clear PG_dcache_clean if the page is dirty when munmap Date: Wed, 24 Aug 2016 17:00:50 +0800 [thread overview] Message-ID: <57BD6242.1080801@huawei.com> (raw) In-Reply-To: <20160823172852.GB16213@e104818-lin.cambridge.arm.com> On 2016/8/24 1:28, Catalin Marinas wrote: > On Mon, Aug 22, 2016 at 12:19:04PM +0800, Leizhen (ThunderTown) wrote: >> On 2016/7/20 17:19, Catalin Marinas wrote: >>> On Wed, Jul 20, 2016 at 10:46:27AM +0800, Leizhen (ThunderTown) 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<--------------------- >>>> >>>> Do you plan to send this patch? My colleagues told me that if our >>>> patches are quite different, it should be Signed-off-by you. >>> >>> The reason I'm not sending it is that I don't fully understand how it >>> solves the problem for a shared file mmap(), not just hugetlbfs. As I >>> said in an earlier email: after an msync() in user space we >>> should flush the pages to disk via write_cache_pages(). This function >> Hi Catalin: >> I'm so sorry for my fault. The previous small pages test result I actually ran on ramfs. >> Today, I ran the case on harddisk fs, it worked well without this patch. >> >> Summarized as follows: >> small pages on ramfs: need this patch >> small pages on harddisk fs: no need this patch >> hugetlbfs: need this patch > > I would add: > > small pages over nfs: fails with or without this patch > > (tested on Juno, Cortex-A57; seems to be fixed if I remove the > PG_dcache_clean test altogether but, well, we end up over-flushing) > > I assume that when using a hard drive, it goes through the block I/O > layer and we may have a flush_dcache_page() called when the kernel is > about to read a page that has been mapped in user space. This would > clear the PG_dcache_clean bit and subsequent __sync_icache_dcache() > would perform cache maintenance. > > Could you try on your system the test case without the msync() call? I'm According to my test results: without msync, the test case may failed. 10-175-112-211:~ # ./tst_small_page_no_msync Test is Failed: The result is 0x316b9, expect = 0x365a5 10-175-112-211:~ # ./tst_small_page_no_msync Test is Failed: The result is 0x31023, expect = 0x31efa 10-175-112-211:~ # ./tst_small_page_no_msync Test is Passed: The result is 0x31efa, expect = 0x31efa 10-175-112-211:~ # ./tst_small_page Test is Passed: The result is 0x31eb7, expect = 0x31eb7 10-175-112-211:~ # ./tst_small_page Test is Passed: The result is 0x3111f, expect = 0x3111f 10-175-112-211:~ # ./tst_small_page Test is Passed: The result is 0x3111f, expect = 0x3111f > not sure whether munmap() would trigger an immediate write-back, in > which case we may see the issue even with the filesystem on a hard > drive. >
WARNING: multiple messages have this Message-ID (diff)
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: Wed, 24 Aug 2016 17:00:50 +0800 [thread overview] Message-ID: <57BD6242.1080801@huawei.com> (raw) In-Reply-To: <20160823172852.GB16213@e104818-lin.cambridge.arm.com> On 2016/8/24 1:28, Catalin Marinas wrote: > On Mon, Aug 22, 2016 at 12:19:04PM +0800, Leizhen (ThunderTown) wrote: >> On 2016/7/20 17:19, Catalin Marinas wrote: >>> On Wed, Jul 20, 2016 at 10:46:27AM +0800, Leizhen (ThunderTown) 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<--------------------- >>>> >>>> Do you plan to send this patch? My colleagues told me that if our >>>> patches are quite different, it should be Signed-off-by you. >>> >>> The reason I'm not sending it is that I don't fully understand how it >>> solves the problem for a shared file mmap(), not just hugetlbfs. As I >>> said in an earlier email: after an msync() in user space we >>> should flush the pages to disk via write_cache_pages(). This function >> Hi Catalin: >> I'm so sorry for my fault. The previous small pages test result I actually ran on ramfs. >> Today, I ran the case on harddisk fs, it worked well without this patch. >> >> Summarized as follows: >> small pages on ramfs: need this patch >> small pages on harddisk fs: no need this patch >> hugetlbfs: need this patch > > I would add: > > small pages over nfs: fails with or without this patch > > (tested on Juno, Cortex-A57; seems to be fixed if I remove the > PG_dcache_clean test altogether but, well, we end up over-flushing) > > I assume that when using a hard drive, it goes through the block I/O > layer and we may have a flush_dcache_page() called when the kernel is > about to read a page that has been mapped in user space. This would > clear the PG_dcache_clean bit and subsequent __sync_icache_dcache() > would perform cache maintenance. > > Could you try on your system the test case without the msync() call? I'm According to my test results: without msync, the test case may failed. 10-175-112-211:~ # ./tst_small_page_no_msync Test is Failed: The result is 0x316b9, expect = 0x365a5 10-175-112-211:~ # ./tst_small_page_no_msync Test is Failed: The result is 0x31023, expect = 0x31efa 10-175-112-211:~ # ./tst_small_page_no_msync Test is Passed: The result is 0x31efa, expect = 0x31efa 10-175-112-211:~ # ./tst_small_page Test is Passed: The result is 0x31eb7, expect = 0x31eb7 10-175-112-211:~ # ./tst_small_page Test is Passed: The result is 0x3111f, expect = 0x3111f 10-175-112-211:~ # ./tst_small_page Test is Passed: The result is 0x3111f, expect = 0x3111f > not sure whether munmap() would trigger an immediate write-back, in > which case we may see the issue even with the filesystem on a hard > drive. >
next prev parent reply other threads:[~2016-08-24 9:04 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) [this message] 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) 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=57BD6242.1080801@huawei.com \ --to=thunder.leizhen@huawei.com \ --cc=Steve.Capper@arm.com \ --cc=catalin.marinas@arm.com \ --cc=dingtianhong@huawei.com \ --cc=dwoods@ezchip.com \ --cc=fangwei1@huawei.com \ --cc=guohanjun@huawei.com \ --cc=huxinwei@huawei.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lizefan@huawei.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: linkBe 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.