Date: Fri, 17 Mar 2017 13:53:46 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Dimitry Andric <dim@FreeBSD.org>, Ian Lepore <ian@FreeBSD.org>, "Gergely Czuczy" <gergely.czuczy@harmless.hu>, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: process killed: text file modification Message-ID: <YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <20170317083605.GQ16105@kib.kiev.ua> References: <d4d04499-17f8-e3d7-181f-c8ee8285e32b@harmless.hu> <646c1395-9482-b214-118c-01573243ae5a@harmless.hu> <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> <FF55DB37-4A6B-4D88-B201-B3BCA1A11E87@FreeBSD.org> <YTXPR01MB01898D49933A82170FCAB7A0DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <YTXPR01MB018944EF4248402AD421D825DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM>, <20170317083605.GQ16105@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
--_002_YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390YTXPR01MB0189CANP_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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. What the patch was missing was updating n_mtime after the dirty page flush. Btw, a flush without OBJPC_SYNC happens when the file is VOP_CLOSE()'d unless the default value of vfs.nfs.clean_[ages_on_close is changed, which I think is why the 1sec resolution always seemed to work, at least for the example where there was an munmap before close. Attached is an updated version with that in it, rick ________________________________________ From: owner-freebsd-current@freebsd.org <owner-freebsd-current@freebsd.org>= on behalf of Konstantin Belousov <kostikbel@gmail.com> Sent: Friday, March 17, 2017 4:36:05 AM To: Rick Macklem Cc: Dimitry Andric; Ian Lepore; Gergely Czuczy; FreeBSD Current Subject: Re: process killed: text file modification On Fri, Mar 17, 2017 at 03:10:57AM +0000, Rick Macklem wrote: > Hope you don't mind a top post... > Attached is a little patch you could test maybe? > > rick > ________________________________________ > From: owner-freebsd-current@freebsd.org <owner-freebsd-current@freebsd.or= g> on behalf of Rick Macklem <rmacklem@uoguelph.ca> > Sent: Thursday, March 16, 2017 9:57:23 PM > To: Dimitry Andric; Ian Lepore > Cc: Gergely Czuczy; FreeBSD Current > Subject: Re: process killed: text file modification > > Dimitry Andric wrote: > [lots of stuff snipped] > > I'm also running into this problem, but while using lld. I must set > > vfs.timestamp_precision to 1 (e.g. sec + ns accurate to 1/HZ) on both > > the client and the server, to make it work. > > > > Instead of GNU ld, lld uses mmap to write to the output executable. If > > this executable is more than one page, and resides on an NFS share, > > running it will almost always result in "text file modification", if > > vfs_timestamp_precision >=3D 2. > > > > A small test case: http://www.andric.com/freebsd/test-mmap-write.c, > > which writes a simple "hello world" i386-freebsd executable file, using > > the sequence: open() -> ftruncate() -> mmap() -> memcpy() -> munmap() -= > > > close(). > Hopefully Kostik will correct me if I have this wrong, but I don't believ= e any > of the above syscalls guarantee that dirty pages have been flushed. > At least for cases without munmap(), the writes of dirty pages can occur = after > the file descriptor is closed. I run into this in NFSv4, where there is a= Close (NFSv4 one) > that can't be done until VOP_INACTIVE(). > If you look in the NFS VOP_INACTIVE() { called ncl_inactive() } you'll se= e: > if (NFS_ISV4(vp) && vp->v_type =3D=3D VREG) { > 237 /* > 238 * Since mmap()'d files do I/O after VOP_CLOSE(),= the NFSv4 > 239 * Close operations are delayed until now. Any di= rty > 240 * buffers/pages must be flushed before the close= , so that the > 241 * stateid is available for the writes. > 242 */ > 243 if (vp->v_object !=3D NULL) { > 244 VM_OBJECT_WLOCK(vp->v_object); > 245 retv =3D vm_object_page_clean(vp->v_objec= t, 0, 0, > 246 OBJPC_SYNC); > 247 VM_OBJECT_WUNLOCK(vp->v_object); > 248 } else > 249 retv =3D TRUE; > 250 if (retv =3D=3D TRUE) { > 251 (void)ncl_flush(vp, MNT_WAIT, NULL, ap->a= _td, 1, 0); > 252 (void)nfsrpc_close(vp, 1, ap->a_td); > 253 } > 254 } > Note that nothing like this is done for NFSv3. > What might work is implementing a VOP_SET_TEXT() vnode op for the NFS > client that does most of the above (except for nfsrpc_close()) and then s= ets > VV_TEXT. > --> That way, all the dirty pages will be flushed to the server before th= e executable > starts executing. > > > Running this on an NFS share, and then attempting to run the resulting > > 'helloworld' executable will result in the "text file modification" > > error, and it will be killed. But if you simply copy the executable to > > something else, then it runs fine, even if you use -p to retain the > > properties! > > > > IMHO this is a rather surprising problem with the NFS code, and Kostik > > remarked that the problem seems to be that the VV_TEXT flag is set too > > early, before the nfs cache is invalidated. Rick, do you have any idea= s > > about this? > I don't think it is the "nfs cache" that needs invalidation, but the dirt= y > pages written by the mmap'd file need to be flushed, before the VV_TEXT > is set, I think? > > If Kostik meant "attribute cache" when he said "nfs cache", I'll note tha= t the > cached attributes (including mtime) are updated by the reply to every wri= te. > (As such, I think it is the writes that must be done before setting VV_TE= XT > that is needed.) > > It is a fairly simple patch to create. I'll post one to this thread in a = day or so > unless Kostik thinks this isn't correct and not worth trying. > I think that the patch is in right direction, but I am not convinced that it is enough. What we need is to ensure that mtime on server does not change after VV_TEXT is set. Dirty pages indeed would cause the mtime update on flush, but wouldn't dirty buffers writes cause the same issue ? In other words, I think that enchanced VOP_SET_TEXT() for nfs must flush everything to ensure that mtime on server would not change due to further actions by this machine' nfs client. _______________________________________________ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" --_002_YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390YTXPR01MB0189CANP_ Content-Type: application/octet-stream; name="textmod.patch" Content-Description: textmod.patch Content-Disposition: attachment; filename="textmod.patch"; size=1343; creation-date="Fri, 17 Mar 2017 13:49:47 GMT"; modification-date="Fri, 17 Mar 2017 13:49:47 GMT" Content-Transfer-Encoding: base64 LS0tIGZzL25mc2NsaWVudC9uZnNfY2x2bm9wcy5jLnRleHQJMjAxNy0wMy0xNiAyMTo1NToxNi4y NjMzOTMwMDAgLTA0MDAKKysrIGZzL25mc2NsaWVudC9uZnNfY2x2bm9wcy5jCTIwMTctMDMtMTcg MDk6MzE6MjMuNjMyODE0MDAwIC0wNDAwCkBAIC0xNDAsNiArMTQwLDcgQEAgc3RhdGljIHZvcF9h ZHZsb2NrX3QJbmZzX2FkdmxvY2s7CiBzdGF0aWMgdm9wX2FkdmxvY2thc3luY190IG5mc19hZHZs b2NrYXN5bmM7CiBzdGF0aWMgdm9wX2dldGFjbF90IG5mc19nZXRhY2w7CiBzdGF0aWMgdm9wX3Nl dGFjbF90IG5mc19zZXRhY2w7CitzdGF0aWMgdm9wX3NldF90ZXh0X3QgbmZzX3NldF90ZXh0Owog CiAvKgogICogR2xvYmFsIHZmcyBkYXRhIHN0cnVjdHVyZXMgZm9yIG5mcwpAQCAtMTc2LDYgKzE3 Nyw3IEBAIHN0cnVjdCB2b3BfdmVjdG9yIG5ld25mc192bm9kZW9wcyA9IHsKIAkudm9wX3dyaXRl ID0JCW5jbF93cml0ZSwKIAkudm9wX2dldGFjbCA9CQluZnNfZ2V0YWNsLAogCS52b3Bfc2V0YWNs ID0JCW5mc19zZXRhY2wsCisJLnZvcF9zZXRfdGV4dCA9CQluZnNfc2V0X3RleHQsCiB9OwogCiBz dHJ1Y3Qgdm9wX3ZlY3RvciBuZXduZnNfZmlmb29wcyA9IHsKQEAgLTMzNzMsNiArMzM3NSwyOSBA QCBuZnNfc2V0YWNsKHN0cnVjdCB2b3Bfc2V0YWNsX2FyZ3MgKmFwKQogCXJldHVybiAoZXJyb3Ip OwogfQogCitzdGF0aWMgaW50CituZnNfc2V0X3RleHQoc3RydWN0IHZvcF9zZXRfdGV4dF9hcmdz ICphcCkKK3sKKwlzdHJ1Y3Qgdm5vZGUgKnZwID0gYXAtPmFfdnA7CisJc3RydWN0IG5mc25vZGUg Km5wOworCisJLyoKKwkgKiBJZiB0aGUgdGV4dCBmaWxlIGhhcyBiZWVuIG1tYXAnZCwgdGhlIGRp cnR5IHBhZ2VzIG11c3QgYmUgZmx1c2hlZAorCSAqIHNvIHRoYXQgdGhlIG1vZGlmeSB0aW1lIG9m IHRoZSBmaWxlIHdpbGwgYmUgdXAgdG8gZGF0ZS4KKwkgKi8KKwlpZiAodnAtPnZfb2JqZWN0ICE9 IE5VTEwpIHsKKwkJbnAgPSBWVE9ORlModnApOworCQlWTV9PQkpFQ1RfV0xPQ0sodnAtPnZfb2Jq ZWN0KTsKKwkJdm1fb2JqZWN0X3BhZ2VfY2xlYW4odnAtPnZfb2JqZWN0LCAwLCAwLCBPQkpQQ19T WU5DKTsKKwkJVk1fT0JKRUNUX1dVTkxPQ0sodnAtPnZfb2JqZWN0KTsKKwkJbXR4X2xvY2soJm5w LT5uX210eCk7CisJCW5wLT5uX210aW1lID0gbnAtPm5fdmF0dHIubmFfbXRpbWU7CisJCW10eF91 bmxvY2soJm5wLT5uX210eCk7CisJfQorCXZwLT52X3ZmbGFnIHw9IFZWX1RFWFQ7CisJcmV0dXJu ICgwKTsKK30KKwogLyoKICAqIFJldHVybiBQT1NJWCBwYXRoY29uZiBpbmZvcm1hdGlvbiBhcHBs aWNhYmxlIHRvIG5mcyBmaWxlc3lzdGVtcy4KICAqLwo= --_002_YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390YTXPR01MB0189CANP_--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390>