Re: process killed: text file modification

Kostik wrote:
[stuff snipped]
>> >> Dirty pages are flushed by writes, so if we have a set of dirty pages and
>> >> 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.
> But dirty buffers can appear on the vnode queue due to dirty pages msyncing
> by syncer, for instance.
Ok, just to clarify this, in case I don't understand it...
- You aren't saying that anything will be added to v_bufobj.bo_dirty.bv_hd by
  vfs_msync() or similar, after VOP_CLOSE(), right?
--> ncl_flush() { was called nfs_flush() in the old NFS client } only deals with
     "struct buf's" hanging off v_bufobj.bo_dirty.bv_hd, so I don't see a use for
     it in the patch.

As for pages added to v_bufobj.bo_object...the patch assumes that the process
that was writing the executable file mmap'd is done { normally exited } before
the exec() syscall occurs. If it is still dirtying pages when the exec() occurs, then
failing with "Text file modified" seems correct to me. As you mentioned, another
client can do this to the file anyhow.

My understanding is that vm_object_page_clean() will get all the dirty pages written
back to the server at that point and if that is done in VOP_SET_TEXT() as this patch
does, what more can the NFS client do?

[more stuff snipped]
> Syncer does not open the vnode inside the vfs_msync() operations.
Ok, but this doesn't put "struct buf's" on v_bufobj.bo_dirty.bv_hd. Am I right?
(When I said "buffers". I meant "struct buf's" under bo_dirty, not stuff under
 v_bufobj.bo_object.)

> We do track writeability to the file, and do not allow execution if there is
> an active writer, be it a file descriptor opened for write, or a writeable
> mapping.  And in reverse, if the file is executed (VV_TEXT is set), then
> we disallow opening the file for write.
Yes, and that was why I figured doing this in VOP_SET_TEXT(), just before
setting VV_TEXT, was the right place to do it.
[more stuff snipped]
>
> Thanks for testing the patch. Now, if others can test it...rick
>
Again, hopefully others (especially the original reporter) will be able to
test the patch, rick