On 2017. 03. 19. 21:52, Rick Macklem wrote:
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
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 
      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 
back to the server at that point and if that is done in VOP_SET_TEXT() as this 
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

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
Actually I want to test it, but you guys are so vehemently discussing it, I thought it would be better to do so, once you guys settled your analysis on the code. Also, me not having the problem occurring, I don't think would mean it's solved, since that would only mean, the codepath for my specific usecase works. There might be other things there as well, what I don't hit.

Let me know which patch should I test, and I will see to it in the next couple of days, when I get the time to do it.


freebsd-current@freebsd.org mailing list
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to