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_at_freebsd.org <owner-freebsd-current_at_freebsd.org> on behalf of Konstantin Belousov <kostikbel_at_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_at_freebsd.org <owner-freebsd-current_at_freebsd.org> on behalf of Rick Macklem <rmacklem_at_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 >= 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 believe 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 see: > if (NFS_ISV4(vp) && vp->v_type == VREG) { > 237 /* > 238 * Since mmap()'d files do I/O after VOP_CLOSE(), the NFSv4 > 239 * Close operations are delayed until now. Any dirty > 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 != NULL) { > 244 VM_OBJECT_WLOCK(vp->v_object); > 245 retv = vm_object_page_clean(vp->v_object, 0, 0, > 246 OBJPC_SYNC); > 247 VM_OBJECT_WUNLOCK(vp->v_object); > 248 } else > 249 retv = TRUE; > 250 if (retv == 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 sets > VV_TEXT. > --> That way, all the dirty pages will be flushed to the server before the 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 ideas > > about this? > I don't think it is the "nfs cache" that needs invalidation, but the dirty > 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 that the > cached attributes (including mtime) are updated by the reply to every write. > (As such, I think it is the writes that must be done before setting VV_TEXT > 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_at_freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:10 UTC