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
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
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.
Regards,
-czg
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
