Mostly, attempt to catch up to r292373.
Other general tidying while in the area.
Details
Details
- Reviewers
glebius kib - Group Reviewers
manpages - Commits
- rS317910: Bring VOP_GETPAGES.9 more up-to-date
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| share/man/man9/VOP_GETPAGES.9 | ||
|---|---|---|
| 46 ↗ | (On Diff #27960) | Change both declarations to .Fo, to avoid too long lines, perhaps. "sync" is really flags. This is not consistently applied to whole sources, e.g. vnode_if.src still calls the argument sync, but since you change the line, it is reasonable to change the argument name. |
| 73 ↗ | (On Diff #27960) | I believe it is simpler to describe 'count' as ma array size, then note somewhere that all pages in array must be validated on success. |
| 76 ↗ | (On Diff #27960) | This is flags, and it probably requires introduction as such. Also, there are more flags which can be specified, see the corresponding block in vm/vm_pager.h. |
| 140 ↗ | (On Diff #27960) | 'must keep' is somewhat misleading. VOP_GETPAGES() is entered with pages in ma being exclusively busy, and must return in the same state. But pagers are free to unbusy the pages as needed in process, and current pager used for buffer cache takes advantage of that. More, this might cause replacement of the page in ma array if page is reclaimed while such pager drops busy state. |
| 145 ↗ | (On Diff #27960) | This is really not true, an probably even unhealthy. Handling activation of the cleaned pages is the combined duty of the PUTPAGES() caller, which is vm_pageout_flush(), and other filesystem mechanisms. E.g. for UFS the written page must participate in the buffer, which imposes the page state as wired and thus unqueued. On reclamation of the buffer, page is unwired and deactivated. In other words, there is no requirement that activation is handled in VOP_PUTPAGES(), it is driven by the filesystem code structure. |
| share/man/man9/VOP_GETPAGES.9 | ||
|---|---|---|
| 94 ↗ | (On Diff #27998) | ... be synchronous, the control is returned to the caller after the write is finished. Because synchronous means different things. |
| 100 ↗ | (On Diff #27998) | Set IO_NOREUSE io flag, to Indicate to filesystem that pages should be marked for fast reuse if needed, e.g. by .Xr vm_page_deactivate_noreuse(), which put the pages into the head of inactive queue. |
| 104 ↗ | (On Diff #27998) | so that related writes can be coalesced for efficiency, e.g. using the clustering mechanism of buffer cache. |
| 76 ↗ | (On Diff #27960) | vm_pager.h lists flags. |
| 145 ↗ | (On Diff #27960) | It is useful to make a note that the page should be put back into a queue when appropriate, to not create a leak of non-pageable pages. The thing which I did not liked in the original and changed formulations is that it stated that page might be put on queue in the VOP method (why ? why should it be noted at all ? there are many things which can be done with pages). Typically filesystems handle the queueing at proper times according to their code structure. |
| share/man/man9/VOP_GETPAGES.9 | ||
|---|---|---|
| 94 ↗ | (On Diff #27998) | Indeed, that is a good distinction to make. |
Comment Actions
Mostly took kib's updates as-is, though with some changes. In particular, using vm_vm_page_deactivate_noreuse() as an example made the sentence too long, so I split it up.
I also added a bland note that pages not being processed should be returned to an appropriate page queue.
| share/man/man9/VOP_GETPAGES.9 | ||
|---|---|---|
| 181 ↗ | (On Diff #28045) | This sentence is even more confusing the all previous variants. "It is filesystem duty to activate the paged-out pages, but not necessary during the VOP_PUTPAGES call". |