From owner-freebsd-current@freebsd.org Fri Mar 17 08:36:12 2017 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 32A36D0FC30 for ; Fri, 17 Mar 2017 08:36:12 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id CEF18128A; Fri, 17 Mar 2017 08:36:11 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id v2H8a64b015268 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 17 Mar 2017 10:36:06 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua v2H8a64b015268 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id v2H8a5Pf015267; Fri, 17 Mar 2017 10:36:05 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 17 Mar 2017 10:36:05 +0200 From: Konstantin Belousov To: Rick Macklem Cc: Dimitry Andric , Ian Lepore , Gergely Czuczy , FreeBSD Current Subject: Re: process killed: text file modification Message-ID: <20170317083605.GQ16105@kib.kiev.ua> References: <646c1395-9482-b214-118c-01573243ae5a@harmless.hu> <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.0 (2017-02-23) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Mar 2017 08:36:12 -0000 On Fri, Mar 17, 2017 at 03:10:57AM +0000, Rick Macklem wrote: > Hope you don't mind a top post... > Attached is a little patch you could test maybe? > > rick > ________________________________________ > From: owner-freebsd-current@freebsd.org on behalf of Rick Macklem > Sent: Thursday, March 16, 2017 9:57:23 PM > To: Dimitry Andric; Ian Lepore > Cc: Gergely Czuczy; FreeBSD Current > Subject: Re: process killed: text file modification > > Dimitry Andric wrote: > [lots of stuff snipped] > > I'm also running into this problem, but while using lld. I must set > > vfs.timestamp_precision to 1 (e.g. sec + ns accurate to 1/HZ) on both > > the client and the server, to make it work. > > > > Instead of GNU ld, lld uses mmap to write to the output executable. If > > this executable is more than one page, and resides on an NFS share, > > running it will almost always result in "text file modification", if > > vfs_timestamp_precision >= 2. > > > > A small test case: http://www.andric.com/freebsd/test-mmap-write.c, > > which writes a simple "hello world" i386-freebsd executable file, using > > the sequence: open() -> ftruncate() -> mmap() -> memcpy() -> munmap() -> > > close(). > Hopefully Kostik will correct me if I have this wrong, but I don't believe any > of the above syscalls guarantee that dirty pages have been flushed. > At least for cases without munmap(), the writes of dirty pages can occur after > the file descriptor is closed. I run into this in NFSv4, where there is a Close (NFSv4 one) > that can't be done until VOP_INACTIVE(). > If you look in the NFS VOP_INACTIVE() { called ncl_inactive() } you'll see: > if (NFS_ISV4(vp) && vp->v_type == VREG) { > 237 /* > 238 * Since mmap()'d files do I/O after VOP_CLOSE(), the NFSv4 > 239 * Close operations are delayed until now. Any dirty > 240 * buffers/pages must be flushed before the close, so that the > 241 * stateid is available for the writes. > 242 */ > 243 if (vp->v_object != NULL) { > 244 VM_OBJECT_WLOCK(vp->v_object); > 245 retv = vm_object_page_clean(vp->v_object, 0, 0, > 246 OBJPC_SYNC); > 247 VM_OBJECT_WUNLOCK(vp->v_object); > 248 } else > 249 retv = TRUE; > 250 if (retv == TRUE) { > 251 (void)ncl_flush(vp, MNT_WAIT, NULL, ap->a_td, 1, 0); > 252 (void)nfsrpc_close(vp, 1, ap->a_td); > 253 } > 254 } > Note that nothing like this is done for NFSv3. > What might work is implementing a VOP_SET_TEXT() vnode op for the NFS > client that does most of the above (except for nfsrpc_close()) and then sets > VV_TEXT. > --> That way, all the dirty pages will be flushed to the server before the executable > starts executing. > > > Running this on an NFS share, and then attempting to run the resulting > > 'helloworld' executable will result in the "text file modification" > > error, and it will be killed. But if you simply copy the executable to > > something else, then it runs fine, even if you use -p to retain the > > properties! > > > > IMHO this is a rather surprising problem with the NFS code, and Kostik > > remarked that the problem seems to be that the VV_TEXT flag is set too > > early, before the nfs cache is invalidated. Rick, do you have any ideas > > about this? > I don't think it is the "nfs cache" that needs invalidation, but the dirty > pages written by the mmap'd file need to be flushed, before the VV_TEXT > is set, I think? > > If Kostik meant "attribute cache" when he said "nfs cache", I'll note that the > cached attributes (including mtime) are updated by the reply to every write. > (As such, I think it is the writes that must be done before setting VV_TEXT > that is needed.) > > It is a fairly simple patch to create. I'll post one to this thread in a day or so > unless Kostik thinks this isn't correct and not worth trying. > I think that the patch is in right direction, but I am not convinced that it is enough. What we need is to ensure that mtime on server does not change after VV_TEXT is set. Dirty pages indeed would cause the mtime update on flush, but wouldn't dirty buffers writes cause the same issue ? In other words, I think that enchanced VOP_SET_TEXT() for nfs must flush everything to ensure that mtime on server would not change due to further actions by this machine' nfs client.