From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755586AbcGHPZG (ORCPT ); Fri, 8 Jul 2016 11:25:06 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:3420 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754911AbcGHPY5 (ORCPT ); Fri, 8 Jul 2016 11:24:57 -0400 Subject: Re: [PATCH 1/1] arm64/hugetlb: clear PG_dcache_clean if the page is dirty when munmap To: Catalin Marinas References: <1467893344-8352-1-git-send-email-thunder.leizhen@huawei.com> <20160707153741.GC27180@e104818-lin.cambridge.arm.com> <577F1FD9.1040205@huawei.com> <20160708135447.GB22099@e104818-lin.cambridge.arm.com> CC: Steve Capper , David Woods , Tianhong Ding , Will Deacon , linux-kernel , Xinwei Hu , Zefan Li , Hanjun Guo , linux-arm-kernel From: "Leizhen (ThunderTown)" Message-ID: <577FC5AA.5010709@huawei.com> Date: Fri, 8 Jul 2016 23:24:26 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160708135447.GB22099@e104818-lin.cambridge.arm.com> Content-Type: multipart/mixed; boundary="------------070007080608020707000303" X-Originating-IP: [10.177.23.164] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090203.577FC5B7.013B,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 852eeb0d2cf9728cad527fa79e9c3dbb Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --------------070007080608020707000303 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit On 2016/7/8 21:54, Catalin Marinas wrote: > On Fri, Jul 08, 2016 at 11:36:57AM +0800, Leizhen (ThunderTown) wrote: >> On 2016/7/7 23:37, Catalin Marinas wrote: >>> On Thu, Jul 07, 2016 at 08:09:04PM +0800, Zhen Lei wrote: >>>> At present, PG_dcache_clean is only cleared when the related huge page >>>> is about to be freed. But sometimes, there maybe a process is in charge >>>> to copy binary codes into a shared memory, and notifies other processes >>>> to execute base on that. For the first time, there is no problem, because >>>> the default value of page->flags is PG_dcache_clean cleared. So the cache >>>> will be maintained at the time of set_pte_at for other processes. But if >>>> the content of the shared memory have been updated again, there is no >>>> cache operations, because the PG_dcache_clean is still set. >>>> >>>> For example: >>>> Process A >>>> open a hugetlbfs file >>>> mmap it as a shared memory >>>> copy some binary codes into it >>>> munmap >>>> >>>> Process B >>>> open the hugetlbfs file >>>> mmap it as a shared memory, executable >>>> invoke the functions in the shared memory >>>> munmap >>>> >>>> repeat the above steps. >>> >>> Does this work as you would expect with small pages (and for example >>> shared file mmap)? I don't want to have a different behaviour between >>> small and huge pages. >> >> The small pages also have this problem, I will try to fix it too. > > Have you run the above tests on a standard file (with small pages)? It's > strange that we haven't hit this so far with gcc or something else > generating code (unless they don't use mmap but just sequential writes). The test code should be randomly generated, to make sure the context in ICache is always stale. I have attached the simplified testcase demo. The main portion is picked as below: srand(time(NULL)); ptr = (unsigned int *)share_mem; *ptr++ = 0xd2800000; //mov x0, #0 for (i = 0, total = 0; i < 100; i++) { value = 0xfff & rand(); total += value; *ptr++ = 0xb1000000 | (value << 10); //adds x0, x0, #value } *ptr = 0xd65f03c0; //ret > > If both cases need solving, we might better move the fix in the > __sync_icache_dcache() function. Untested: Yes. At first I also want to fix it as below. But I'm not sure which time the PageDirty will be cleared, and if two or more processes mmap it as executable, cache operations will be duplicated. At present, I really have not found any good place to clear PG_dcache_clean. So the below modification may be the best choice, concisely and clearly. > > ------------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<--------------------- > > BTW, can you make your tests (source) available somewhere? Both cases worked well with this patch. > > Thanks. > --------------070007080608020707000303 Content-Type: text/plain; charset="UTF-8"; name="tst_mmap.c" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="tst_mmap.c" I2luY2x1ZGUgPHN0ZGlvLmg+CiNpbmNsdWRlIDxzdGRsaWIuaD4KI2luY2x1ZGUgPHN0cmlu Zy5oPgojaW5jbHVkZSA8ZmNudGwuaD4KI2luY2x1ZGUgPGVycm5vLmg+CiNpbmNsdWRlIDxz eXMvbW1hbi5oPgojaW5jbHVkZSA8c3lzL3N0YXQuaD4KCiNkZWZpbmUgRklMRU5BTUUJCSIv bW50L2h1Z2UvdGVzdF9maWxlIgojZGVmaW5lIFRTVF9NTUFQX1NJWkUJCTB4MjAwMDAwCgp0 eXBlZGVmIHVuc2lnbmVkIGludCAoKlRFU1RfRlVOQ19UKSh2b2lkKTsKCi8qCiAqIG1rZGly IC1wIC9tbnQvaHVnZQogKiBlY2hvIDIwID4gL3Byb2Mvc3lzL3ZtL25yX2h1Z2VwYWdlcwog KiBtb3VudCBub25lIC9tbnQvaHVnZSAtdCBodWdldGxiZnMgLW8gcGFnZXNpemU9MjA0OEsK ICovCmludCBtYWluKHZvaWQpCnsKCWludCBpOwoJaW50IGZkOwoJaW50IHJldDsKCXZvaWQg KnNoYXJlX21lbTsKCXNpemVfdCBzaXplOwoJc3RydWN0IHN0YXQgc2I7CglURVNUX0ZVTkNf VCBmdW5jX3B0cjsKCXVuc2lnbmVkIGludCB2YWx1ZSwgdG90YWw7Cgl1bnNpZ25lZCBpbnQg KnB0cjsKCglmZCA9IG9wZW4oRklMRU5BTUUsIE9fUkRXUiB8IE9fQ1JFQVQpOwoJaWYgKGZk ID09IC0xKSB7CgkJcHJpbnRmKCJPcGVuIGZpbGUgJXMgZmFpbGVkIFAxOiAlc1xuIiwgRklM RU5BTUUsIHN0cmVycm9yKGVycm5vKSk7CgkJcmV0dXJuIDE7Cgl9CgoJbHNlZWsoZmQsIFRT VF9NTUFQX1NJWkUgLSAxLCBTRUVLX1NFVCk7ICANCgl3cml0ZShmZCwgIiIsIDEpOw0KCglz aGFyZV9tZW0gPSBtbWFwKE5VTEwsIFRTVF9NTUFQX1NJWkUsIFBST1RfUkVBRCB8IFBST1Rf V1JJVEUsIE1BUF9TSEFSRUQsIGZkLCAwKTsKCWlmIChzaGFyZV9tZW0gPT0gTUFQX0ZBSUxF RCkgewoJCXByaW50ZigiQ2FsbCBtbWFwIGZhaWxlZCBQMTogJXNcbiIsIHN0cmVycm9yKGVy cm5vKSk7CgkJZXhpdCgxKTsKCX0KCgljbG9zZShmZCk7CgoJc3JhbmQodGltZShOVUxMKSk7 CglwdHIgPSAodW5zaWduZWQgaW50ICopc2hhcmVfbWVtOwoJKnB0cisrID0gMHhkMjgwMDAw MDsJCQkJLy9tb3YgeDAsICMwCglmb3IgKGkgPSAwLCB0b3RhbCA9IDA7IGkgPCAxMDA7IGkr KykgewoJCXZhbHVlID0gMHhmZmYgJiByYW5kKCk7CgkJdG90YWwgKz0gdmFsdWU7CgkJKnB0 cisrID0gMHhiMTAwMDAwMCB8ICh2YWx1ZSA8PCAxMCk7CS8vYWRkcyB4MCwgeDAsICN2YWx1 ZQoJfQoJKnB0ciA9IDB4ZDY1ZjAzYzA7CQkJCS8vcmV0CgoJLy9fX2NsZWFyX2NhY2hlKChj aGFyICopc2hhcmVfbWVtLCAoY2hhciAqKXNoYXJlX21lbSArIDB4MjAwKTsKCglyZXQgPSBt c3luYyhzaGFyZV9tZW0sIFRTVF9NTUFQX1NJWkUsIE1TX1NZTkMpOwoJaWYgKHJldCkgewoJ CXByaW50ZigiQ2FsbCBtc3luYyBmYWlsZWQ6ICVzXG4iLCBzdHJlcnJvcihlcnJubykpOwoJ CWV4aXQoMSk7Cgl9CgoJcmV0ID0gbXVubWFwKHNoYXJlX21lbSwgVFNUX01NQVBfU0laRSk7 CglpZiAocmV0KSB7CgkJcHJpbnRmKCJDYWxsIG11bm1hcCBmYWlsZWQgUDE6ICVzXG4iLCBz dHJlcnJvcihlcnJubykpOwoJCWV4aXQoMSk7Cgl9CgoJZmQgPSBvcGVuKEZJTEVOQU1FLCBT X0lYVVNSKTsKCWlmIChmZCA9PSAtMSkgewoJCXByaW50ZigiT3BlbiBmaWxlICVzIGZhaWxl ZCBQMjogJXNcbiIsIEZJTEVOQU1FLCBzdHJlcnJvcihlcnJubykpOwoJCXJldHVybiAxOwoJ fQoKCWlmIChmc3RhdChmZCwgJnNiKSA9PSAtMSkgewoJCXByaW50ZigiQ2FsbCBmc3RhdCBm YWlsZWQ6ICVzXG4iLCBzdHJlcnJvcihlcnJubykpOwoJCWV4aXQoMSk7Cgl9CglzaXplID0g c2Iuc3Rfc2l6ZTsKCglmdW5jX3B0ciA9IChURVNUX0ZVTkNfVCltbWFwKE5VTEwsIHNpemUs IFBST1RfRVhFQywgTUFQX1NIQVJFRCwgZmQsIDApOwoJaWYgKGZ1bmNfcHRyID09IE1BUF9G QUlMRUQpIHsKCQlwcmludGYoIkNhbGwgbW1hcCBmYWlsZWQgUDI6ICVzXG4iLCBzdHJlcnJv cihlcnJubykpOwoJCWV4aXQoMSk7Cgl9CgoJY2xvc2UoZmQpOwoKCXZhbHVlID0gZnVuY19w dHIoKTsKCXByaW50ZigiVGVzdCBpcyAlczogVGhlIHJlc3VsdCBpcyAweCV4LCBleHBlY3Qg PSAweCV4XG4iLCAodmFsdWUgPT0gdG90YWwpID8gIlBhc3NlZCIgOiAiRmFpbGVkIiwgdmFs dWUsIHRvdGFsKTsKCglyZXQgPSBtdW5tYXAoc2hhcmVfbWVtLCBUU1RfTU1BUF9TSVpFKTsK CWlmIChyZXQpIHsKCQlwcmludGYoIkNhbGwgbXVubWFwIGZhaWxlZCBQMjogJXNcbiIsIHN0 cmVycm9yKGVycm5vKSk7CgkJZXhpdCgxKTsKCX0KCglyZXR1cm4gMDsKfQo= --------------070007080608020707000303--