Differential D26212

vm_pageout_scan_active: ensure ps_delta is initialized
ClosedPublic
Actions

Authored by vangyzen on Aug 27 2020, 3:08 PM.

Details

Reviewers

markj

Commits

rS364935: vm_pageout_scan_active: ensure ps_delta is initialized

Summary

Ensure ps_delta is initialized.

Reported by: Coverity

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 33194
Build 30541: arc lint + arc unit

Event Timeline

vangyzen created this revision.Aug 27 2020, 3:08 PM

Herald added subscribers: dab, badger. · View Herald TranscriptAug 27 2020, 3:08 PM

Harbormaster completed remote builds in B33194: Diff 76283.Aug 27 2020, 3:08 PM

vangyzen requested review of this revision.Aug 27 2020, 3:08 PM

markj added inline comments.Aug 27 2020, 3:36 PM

sys/vm/vm_pageout.c
1293	I guess this is the only case where we can end up with a bogus ps_delta. It should be quite rare. The assignment should go here, not at the top of the loop. We only break from the loop when the atomic_fcmpset at the end succeeds. In one iteration we might set ps_delta to a nonzero value, fail the update, and then exit the loop through this case, in which case ps_delta will be left equal to the old and now-incorrect value.

move init to the right place

Harbormaster completed remote builds in B33197: Diff 76286.Aug 27 2020, 4:15 PM

vangyzen marked an inline comment as done.Aug 27 2020, 4:15 PM

vangyzen added inline comments.

sys/vm/vm_pageout.c
1293	Indeed. Thank you.

markj accepted this revision.Aug 27 2020, 4:34 PM

This revision is now accepted and ready to land.Aug 27 2020, 4:34 PM

Closed by commit rS364935: vm_pageout_scan_active: ensure ps_delta is initialized (authored by vangyzen). · Explain WhyAug 28 2020, 7:59 PM

This revision was automatically updated to reflect the committed changes.

vangyzen marked an inline comment as done.

vangyzen added a commit: rS364935: vm_pageout_scan_active: ensure ps_delta is initialized.

Herald added a subscriber: imp. · View Herald TranscriptAug 28 2020, 7:59 PM

Revision Contents
Changeset List

Path

Size

sys/

vm/

vm_pageout.c

1 line

Diff 76283

View Options

sys/vm/vm_pageout.c

Show First 20 Lines • Show All 93 Lines • ▼ Show 20 Lines

object = vm_object_allocate_dyn(tmpfs_pager_type, size,

OBJ_COLORED | OBJ_SWAP);

if (!swap_pager_init_object(object, NULL, NULL, size, 0)) {

vm_object_deallocate(object);

object = NULL;

}

return (object);

}

* Make sure tmpfs vnodes with writable mappings can be found on the lazy list.

* This allows for periodic mtime updates while only scanning vnodes which are

* plausibly dirty, see tmpfs_update_mtime_lazy.

static void

tmpfs_pager_writecount_recalc(vm_object_t object, vm_offset_t old,

vm_offset_t new)

{

struct vnode *vp;

VM_OBJECT_ASSERT_WLOCKED(object);

vp = object->un_pager.swp.swp_tmpfs;

* Forced unmount?

if (vp == NULL) {

KASSERT((object->flags & OBJ_TMPFS_VREF) == 0,

("object %p with OBJ_TMPFS_VREF but without vnode", object));

VM_OBJECT_WUNLOCK(object);

return;

}

if (old == 0) {

VNASSERT((object->flags & OBJ_TMPFS_VREF) == 0, vp,

("object without writtable mappings has a reference"));

markjUnsubmitted

Not Done

VNASSERT((object->flags & OBJ_TMPFS_VREF) == 0, vp,

- ("object without writtable mappings has a reference"));

+ ("object without writable mappings has a reference"));

VNPASS(vp->v_usecount > 0, vp);

markj:

VNPASS(vp->v_usecount > 0, vp);

} else {

VNASSERT((object->flags & OBJ_TMPFS_VREF) != 0, vp,

("object with writtable mappings does not have a reference"));

markjUnsubmitted

Not Done

VNASSERT((object->flags & OBJ_TMPFS_VREF) != 0, vp,

- ("object with writtable mappings does not have a reference"));

+ ("object with writable mappings does not have a reference"));

}

if (old == new) {

markj:

mjgAuthorUnsubmitted

Done

fixed locally

mjg: fixed locally

}

if (old == new) {

VM_OBJECT_WUNLOCK(object);

return;

}

if (new == 0) {

vm_object_clear_flag(object, OBJ_TMPFS_VREF);

VM_OBJECT_WUNLOCK(object);

vrele(vp);

} else {

if ((object->flags & OBJ_TMPFS_VREF) == 0) {

vref(vp);

vlazy(vp);

vm_object_set_flag(object, OBJ_TMPFS_VREF);

}

VM_OBJECT_WUNLOCK(object);

}

static void

tmpfs_pager_update_writecount(vm_object_t object, vm_offset_t start,

vm_offset_t end)

{

vm_offset_t new, old;

VM_OBJECT_WLOCK(object);

KASSERT((object->flags & OBJ_ANON) == 0,

mjgAuthorUnsubmitted

Done

I don't know if this assert is of any use, cargo culted this bit from the original swap pager.

mjg: I don't know if this assert is of any use, cargo culted this bit from the original swap pager.

kibUnsubmitted

Not Done

It does make sense, but perhaps replace the word 'Splittable' with 'tmpfs vm'

kib: It does make sense, but perhaps replace the word 'Splittable' with 'tmpfs vm'

("Splittable object with writecount"));

old = object->un_pager.swp.writemappings;

object->un_pager.swp.writemappings += (vm_ooffset_t)end - start;

new = object->un_pager.swp.writemappings;

tmpfs_pager_writecount_recalc(object, old, new);

VM_OBJECT_ASSERT_UNLOCKED(object);

}

static void

tmpfs_pager_release_writecount(vm_object_t object, vm_offset_t start,

vm_offset_t end)

{

vm_offset_t new, old;

VM_OBJECT_WLOCK(object);

KASSERT((object->flags & OBJ_ANON) == 0,

("Splittable object with writecount"));

old = object->un_pager.swp.writemappings;

object->un_pager.swp.writemappings -= (vm_ooffset_t)end - start;

new = object->un_pager.swp.writemappings;

tmpfs_pager_writecount_recalc(object, old, new);

VM_OBJECT_ASSERT_UNLOCKED(object);

}

static void

tmpfs_pager_getvp(vm_object_t object, struct vnode **vpp, bool *vp_heldp)

{

struct vnode *vp;

* Tmpfs VREG node, which was reclaimed, has tmpfs_pager_type

* type, but not OBJ_TMPFS flag. In this case there is no

* v_writecount to adjust.

Show All 15 Lines

tmpfs_pager_getvp(vm_object_t object, struct vnode **vpp, bool *vp_heldp)

if (vp_heldp != NULL)

VM_OBJECT_RUNLOCK(object);

}

struct pagerops tmpfs_pager_ops = {

.pgo_kvme_type = KVME_TYPE_VNODE,

.pgo_alloc = tmpfs_pager_alloc,

.pgo_set_writeable_dirty = vm_object_set_writeable_dirty_,

.pgo_update_writecount = tmpfs_pager_update_writecount,

.pgo_release_writecount = tmpfs_pager_release_writecount,

.pgo_mightbedirty = vm_object_mightbedirty_,

.pgo_getvp = tmpfs_pager_getvp,

};

static int

tmpfs_node_ctor(void *mem, int size, void *arg, int flags)

{

struct tmpfs_node *node;

▲ Show 20 Lines • Show All 496 Lines • ▼ Show 20 Lines

tmpfs_free_dirent(struct tmpfs_mount *tmp, struct tmpfs_dirent *de)

if (!tmpfs_dirent_duphead(de) && de->ud.td_name != NULL)

free(de->ud.td_name, M_TMPFSNAME);

free(de, M_TMPFSDIR);

}

void

tmpfs_destroy_vobject(struct vnode *vp, vm_object_t obj)

{

bool want_vrele;

ASSERT_VOP_ELOCKED(vp, "tmpfs_destroy_vobject");

if (vp->v_type != VREG || obj == NULL)

return;

VM_OBJECT_WLOCK(obj);

VI_LOCK(vp);

* May be going through forced unmount.

want_vrele = false;

if ((obj->flags & OBJ_TMPFS_VREF) != 0) {

vm_object_clear_flag(obj, OBJ_TMPFS_VREF);

want_vrele = true;

}

kibUnsubmitted

Not Done

There should be a blank line after the last line of the text where the multi-line comment applies.

kib: There should be a blank line after the last line of the text where the multi-line comment…

vm_object_clear_flag(obj, OBJ_TMPFS);

obj->un_pager.swp.swp_tmpfs = NULL;

if (vp->v_writecount < 0)

vp->v_writecount = 0;

VI_UNLOCK(vp);

VM_OBJECT_WUNLOCK(obj);

if (want_vrele) {

vrele(vp);

}

* Need to clear v_object for insmntque failure.

static void

tmpfs_insmntque_dtr(struct vnode *vp, void *dtr_arg)

{

▲ Show 20 Lines • Show All 1,397 Lines • Show Last 20 Lines

vm_pageout_scan_active: ensure ps_delta is initializedClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 76283

sys/vm/vm_pageout.c

vm_pageout_scan_active: ensure ps_delta is initialized
ClosedPublic
Actions

Revision Contents
Changeset List