vm_pageout_cluster() has several incorrect comments. This revision fixes those comments. In addition, vm_pageout_cluster() has some comments that are "out of context". In other words, while they might be true, they are out of place. This revision eliminates those comments. Finally, this revision fixes some style issues within vm_pageout_cluster().
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| vm/vm_pageout.c | ||
|---|---|---|
| 390 ↗ | (On Diff #19018) | In PQ_LAUNDRY, we only consider pages in the laundry queue. Should we also attempt to cluster pages from the inactive queue? |
| vm/vm_pageout.c | ||
|---|---|---|
| 455 ↗ | (On Diff #19018) | The word "page" in the previous line didn't make sense to me, but I was sure what it should really be. Should it be "alignment"? |
| vm/vm_pageout.c | ||
|---|---|---|
| 390 ↗ | (On Diff #19018) | I'm not sure. It might well depend on whether we are in "shortfall" or "background" laundering mode. In background mode, I would say, "Consider PQ_LAUNDRY only." The reason being that I want to give dirty pages longer to be re-referenced before we attempt to launder and reclaim them. Allowing a page "I" from PQ_INACTIVE to cluster with the page "L" from the head of PQ_LAUNDRY is accelerating the potential reclamation of "I". In other words, we're not giving "I" as much time to be re-referenced as we gave "L". Ultimately, not clustering with PQ_INACTIVE pages may lead to more disk writes, if we later launder "I" after it has migrated to the head of PQ_LAUNDRY, but my assumption is that in background mode we're not I/O bottlenecked. In contrast, in shortfall mode, we might care more about optimizing the number of disk writes, so allowing pages to be either PQ_LAUNDRY or PQ_INACTIVE could be better. Does this make sense? |
| vm/vm_pageout.c | ||
|---|---|---|
| 390 ↗ | (On Diff #19018) | Yes, thanks. Indeed, I'd be reticent about making the change I suggested above since it can cause pages to "bypass" LRU. This is something of a behaviour change with respect to HEAD though, since we can cluster any page in the inactive queue, even those with PG_WINATCFLS not set. In the shortfall case, recall that r292392 causes the pagedaemon to deactivate dirty pages more aggressively than clean pages, which improves the ability of the laundry thread to cluster pages from the PQ_LAUNDRY queue. So I'm inclined to think that no change is needed. |
| 455 ↗ | (On Diff #19018) | "Alignment" seems like a likely candidate to me based on nearby comments. |