svn commit: r232071 - head/sys/vm
Rodney W. Grimes
freebsd at gndrsh.dnsmgr.net
Mon May 27 14:21:33 UTC 2019
> 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 at freebsd.org
More information about the svn-src-head
mailing list