svn commit: r232071 - head/sys/vm

[Konstantin Belousov]

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 ?

> 
> 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);
> > +}