Re: process killed: text file modification

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"