Add a constant OBJ_ANONYMOUS flag to optimize out some locking.

[jeff]

Summary

The intent is to make it easier to check for conditions where we want to maintain an accurate ONEMAPPING and do splits and coalescing. There are now more callsites that create non-anonymous mappings than there are anonymous. This head lead to a buggy proliferation of code that likely wants to set NOSPLIT but does not and which would probably benefit from OBJ_COLOR but does not set it. I changed the default flags for swap and default objects to be those suitable for non-anonymous mappings. I added a new function, vm_object_anonymous(), that is used for the few cases that do want anonymous mappings. I also could've eliminated NOSPLIT and replaced those with checks for ANONYMOUS but I have left it for now.

I am a little concerned about OBJT_DEAD vs OBJ_DEAD. It may be possible that the exact behavior with destruction races will change but I don't believe any of the code I changed yet is affected. It is just another complexity to evaluate if we convert more (type == default || type == swap) checks.

pho is testing this along with my other recent object locking work.

[jeff]

sys/vm/vm_object.c

2047

This line is wrong. I will fix and update later.

[markj]

sys/vm/vm_object.c

266

Doesn't this break the colouring mechanism? vm_object_color() does nothing if OBJ_COLORED is already set. So, this change makes it impossible to set pg_color to a non-zero value in the fault path.

460

Extra newline.

552

This looks like it's part of a future diff.

sys/vm/vm_object.h

190

Why not OBJ_ANON?

343

Extra space after the function name.

[alc]

sys/vm/vm_object.c

266

No, because vm_object_anonymous() clears the flag. That said, there is a related problem. To give an extreme example, when tmpfs creates a 4KB file, we are now going to allocate a reservation just for that one 4KB, and pretty soon we will run out of free 2MB chunks.

sys/vm/vm_object.h

190

In an unusual move for me, I would also argue for the shorter name. :-)

343

For consistency with the general approach to naming these functions, I would prefer vm_object_allocate_anon().

[jeff]

sys/vm/vm_object.c

266

We don't consult the object size?

Does this mean that every consumer needs to set color only when the object becomes large enough?

552

The refcount was approved but not yet committed.

sys/vm/vm_object.h

343

Ok I will happily adjust flag names.

Do you have comments on keeping/remove OBJ_NOSPLIT? It should become !ANON

[alc]

If we are going to start setting a color by default on DEFAULT and SWAP objects, then the following code that appears in two places in vm_reserv.c needs to change. Specifically, it should test for the absence of the OBJ_ANON rather than the object being OBJT_VNODE.

/*                                                                                                                                                              
 * Would the last new reservation extend past the end of the object?                                                                                            
 */
if (first + maxpages > object->size) {
        /*                                                                                                                                                      
         * Don't allocate the last new reservation if the object is a                                                                                           
         * vnode or backed by another object that is a vnode.                                                                                                   
         */
        if (object->type == OBJT_VNODE ||
            (object->backing_object != NULL &&
            object->backing_object->type == OBJT_VNODE)) {
                if (maxpages == VM_LEVEL_0_NPAGES)
                        return (NULL);
                allocpages = minpages;
        }
        /* Speculate that the object may grow. */
}

[jeff]

It is really your call Alan. I was trying to unify the flags among the various non-anonymous swap object users. We also saw with tmpfs that it performs much better with reservations enabled. I can defer this part of the patch or I can fix the above code.

[alc]

sys/vm/vm_object.c

266

See my followup message. I think that it answers your question.

sys/vm/vm_object.h

343

I would gladly see OBJ_NOSPLIT eliminated. Stylistically, I think that the flag OBJ_ANON is better because it explains *why* it is okay to split.

[jeff]

Handle more cases. Fix reservations limit. Other feedback.

[dougm]

sys/vm/vm_map.c

3491

No need for goto.

if ((entry->eflags & MAP_ENTRY_IS_SUB_MAP) != 0 || object == NULL)

entry->object.vm_object = NULL;

else if ((object->flags & OBJ_ANON) != 0 || object == kernel_object) {
...
}

[jeff]

Are there any further comments? This passed pho testing as well as some other ad-hoc workload testing. I would like to commit soon. I have a number of smaller object lock contention reduction patches I would like to move forward with.

[alc]

sys/vm/vm_object.c

473–474

Because of the global object list and manipulation of the flags by code that iterates over that list, this is not safe.

Perhaps, a cleaner and safer approach would be to introduce a vm_object_allocate_flags() and move the switch state that computes the default flags to vm_object_allocate().

[dougm]

sys/vm/vm_map.c

3491

You may have failed to observe this comment.

[jeff]

sys/vm/vm_map.c

3491

I saw thanks. I will fix these two comments and post an updated patch.

[jeff]

Address both pieces of review feedback.

[markj]

sys/vm/vm_object.c

453

I think the cast is not needed.

552

I just mean that I haven't seen a patch that changes vm_object so that ref_count is manipulated using atomics.

sys/vm/vm_object.h

188

Shouldn't it be "is backed by"?

[kib]

sys/vm/vm_map.c

1508

BTW, I do not think that we can observe ref_count == 1 and then ref_count > 1 there.

sys/vm/vm_object.c

449

It might make sense to add charge and ucred args. I might look at this as a followup.

sys/vm/vm_object.h

188

I think both 'backs' and 'backed' are not right there. OBJ_ANON object contains anonymous memory, backed usually refers to the shadow chain.

[jeff]

sys/vm/vm_object.c

449

I would like to see more encapsulating of that functionality. As is it is handled directly everywhere with near identical copies of code.

sys/vm/vm_object.h

188

I will update the comment before I commit.