Eliminate every mention of PG_CACHED pages from the comments under vm/

[alc]

Summary

Test Plan

I took some liberties revising the nearby text of some comments in vm_page.{c,h}, e.g., vm_page_advise(). Please make sure that you're ok with the revised text.

[kib]

There seems to be not many other references to cache that mean the eliminated PG_CACHE facility in sys/vm, I only see something in vm_object.h and vm_reserv.c.

[alc]

In D8752#181330, @kib wrote:

There seems to be not many other references to cache that mean the eliminated PG_CACHE facility in sys/vm, I only see something in vm_object.h and vm_reserv.c.

You inspired me to find them all. :-)

[alc]

I've also corrected the misuse of hyphens after words ending in -ly in vm_reserv.c, because there were several of these misuses that were adjacent to a mention of PG_CACHED pages.

Finally, I've updated the comment describing the four types of page queues.

[alc]

I couldn't decide whether the comments at the head of default_pager_dealloc() and default_pager_putpages() were referring to PG_CACHED pages. Also, I'm not sure that part of the comment describing the latter is factually correct.

[markj]

In D8752#181427, @alc wrote:

I couldn't decide whether the comments at the head of default_pager_dealloc() and default_pager_putpages() were referring to PG_CACHED pages. Also, I'm not sure that part of the comment describing the latter is factually correct.

Reading some of the code in sys/vm@r42957, it looks like "cache list" refers to the PQ_CACHE lists removed in r172317.

I think the comment above default_pager_putpages() has always been bogus: before that revision, the OBJT_DEFAULT->OBJT_SWAP conversion was done in default_pager_putpages() or in default_pager_convert_to_swapq(). The latter was invoked in a few different circumstances from vm_map code; after that revision, it's only done at the beginning of swap_pager_putpages(). However, I'm not clear on which cache->free transition that comment might have been referring to.

vm/vm_reserv.c

951 ↗

(On Diff #22826)

Wasn't "satisfy" more correct? The request specifies constraints on (a run of) pages, not on the reservation itself.

[alc]

vm/vm_reserv.c

951 ↗

(On Diff #22826)

Yes, it was.

[alc]

Revert "satisfies" to "satisfy".

[kib]

vm/vm_object.h

84 ↗

(On Diff #22835)

(f) annotation explanation can be removed from the list, it is on the next line after the patch context.

vm/vm_reserv.c

199 ↗

(On Diff #22835)

Finish the defining sentence with the dot ?

[kib]

In D8752#181444, @markj wrote:

In D8752#181427, @alc wrote:

I couldn't decide whether the comments at the head of default_pager_dealloc() and default_pager_putpages() were referring to PG_CACHED pages. Also, I'm not sure that part of the comment describing the latter is factually correct.

Reading some of the code in sys/vm@r42957, it looks like "cache list" refers to the PQ_CACHE lists removed in r172317.

I think the comment above default_pager_putpages() has always been bogus: before that revision, the OBJT_DEFAULT->OBJT_SWAP conversion was done in default_pager_putpages() or in default_pager_convert_to_swapq(). The latter was invoked in a few different circumstances from vm_map code; after that revision, it's only done at the beginning of swap_pager_putpages(). However, I'm not clear on which cache->free transition that comment might have been referring to.

If the comment is confusing even to you, I do not expect the comment giving any goods to the random reader who is not acquainted with the VM code. I think it is more useful to remove that parts then to try to find some bits of meaning.

[alc]

vm/vm_object.h

84 ↗

(On Diff #22835)

Access to the object's reservation list is synchronized by the free page queues lock. I'll add an item to my TODO list to add the lock annotations to all of the fields. Since that could be MFCed immediately, I'll keep it separate from this change.

[alc]

vm/vm_reserv.c

199 ↗

(On Diff #22835)

I'm uncomfortable with adding a period here because it's not really a sentence without a verb.

[alc]

In D8752#181503, @kib wrote:

In D8752#181444, @markj wrote:

In D8752#181427, @alc wrote:

I couldn't decide whether the comments at the head of default_pager_dealloc() and default_pager_putpages() were referring to PG_CACHED pages. Also, I'm not sure that part of the comment describing the latter is factually correct.

Reading some of the code in sys/vm@r42957, it looks like "cache list" refers to the PQ_CACHE lists removed in r172317.

I think the comment above default_pager_putpages() has always been bogus: before that revision, the OBJT_DEFAULT->OBJT_SWAP conversion was done in default_pager_putpages() or in default_pager_convert_to_swapq(). The latter was invoked in a few different circumstances from vm_map code; after that revision, it's only done at the beginning of swap_pager_putpages(). However, I'm not clear on which cache->free transition that comment might have been referring to.

If the comment is confusing even to you, I do not expect the comment giving any goods to the random reader who is not acquainted with the VM code. I think it is more useful to remove that parts then to try to find some bits of meaning.

I'll make another note to deal with this separately, since it will also immediately apply to older branches.