On 10/24/23 2:52 AM, Alejandro Colomar wrote: > Hi Muhammad, > > [CC += Branden] > > On Thu, Oct 19, 2023 at 06:12:45PM +0500, Muhammad Usama Anjum wrote: >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@xxxxxxxxxxxxx> >> --- >> The feature has been added to mm-stable: >> https://lore.kernel.org/all/20231018213453.BF1ACC43395@xxxxxxxxxxxxxxx >> >> Changes since v1: >> - Several formatting updates >> - Added some additional sentences > > Wow, the formatting is very well done. Great job! Patch applied. Alex, thank you so much for explaining stuff in previous revision and in this patch. I didn't know that comma before and and or is called Oxford comma. > See a few small comments below. > >> --- >> man2/ioctl_pagemap_scan.2 | 203 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 203 insertions(+) >> create mode 100644 man2/ioctl_pagemap_scan.2 >> >> diff --git a/man2/ioctl_pagemap_scan.2 b/man2/ioctl_pagemap_scan.2 >> new file mode 100644 >> index 000000000..c257072d7 >> --- /dev/null >> +++ b/man2/ioctl_pagemap_scan.2 >> @@ -0,0 +1,203 @@ >> +.\" This manpage is Copyright (C) 2023 Collabora; >> +.\" Written by Muhammad Usama Anjum <usama.anjum@xxxxxxxxxxxxx> >> +.\" >> +.\" SPDX-License-Identifier: Linux-man-pages-copyleft >> +.\" >> +.TH ioctl_pagemap_scan 2 2023-10-17 "Linux man-pages (unreleased)" >> +.SH NAME >> +ioctl_pagemap_scan \- get and/or clear page flags >> +.SH LIBRARY >> +Standard C library >> +.RI ( libc ", " \-lc ) >> +.SH SYNOPSIS >> +.nf >> +.BR "#include <linux/fs.h>" " /* Definition of " struct " " pm_scan_arg ", " >> +.BR " struct page_region "and " PAGE_IS_* "constants " */" > > That space is not necessary after a closing '"' is something I don't > know why exists. I changed that slightly. > > Also, we use the Oxford comma (a comma right before 'and' and 'or'. > > -.BR "#include <linux/fs.h>" " /* Definition of " struct " " pm_scan_arg ", " > -.BR " struct page_region "and " PAGE_IS_* "constants " */" > +.BR "#include <linux/fs.h>" " /* Definition of " "struct pm_scan_arg" , > +.BR " struct page_region" ", and " PAGE_IS_* " constants */" > >> +.B #include <sys/ioctl.h> >> +.PP >> +.BI "int ioctl(int " pagemap_fd ", PAGEMAP_SCAN, struct pm_scan_arg *" arg ); >> +.fi >> +.SH DESCRIPTION >> +This >> +.BR ioctl (2) >> +is used to get and optionally clear some specific flags from page table entries. >> +The information is returned with >> +.B PAGE_SIZE >> +granularity. >> +.PP >> +To start tracking the written state (flag) of a page or range of memory, >> +the >> +.B UFFD_FEATURE_WP_ASYNC >> +must be enabled by >> +.B UFFDIO_API >> +.BR ioctl (2) >> +on >> +.B userfaultfd >> +and memory range must be registered with >> +.B UFFDIO_REGISTER >> +.BR ioctl (2) >> +in >> +.B UFFDIO_REGISTER_MODE_WP >> +mode. >> +.SS Supported page flags >> +The following page table entry flags are supported: >> +.TP >> +.B PAGE_IS_WPALLOWED >> +The page has asynchronous write-protection enabled. >> +.TP >> +.B PAGE_IS_WRITTEN >> +The page has been written to from the time it was write protected. >> +.TP >> +.B PAGE_IS_FILE >> +The page is file backed. >> +.TP >> +.B PAGE_IS_PRESENT >> +The page is present in the memory. >> +.TP >> +.B PAGE_IS_SWAPPED >> +The page is swapped. >> +.TP >> +.B PAGE_IS_PFNZERO >> +The page has zero PFN. >> +.TP >> +.B PAGE_IS_HUGE >> +The page is THP or Hugetlb backed. >> +.SS Supported operations >> +The get operation is always performed >> +if the output buffer is specified. >> +The other operations are as following: >> +.TP >> +.B PM_SCAN_WP_MATCHING >> +Write protect the matched pages. >> +.TP >> +.B PM_SCAN_CHECK_WPASYNC >> +Abort the scan >> +when a page is found >> +which doesn't have the Userfaultfd Asynchronous Write protection enabled. >> +.SS The \f[I]struct pm_scan_arg\f[] argument >> +.EX >> +struct pm_scan_arg { >> + __u64 size; >> + __u64 flags; >> + __u64 start; >> + __u64 end; >> + __u64 walk_end; >> + __u64 vec; >> + __u64 vec_len; >> + __u64 max_pages >> + __u64 category_inverted; >> + __u64 category_mask; >> + __u64 category_anyof_mask >> + __u64 return_mask; > > I prefer two spaces between the type and the name. I got that habit > from nginx. > <https://nginx.org/en/docs/dev/development_guide.html#code_style> > > struct pm_scan_arg { > - __u64 size; > - __u64 flags; > - __u64 start; > - __u64 end; > - __u64 walk_end; > - __u64 vec; > - __u64 vec_len; > - __u64 max_pages > - __u64 category_inverted; > - __u64 category_mask; > - __u64 category_anyof_mask > - __u64 return_mask; > + __u64 size; > + __u64 flags; > + __u64 start; > + __u64 end; > + __u64 walk_end; > + __u64 vec; > + __u64 vec_len; > + __u64 max_pages > + __u64 category_inverted; > + __u64 category_mask; > + __u64 category_anyof_mask > + __u64 return_mask; > }; > > >> +}; >> +.EE >> +.TP >> +.B size >> +This field should be set to the size of the structure in bytes, >> +as in >> +.IR "sizeof(struct pm_scan_arg)" . > > We try to use \~ for a fillable space; it has the nice effect of > removing the quotes. > > -.IR "sizeof(struct pm_scan_arg)" . > +.IR sizeof(struct\~pm_scan_arg) . > >> +.TP >> +.B flags >> +The operations to be performed are specified in it. >> +.TP >> +.B start >> +The starting address of the scan is specified in it. >> +.TP >> +.B end >> +The ending address of the scan is specified in it. >> +.TP >> +.B walk_end >> +The kernel returns the scan's ending address in it. >> +The >> +.I walk_end >> +equal to >> +.I end >> +means that scan has completed on the entire range. >> +.TP >> +.B vec >> +The address of >> +.I page_region >> +array for output. >> +.PP >> +.in +8n > > Ahh, this is great, because I needed to explain to groff(1) maintainers > what is the problem with TP that I was complaining about in another > thread. > > Branden, here's what I mean. If you're new to man(7), it is rather > unintuitive what to do here. > > Muhammad, in this project, we usually use IP to continuate a TP. PP > would break the indentation back to before the TP, which is why you > needed so much in 'in'. > > Another solution, which we're discussing is wrapping everything is RS/RE. > > I applied this: > > -.PP > -.in +8n > +.IP > +.in +4n > > >> +.EX >> +struct page_region { >> + __u64 start; >> + __u64 end; >> + __u64 categories; >> +}; >> +.EE >> +.in >> +.TP >> +.B vec_len >> +The length of the >> +.I page_region >> +struct array. >> +.TP >> +.B max_pages >> +It is the optional limit for the number of output pages required. >> +.TP >> +.B category_inverted >> +.BI PAGE_IS_ * >> +categories which values match if 0 instead of 1. >> +.TP >> +.B category_mask >> +Skip pages for which any >> +.BI PAGE_IS_ * >> +category doesn't match. >> +.TP >> +.B category_anyof_mask >> +Skip pages for which no >> +.BI PAGE_IS_ * >> +category matches. >> +.TP >> +.B return_mask >> +.BI PAGE_IS_ * >> +categories that are to be reported in >> +.IR page_region . >> +.SH RETURN VALUE >> +On error, \-1 is returned, and >> +.I errno >> +is set to indicate the error. >> +.SH ERRORS >> +Error codes can be one of, but are not limited to, the following: >> +.TP >> +.B EINVAL >> +Invalid arguments i.e., invalid >> +.I size >> +of the argument, invalid >> +.IR flags , >> +invalid >> +.IR categories , >> +the >> +.I start >> +address isn't aligned with >> +.B PAGE_SIZE >> +or >> +.I vec_len >> +is specified when >> +.I vec >> +is >> +.BR NULL . >> +.TP >> +.B EFAULT >> +Invalid >> +.I arg >> +pointer, invalid >> +.I vec >> +pointer or invalid address range specified by >> +.I start >> +and >> +.IR end . >> +.TP >> +.B ENOMEM >> +No memory is available. >> +.TP >> +.B EINTR >> +Fetal signal is pending. > > And a bit more of semantic newlines: > > @@ -163,29 +163,32 @@ .SH ERRORS > Error codes can be one of, but are not limited to, the following: > .TP > .B EINVAL > -Invalid arguments i.e., invalid > +Invalid arguments i.e., > +invalid > .I size > -of the argument, invalid > +of the argument, > +invalid > .IR flags , > invalid > .IR categories , > the > .I start > address isn't aligned with > -.B PAGE_SIZE > +.BR PAGE_SIZE , > or > .I vec_len > is specified when > .I vec > -is > -.BR NULL . > +is NULL. > .TP > .B EFAULT > Invalid > .I arg > -pointer, invalid > +pointer, > +invalid > .I vec > -pointer or invalid address range specified by > +pointer, > +or invalid address range specified by > .I start > and > .IR end . > > > Cheers, > Alex > >> +.SH STANDARDS >> +Linux. >> +.SH HISTORY >> +Linux 6.7. >> +.SH SEE ALSO >> +.BR ioctl (2) >> -- >> 2.42.0 >> > -- BR, Muhammad Usama Anjum
