Re: process killed: text file modification

Dimitry Andric wrote:
>On 17 Mar 2017, at 15:19, Konstantin Belousov <kostikbel_at_gmail.com> wrote:
>>
>> On Fri, Mar 17, 2017 at 01:53:46PM +0000, Rick Macklem wrote:
>>> 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.
>> 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.

>>
>> I agree that the patch would not create new dirty buffers, but it is possible
>> to get them by other means.
To write to a buffer cache block, the file would be opened by another thread and
that is what this sanity check was meant to catch. As for dirtying pages that are mmap'd,
as far as I understand it, the NFS client has no way of knowing if this will happen more
until VOP_INACTIVE() is called on the vnode.

To be honest, this check could easily be deleted. After all, NFS could care less if a file
is being executed (all it sees are reads and writes). Without the check, the executable
might do "interesting" things;-)
[stuff snipped]
> FWIW, Rick's patch seems to do the trick, both for my test case and lld
> itself.  And even with vfs.timestamp_precision=3 on both server and
> client.
Hopefully the original reporter of the problem (Gergely ??) can test the patch as well.
I think the patch is pretty harmless, although it could be argued that setting
    np->m_mtime = np->n_nattr.na_mtime (or close to that)
shouldn't happen for the case where there isn't any dirty pages found to flush.
However, once a file mmap'd we don't know when it does get modified anyhow
(as discussed above), so setting it here doesn't seem harmful to me.

Thanks for testing the patch. Now, if others can test it...rick