From owner-svn-src-head@freebsd.org Mon May 27 14:21:33 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 769241597DB4; Mon, 27 May 2019 14:21:33 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id F037E6ED3D; Mon, 27 May 2019 14:21:32 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (localhost [127.0.0.1]) by gndrsh.dnsmgr.net (8.13.3/8.13.3) with ESMTP id x4RELOjI011065; Mon, 27 May 2019 07:21:24 -0700 (PDT) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: (from freebsd@localhost) by gndrsh.dnsmgr.net (8.13.3/8.13.3/Submit) id x4RELOfH011064; Mon, 27 May 2019 07:21:24 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201905271421.x4RELOfH011064@gndrsh.dnsmgr.net> Subject: Re: svn commit: r232071 - head/sys/vm In-Reply-To: <20190527112155.GZ2748@kib.kiev.ua> To: Konstantin Belousov Date: Mon, 27 May 2019 07:21:24 -0700 (PDT) CC: Alexey Dokuchaev , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: F037E6ED3D X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.98 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.980,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 May 2019 14:21:33 -0000 > On Mon, May 27, 2019 at 09:52:56AM +0000, Alexey Dokuchaev wrote: > > On Thu, Feb 23, 2012 at 09:07:16PM +0000, Konstantin Belousov wrote: > > > New Revision: 232071 > > > URL: http://svn.freebsd.org/changeset/base/232071 > > > > > > Log: > > > Account the writeable shared mappings backed by file in the vnode > > > v_writecount. Keep the amount of the virtual address space used by > > > the mappings in the new vm_object un_pager.vnp.writemappings > > > counter. The vnode v_writecount is incremented when writemappings gets > > > non-zero value, and decremented when writemappings is returned to > > > zero. > > > > > > Writeable shared vnode-backed mappings are accounted for in vm_mmap(), > > > and vm_map_insert() is instructed to set MAP_ENTRY_VN_WRITECNT flag on > > > the created map entry. During deferred map entry deallocation, > > > vm_map_process_deferred() checks for MAP_ENTRY_VN_WRITECOUNT and > > > decrements writemappings for the vm object. > > > > > > Now, the writeable mount cannot be demoted to read-only while > > > writeable shared mappings of the vnodes from the mount point > > > exist. Also, execve(2) fails for such files with ETXTBUSY, as it > > > should be. > > > > > > ... > > > Modified: head/sys/vm/vnode_pager.c > > > ============================================================================== > > > --- head/sys/vm/vnode_pager.c Thu Feb 23 20:58:52 2012 (r232070) > > > +++ head/sys/vm/vnode_pager.c Thu Feb 23 21:07:16 2012 (r232071) > > > @@ -1215,3 +1222,81 @@ vnode_pager_undirty_pages(vm_page_t *ma, > > > } > > > VM_OBJECT_UNLOCK(obj); > > > } > > > + > > > +void > > > +vnode_pager_update_writecount(vm_object_t object, vm_offset_t start, > > > + vm_offset_t end) > > > > So, it is first `start, then `end', but below... > > > > > +{ > > > + struct vnode *vp; > > > + vm_ooffset_t old_wm; > > > + > > > + ... > > > +} > > > + > > > +void > > > +vnode_pager_release_writecount(vm_object_t object, vm_offset_t start, > > > + vm_offset_t end) > > > +{ > > > + struct vnode *vp; > > > + struct mount *mp; > > > + vm_offset_t inc; > > > + int vfslocked; > > > + > > > + VM_OBJECT_LOCK(object); > > > + > > > + /* > > > + * First, recheck the object type to account for the race when > > > + * the vnode is reclaimed. > > > + */ > > > + if (object->type != OBJT_VNODE) { > > > + VM_OBJECT_UNLOCK(object); > > > + return; > > > + } > > > + > > > + /* > > > + * Optimize for the case when writemappings is not going to > > > + * zero. > > > + */ > > > + inc = end - start; > > > + if (object->un_pager.vnp.writemappings != inc) { > > > + object->un_pager.vnp.writemappings -= inc; > > > + VM_OBJECT_UNLOCK(object); > > > + return; > > > + } > > > + > > > + vp = object->handle; > > > + vhold(vp); > > > + VM_OBJECT_UNLOCK(object); > > > + vfslocked = VFS_LOCK_GIANT(vp->v_mount); > > > + mp = NULL; > > > + vn_start_write(vp, &mp, V_WAIT); > > > + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); > > > + > > > + /* > > > + * Decrement the object's writemappings, by swapping the start > > > + * and end arguments for vnode_pager_update_writecount(). If > > > + * there was not a race with vnode reclaimation, then the > > > + * vnode's v_writecount is decremented. > > > + */ > > > + vnode_pager_update_writecount(object, end, start); > > > > ... here, first `end' is passed, then `start'. Is this intentional? > Did you read the comment right before the call ? I wish to assert again that all changes based on static analysis tools require a formal code review by at minimum: a) An expert in static analysis tools or a group of people selected who we consider language experts. b) An area expert from the area that is being affected We as a project look bad by not having this minimal set of code reviews in place for things that are coming from a source that is frought with introducing bad change. Regards, Rod > > PVS Studio complains: > > > > /usr/src/sys/vm/vnode_pager.c:1584:1: warning: V764 Possible > > incorrect order of arguments passed to 'vnode_pager_update_writecount' > > function: 'end' and 'start'. > > > > > + VOP_UNLOCK(vp, 0); > > > + vdrop(vp); > > > + if (mp != NULL) > > > + vn_finished_write(mp); > > > + VFS_UNLOCK_GIANT(vfslocked); > > > +} > > -- Rod Grimes rgrimes@freebsd.org