From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46A9ACA9EC3 for ; Thu, 31 Oct 2019 04:55:54 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A716820862 for ; Thu, 31 Oct 2019 04:55:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=fb.com header.i=@fb.com header.b="PmQVW+UN"; dkim=pass (1024-bit key) header.d=fb.onmicrosoft.com header.i=@fb.onmicrosoft.com header.b="JSHA4MUK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A716820862 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=fb.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 3EEC46B0003; Thu, 31 Oct 2019 00:55:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 377746B0005; Thu, 31 Oct 2019 00:55:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1CA6B6B0007; Thu, 31 Oct 2019 00:55:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0063.hostedemail.com [216.40.44.63]) by kanga.kvack.org (Postfix) with ESMTP id D89336B0003 for ; Thu, 31 Oct 2019 00:55:52 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with SMTP id 653B2181AEF3C for ; Thu, 31 Oct 2019 04:55:52 +0000 (UTC) X-FDA: 76102867344.16.error63_6d4d33f854449 X-HE-Tag: error63_6d4d33f854449 X-Filterd-Recvd-Size: 55405 Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) by imf24.hostedemail.com (Postfix) with ESMTP for ; Thu, 31 Oct 2019 04:55:50 +0000 (UTC) Received: from pps.filterd (m0148461.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id x9V4rXMj021496; Wed, 30 Oct 2019 21:55:34 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : mime-version; s=facebook; bh=hWOtGArU9r+rKtQ5YenVx79YjB4nGIk1+vuVEX4Imog=; b=PmQVW+UN9q3z0f+lmknyw4psoynt/z3q7dYRi/ELiJ6Y3Jrrt3Y1QTtCFRpOQNr2wILX y+vyGrcdP281OWSih18WenEZ3nnYiy/hdskowLD3OXSUmHYPhTqaDKV5QBHCqT4ONy7n wI8Uu/oiFv/UaLeP6mrcL+IHMFZ2dw9qeP4= Received: from mail.thefacebook.com (mailout.thefacebook.com [199.201.64.23]) by mx0a-00082601.pphosted.com with ESMTP id 2vybrebyqj-7 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 30 Oct 2019 21:55:33 -0700 Received: from prn-mbx04.TheFacebook.com (2620:10d:c081:6::18) by prn-hub01.TheFacebook.com (2620:10d:c081:35::125) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.1.1713.5; Wed, 30 Oct 2019 21:55:31 -0700 Received: from prn-hub04.TheFacebook.com (2620:10d:c081:35::128) by prn-mbx04.TheFacebook.com (2620:10d:c081:6::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.1.1713.5; Wed, 30 Oct 2019 21:55:31 -0700 Received: from NAM02-BL2-obe.outbound.protection.outlook.com (192.168.54.28) by o365-in.thefacebook.com (192.168.16.28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.1.1713.5 via Frontend Transport; Wed, 30 Oct 2019 21:55:31 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=j1gAkCln8dAs4l1NoWyXE3+plA0CETNK4ApV7xKzlzB7lhXBRyZxEr0KAPw2++Zp+FYc5ER30VuH3JqXB+eHhd9nsEuNf/VoFmIdx2Uti9TUH2EsaezwN4yhtfY7k6T8iMlYYeggOtXLLQY8egWmZUaVRWds7rcMG2JXYD8FpVPBdOiWyA9DSwPe2jHGapdNWZT5orcSRa0pjh9EWoNpA6B5ysn7TnIYXuB6FAkf+zQPAIHclhoH2xEe8rRRiweRgpFEd+U/Twc8eijrTlkuRgz489A9aTKgNJZG6DUCmAK4fMkNxUVzbRnvvrPAhL5w0/ZOTYjcNC5envSopnaS3A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=hWOtGArU9r+rKtQ5YenVx79YjB4nGIk1+vuVEX4Imog=; b=JDn618tu3oRQYMdgzR9CEGl9wFxcpGkRiDfWWVGRFaYtxXaO3exXCr/ZXnVszjaj5y7L43vyF+T6ZcH3YrtVHL41f3qToI4eD5N1FVy7wEYuuuxoIN/eGGXRHk6wCzzN9f7FPPiVFLmEqgJoJU/04mQ4j0YlCkkMgD5xPm8dOMyF0mb3fiqsUKOhznLyMbYaQ3SOZkYvpuD3ajv9jBXzc+Gfuq64qhMgJNdQ4LTunlwn0XDukQb5oZyjvHNnQmiN/DN7cDjT1VdlYgcq3VNxVVy0DWfFFo4BAHi6T8PDMrr3A9SPQ2sV/A9cBOrfQW1AzSE43PHBSmN3rS2hDsRHiw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=fb.com; dmarc=pass action=none header.from=fb.com; dkim=pass header.d=fb.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.onmicrosoft.com; s=selector2-fb-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=hWOtGArU9r+rKtQ5YenVx79YjB4nGIk1+vuVEX4Imog=; b=JSHA4MUKnOPgse5obMkYrJZ5Ch3IH2O1TlV2VGz4Ngf2x/IsL4ra4PAKroRPSjO52iy9updNvQXrDGKN3YldAUCu7kEqSxsy4mZX32Mzz/vrylIEQUTbKxTLUKfu96xECnhSoIHW/pJ9Vz/nbIsops3KxZ0kF9K2W7I3gx23f68= Received: from MWHPR15MB1165.namprd15.prod.outlook.com (10.175.3.22) by MWHPR15MB1728.namprd15.prod.outlook.com (10.174.98.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2408.17; Thu, 31 Oct 2019 04:55:29 +0000 Received: from MWHPR15MB1165.namprd15.prod.outlook.com ([fe80::fdc8:5546:bace:15f5]) by MWHPR15MB1165.namprd15.prod.outlook.com ([fe80::fdc8:5546:bace:15f5%5]) with mapi id 15.20.2387.027; Thu, 31 Oct 2019 04:55:29 +0000 From: Song Liu To: Andrew Morton CC: Linux MM , Kernel Team , "Kirill A . Shutemov" , Hugh Dickins , William Kucharski , Johannes Weiner , Matthew Wilcox Subject: Re: [PATCH] mm/thp: flush file for !is_shmem PageDirty() case in collapse_file() Thread-Topic: [PATCH] mm/thp: flush file for !is_shmem PageDirty() case in collapse_file() Thread-Index: AQHVj12wRWDZ2/wcSkePDdk389uWy6dz/KCAgAAzW4A= Date: Thu, 31 Oct 2019 04:55:29 +0000 Message-ID: <4542F512-D964-44AD-9BAA-6683C540AF7E@fb.com> References: <20191030200736.3455046-1-songliubraving@fb.com> <20191030185115.ad780e74c66b6286789559fd@linux-foundation.org> In-Reply-To: <20191030185115.ad780e74c66b6286789559fd@linux-foundation.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-mailer: Apple Mail (2.3594.4.19) x-originating-ip: [2620:10d:c090:180::133c] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 74a6bf5c-dffc-4226-c74f-08d75dbe8d75 x-ms-traffictypediagnostic: MWHPR15MB1728: x-ms-exchange-purlcount: 1 x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 02070414A1 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(396003)(39860400002)(366004)(376002)(136003)(346002)(54534003)(199004)(189003)(476003)(486006)(99286004)(81166006)(6916009)(186003)(8676002)(229853002)(76116006)(36756003)(66946007)(66476007)(66556008)(46003)(66446008)(64756008)(2616005)(446003)(4326008)(81156014)(256004)(50226002)(6486002)(8936002)(14454004)(6436002)(7736002)(606006)(14444005)(76176011)(2906002)(966005)(6512007)(102836004)(316002)(54896002)(6306002)(478600001)(71190400001)(6246003)(6506007)(53546011)(40265005)(71200400001)(6116002)(11346002)(5660300002)(86362001)(25786009)(33656002)(54906003)(236005)(579004);DIR:OUT;SFP:1102;SCL:1;SRVR:MWHPR15MB1728;H:MWHPR15MB1165.namprd15.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; received-spf: None (protection.outlook.com: fb.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: yB0mY7cNfjknNb6yUJedM9X2MWpiMmjBQ3aX0WSAcyZfetO9LxboMSVKSe24vlsrUmgo2zzxcgCOyjR25ELUds5qtOSEo0vAide4OFktV8Glp+iPv0SlKE7h9PsYJJjsVns2nWhEkQr67zbWn7iwfWFCPzedG1KB+mO1uEBdSnBUPVcsiLIxS4PMA/+dUWYeJs1FjaftIclaE65CaQIUS+T0x158ms0TcxYQAhtuQnjMxG4hrRz9bwCCk+BP4byRfsMFe82zjptMbtQJ46z1QRR08GUblEiUUjPtci7iEATpGABrN5ZRVx2QlvT/NfQMngoGwNseYNCbJfaZtMn5Q2fntP+koy7r3Xlv/W4ZOhgGVgfLL9YtrqIcXyzoPBocv0wHndws+rSVNb+EoAqOh9J6hz9x4cI42b+TnkcZ/lVtPx89byvBTQ2Vi2CaUfVXor1RHTL8y0SBb+AYAi/WfcnmlBlOxh7b/VnwtZN2xnM= Content-Type: multipart/alternative; boundary="_000_4542F512D96444AD9BAA6683C540AF7Efbcom_" MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 74a6bf5c-dffc-4226-c74f-08d75dbe8d75 X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Oct 2019 04:55:29.1861 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: n80D6cNIZGomXvTNoGSSwRif0wezBMQsINQB7ElHSdqTpy8foZYN3gjxzzZaHX8GobEDrITReOevGvETWJ3uuA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR15MB1728 X-OriginatorOrg: fb.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,1.0.8 definitions=2019-10-31_01:2019-10-30,2019-10-31 signatures=0 X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 mlxlogscore=999 spamscore=0 lowpriorityscore=0 bulkscore=0 priorityscore=1501 suspectscore=0 mlxscore=0 impostorscore=0 malwarescore=0 adultscore=0 phishscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-1908290000 definitions=main-1910310048 X-FB-Internal: deliver X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --_000_4542F512D96444AD9BAA6683C540AF7Efbcom_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable On Oct 30, 2019, at 6:51 PM, Andrew Morton > wrote: On Wed, 30 Oct 2019 13:07:36 -0700 Song Liu > wrote: For non-shmem file THPs, khugepaged only collapses read only .text mapping (VM_DENYWRITE). These pages should not be dirty except the case where the file hasn't been flushed since first write. Call filemap_flush() in collapse_file() to accelerate the write back in such cases. Also add warning if PageDirty() triggered for pages from readahead path. Reported-and-tested-by: syzbot+efb9e48b9fbdc49bb34a@syzkaller.appspotmail.c= om If sysbot reported something then we definitely want to be able to review that report when reviewing the patch! So please do include a Link: for that sort of thing. A bit of sleuthing leads me to http://lkml.kernel.org/r/000000000000c50fd70595fdd5b2@google.com which shows that this patch is in fact a fix against mmthp-recheck-each-page-before-collapsing-file-thp.patch. This very important information wasn't in the changelog. And this patch doesn't really fix the sysbot hang, does it? The patch restores the flush which was removed in order to fix the sysbot hang. In general, please do try to include all this sort of information when preparing changelogs. Sorry for the confusion. This one is a bit tricky. syzbot reported hang issue with filemap_flush(), which is already fixed by previous patch which removes filemap_flush(). This patch tries to add filemap_flush() back, in a proper way. So this patch is an improvement, not a fix. That's why I didn't include the fixes tag. However, I used syzbot to test this patch, by replying "#syz test: ...". to the previous report (the one you found). I didn't cc the mail list for those tests. syzbot did find bug with an earlier version, and gave green light for this version. This is the reason I would like to give syzbot credit with the tag. Maybe I should just make it "Tested-by: syzbot"? --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -1601,6 +1601,33 @@ static void collapse_file(struct mm_struct *mm, result =3D SCAN_FAIL; goto xa_unlocked; } + if (WARN_ON_ONCE(PageDirty(page))) { I'm not understanding what guarantees this. Can't another process which has the file open for writing come in and dirty the page after the readahead has completed and before this process locks the page? For non-shmem file, khugepaged only work for VM_DENYWRITE mappings. Therefore, the page cannot be opened for write by another process. + /* + * page from readahead should not + * be dirty. Show warning if this + * somehow happens. + */ + result =3D SCAN_FAIL; + goto out_unlock; + } + } else if (PageDirty(page)) { + /* + * khugepaged only works on read-only fd, + * so this page is dirty because it hasn't + * been flushed since first write. There + * won't be new dirty pages. + * + * Trigger async flush here and hope the + * writeback is done when khugepaged + * revisits this page. + * + * This is a one-off situation. We are not + * forcing writeback in loop. + */ + xas_unlock_irq(&xas); + filemap_flush(mapping); + result =3D SCAN_FAIL; + goto xa_unlocked; } else if (trylock_page(page)) { get_page(page); xas_unlock_irq(&xas); The patch mmthp-recheck-each-page-before-collapsing-file-thp.patch has undergone quite a bit of churn so I don't think it should be mainlined without more testing and review. But it fixes a significant issue. So could the appropriate developers please take some time to recheck and retest it all? We are running production tests with previous version (no filemap_flush). syzbot also gives green light to that one. Maybe we should ship the version without filemap_flush with 5.4, and this improvement with 5.5? This improvement also passed syzbot's test. If this looks good, we will also add this to production tests. Thanks, Song --_000_4542F512D96444AD9BAA6683C540AF7Efbcom_ Content-Type: text/html; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable

On Oct 30, 2019, at 6:51 PM, Andrew Morton <akpm@linux-foundation.org> wrote:

On Wed, 30 Oct 2019 13:07:36 -0700 Song Liu <songliubraving@fb.com> wrote:

For non-shmem file THPs, khugepaged on= ly collapses read only .text mapping
(VM_DENYWRITE). These pages should not be dirty except the case where the file hasn't been flushed since first write.

Call filemap_flush() in collapse_file() to accelerate the write back in
such cases.

Also add warning if PageDirty() triggered for pages from readahead path.
Reported-and-tested-by: syzbot+efb9e48b9fbdc49bb34a@syzkaller.appspotmail.com

If sysbot reported something then we definitely want to be able to
review that report when reviewing the patch!  So please do include a Link: for that sort of thing.

A bit of sleuthing leads me to
http://lkml.kernel.org/r/000000000000c50fd70595fdd5b2@google.c= om

which shows that this patch is in fact a fix against
mmthp-recheck-each-page-before-collapsing-file-thp.patch.  This very important information wasn't in the changelog.

And this patch doesn't really fix the sysbot hang, does it?  The patch=
restores the flush which was removed in order to fix the sysbot hang.

In general, please do try to include all this sort of information when
preparing changelogs.

Sorry for the confusion. This one is a bit tricky.

syzbot reported hang issue with filemap_flush(), which is already fixed by previous patch which removes filemap_flush(). 

This patch tries to add filemap_flush() back, in a proper way. So this 
patch is an improvement, not a fix. That's why I didn't include the 
fixes tag. 

However, I used syzbot to test this patch, by replying "#syz test: ...".<= span class=3D"Apple-converted-space"> 
to the previous report (the one you found). I didn't cc the mail list<= br style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Co= urier; font-size: 15px; font-style: normal; font-variant-caps: normal; font= -weight: normal; letter-spacing: normal; orphans: auto; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; widows: auto; = word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-widt= h: 0px; text-decoration: none;" class=3D""> for those tests. syzbot did find bug with an earlier version, and gave 
green light for this version. This is the reason I would like to give 
syzbot credit with the tag. Maybe I should just make it "Tested-by: 
syzbot"? 



--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1601,6 +1601,33 @@ static void collapse_file(struct mm_struct *mm,<= br class=3D""> result =3D SCAN_FAIL;
goto xa_unlocked;
}
+ if (WARN_ON_ONCE(PageDirty(pa= ge))) {

I'm not understanding what guarantees this.  Can't another process
which has the file open for writing come in and dirty the page after
the readahead has completed and before this process locks the page?

For non-shmem file, khugepaged only work for VM_DENYWRITE mappings. 
Therefore, the page cannot be opened for write by another process.


+ /*
+ =  * page from readahead should not
+ =  * be dirty. Show warning if this
+ =  * somehow happens.
+ =  */
+ result =3D SCAN_FAIL;
+ goto out_unlock;
+ }
+ } else if (PageDirty= (page)) {
+ /*
+  * khugepaged only works on read-only fd,
+  * so this page is dirty because it hasn't
+  * been flushed since first write. There
+  * won't be new dirty pages.
+  *
+  * Trigger async flush here and hope the
+  * writeback is done when khugepaged
+  * revisits this page.
+  *
+  * This is a one-off situation. We are not
+  * forcing writeback in loop.
+  */
+ xas_unlock_irq(&xas);
+ filemap_flush(mapping);
+ result =3D SCAN_FAIL;
+ goto xa_unlocked;
} else if (trylock_page(pag= e)) {
get_page(page);
xas_unlock_irq(&xas);

The patch mmthp-recheck-each-page-before-collapsing-file-thp.patch has
undergone quite a bit of churn so I don't think it should be mainlined
without more testing and review.  But it fixes a significant issue. &n= bsp;So
could the appropriate developers please take some time to recheck and
retest it all?

We are running production tests with previous version (no filemap_flush). 
syzbot also gives green light to that one. Maybe we should ship the 
version without filemap_flush with 5.4, and this improvement with 5.5?

This improvement also passed syzbot's test. If this looks good, we will 
also add this to production tests. <= /span>

Thanks,
Song --_000_4542F512D96444AD9BAA6683C540AF7Efbcom_--