From owner-freebsd-current@freebsd.org Fri Mar 17 21:23:03 2017 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E969ED1077B for ; Fri, 17 Mar 2017 21:23:03 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from CAN01-QB1-obe.outbound.protection.outlook.com (mail-eopbgr660088.outbound.protection.outlook.com [40.107.66.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (Client CN "mail.protection.outlook.com", Issuer "Microsoft IT SSL SHA2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9462A1682; Fri, 17 Mar 2017 21:23:02 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM (10.165.218.133) by YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM (10.165.218.133) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.961.17; Fri, 17 Mar 2017 21:23:00 +0000 Received: from YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM ([10.165.218.133]) by YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM ([10.165.218.133]) with mapi id 15.01.0961.022; Fri, 17 Mar 2017 21:23:00 +0000 From: Rick Macklem To: Dimitry Andric , Konstantin Belousov CC: Ian Lepore , Gergely Czuczy , FreeBSD Current Subject: Re: process killed: text file modification Thread-Topic: process killed: text file modification Thread-Index: AQHSnqPLfXcZwdtVHkecT5jK6Yv9dKGYQJIXgAAZ/baAAFsxgIAAVqF+gAAJQ4CAAFvPgIAAF1PP Date: Fri, 17 Mar 2017 21:23:00 +0000 Message-ID: References: <45436522-77df-f894-0569-737a6a74958f@harmless.hu> <55189643.aaZPuY77s8@ralph.baldwin.cx> <3ed3e4a3-23af-7267-39f1-9090093c9c1e@harmless.hu> <5ac94b9a-7ced-9eff-d746-7dddaaeca516@harmless.hu> <1489340839.40576.82.camel@freebsd.org> <20170317083605.GQ16105@kib.kiev.ua> <20170317141917.GS16105@kib.kiev.ua>, In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: FreeBSD.org; dkim=none (message not signed) header.d=none;FreeBSD.org; dmarc=none action=none header.from=uoguelph.ca; x-ms-office365-filtering-correlation-id: 8179d254-b3e6-4d37-f895-08d46d7bca20 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:YTXPR01MB0189; x-microsoft-exchange-diagnostics: 1; YTXPR01MB0189; 7:/osaAg8Os3ZzfxGag1knvqOJDhcgnvN+qr0LZacZ/u+mdo2Z4SUlcct0TGcZq88rDrxhdj7m2Mci2WKVQuVUnEb2cvcqmQuHFgl/sRM+ZFahUk52pXVtyQIjaWdt95a/VWI7JG4qKqhb0U0/p7LoT8SKorVWwKrtaTl8ZzaaJ91JsF+Vt/lgjEqynoiHfaX8ch4on/YblPT2Vj5EWykDcWiphA8Res6fCcL2z+sStGzav4vMZ3ib/G0qpEa1RJ9i1S6hVfvCvSNhbzhspJmaGN1RpvOcwFqwsQXh1WIR/tD+RP5DnwNGJt54JALiJESCeKXUg21dhDNfquUp+UCB8g== x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(158342451672863); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6041248)(20161123558025)(20161123560025)(20161123555025)(20161123564025)(20161123562025)(6072148); SRVR:YTXPR01MB0189; BCL:0; PCL:0; RULEID:; SRVR:YTXPR01MB0189; x-forefront-prvs: 0249EFCB0B x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(39450400003)(24454002)(81166006)(102836003)(8676002)(8936002)(50986999)(86362001)(39060400002)(4326008)(74482002)(6246003)(33656002)(38730400002)(74316002)(305945005)(229853002)(2950100002)(93886004)(6506006)(9686003)(54356999)(6436002)(76176999)(122556002)(5660300001)(53936002)(189998001)(2900100001)(7696004)(3280700002)(77096006)(55016002)(54906002)(3660700001)(2906002); DIR:OUT; SFP:1101; SCL:1; SRVR:YTXPR01MB0189; H:YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM; FPR:; SPF:None; MLV:ovrnspm; PTR:InfoNoRecords; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: uoguelph.ca X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Mar 2017 21:23:00.4395 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: be62a12b-2cad-49a1-a5fa-85f4f3156a7d X-MS-Exchange-Transport-CrossTenantHeadersStamped: YTXPR01MB0189 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Mar 2017 21:23:04 -0000 Dimitry Andric wrote: >On 17 Mar 2017, at 15:19, Konstantin Belousov wrote: >> >> On Fri, Mar 17, 2017 at 01:53:46PM +0000, Rick Macklem wrote: >>> Well, I don't mind adding ncl_flush(), but it shouldn't be >>> necessary. I actually had it in the first >>> rendition of the patch, but took it out because it should happen >>> on the VOP_CLOSE() if any writing to the buffer cache happened >>> and that code hasn't changed in many years. >> Dirty pages are flushed by writes, so if we have a set of dirty pages an= d >> async vm_object_page_clean() is called on the vnode' vm_object, we get >> a bunch of delayed-write AKA dirty buffers. This is possible even after >> VOP_CLOSE() was done, e.g. by syncer performing regular run involving >> vfs_msync(). When I was talking about ncl_flush() above, I was referring to buffer cache buffers written by a write(2) syscall, not the case of mmap'd pages. >> >> I agree that the patch would not create new dirty buffers, but it is pos= sible >> to get them by other means. To write to a buffer cache block, the file would be opened by another threa= d and that is what this sanity check was meant to catch. As for dirtying pages th= at are mmap'd, as far as I understand it, the NFS client has no way of knowing if this wil= l happen more until VOP_INACTIVE() is called on the vnode. To be honest, this check could easily be deleted. After all, NFS could care= less if a file is being executed (all it sees are reads and writes). Without the check, th= e executable might do "interesting" things;-) [stuff snipped] > FWIW, Rick's patch seems to do the trick, both for my test case and lld > itself. And even with vfs.timestamp_precision=3D3 on both server and > client. Hopefully the original reporter of the problem (Gergely ??) can test the pa= tch as well. I think the patch is pretty harmless, although it could be argued that sett= ing np->m_mtime =3D np->n_nattr.na_mtime (or close to that) shouldn't happen for the case where there isn't any dirty pages found to fl= ush. However, once a file mmap'd we don't know when it does get modified anyhow (as discussed above), so setting it here doesn't seem harmful to me. Thanks for testing the patch. Now, if others can test it...rick