Date: Fri, 17 Mar 2017 21:23:00 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: Dimitry Andric <dim@FreeBSD.org>, Konstantin Belousov <kostikbel@gmail.com> Cc: Ian Lepore <ian@FreeBSD.org>, Gergely Czuczy <gergely.czuczy@harmless.hu>, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: process killed: text file modification Message-ID: <YTXPR01MB0189FBB6CF664653C1162936DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <D0770019-3EEA-45D2-A751-18DF1B274F90@FreeBSD.org> References: <45436522-77df-f894-0569-737a6a74958f@harmless.hu> <55189643.aaZPuY77s8@ralph.baldwin.cx> <3ed3e4a3-23af-7267-39f1-9090093c9c1e@harmless.hu> <5ac94b9a-7ced-9eff-d746-7dddaaeca516@harmless.hu> <1489340839.40576.82.camel@freebsd.org> <FF55DB37-4A6B-4D88-B201-B3BCA1A11E87@FreeBSD.org> <YTXPR01MB01898D49933A82170FCAB7A0DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <YTXPR01MB018944EF4248402AD421D825DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170317083605.GQ16105@kib.kiev.ua> <YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170317141917.GS16105@kib.kiev.ua>, <D0770019-3EEA-45D2-A751-18DF1B274F90@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Dimitry Andric wrote:
>On 17 Mar 2017, at 15:19, Konstantin Belousov <kostikbel@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 an=
d
>> 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 pos=
sible
>> to get them by other means.
To write to a buffer cache block, the file would be opened by another threa=
d and
that is what this sanity check was meant to catch. As for dirtying pages th=
at are mmap'd,
as far as I understand it, the NFS client has no way of knowing if this wil=
l 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, th=
e 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=3D3 on both server and
> client.
Hopefully the original reporter of the problem (Gergely ??) can test the pa=
tch as well.
I think the patch is pretty harmless, although it could be argued that sett=
ing
np->m_mtime =3D np->n_nattr.na_mtime (or close to that)
shouldn't happen for the case where there isn't any dirty pages found to fl=
ush.
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YTXPR01MB0189FBB6CF664653C1162936DD390>