Thanks for the review, Peter! I'll send a v2 shortly to address the suggestions. On Thu, Jun 3, 2021 at 12:29 PM Peter Xu <peterx@xxxxxxxxxx> wrote: > > On Thu, Jun 03, 2021 at 11:32:16AM -0700, Axel Rasmussen wrote: > > [...] > > Not a native speaker, feel free to take anything I said with a grain of salt.. > > > @@ -278,14 +287,8 @@ by the current kernel version. > > (Since Linux 4.3.) > > Register a memory address range with the userfaultfd object. > > The pages in the range must be "compatible". > > -.PP > > -Up to Linux kernel 4.11, > > -only private anonymous ranges are compatible for registering with > > -.BR UFFDIO_REGISTER . > > -.PP > > -Since Linux 4.11, > > -hugetlbfs and shared memory ranges are also compatible with > > -.BR UFFDIO_REGISTER . > > +What constitutes "compatible" depends on the mode(s) being used, as described > > +below. > > Would below be slightly better? > > Please refer to the list of register modes below for the compatible memory > backends for each mode. I have no significant preference between the two, so happy to reword this one. > > [...] > > > @@ -735,6 +745,109 @@ or not registered with userfaultfd write-protect mode. > > .TP > > .B EFAULT > > Encountered a generic fault during processing. > > +.\" > > +.SS UFFDIO_CONTINUE > > +(Since Linux 5.13.) > > +Used for resolving minor faults specifically. > > +Take the existing page(s) in the range registered with > > +.B UFFDIO_REGISTER_MODE_MINOR > > +and install page table entries for them. > > "Take the existing page" reads a bit weird to me. How about something like: > "Resolving minor-mode trapped page faults by installing page table entries with > pages in the page cache"? Agreed, "take" is a bit awkward. I'll reword to something close to your suggestion. > > [...] > > > +.TP > > +.B EINVAL > > +An invalid bit was specified in the > > +.IR mode > > +field. > > +.TP > > +.B EEXIST > > +One or more pages were already mapped in the given range. > > I'd think this sentence is good enough; slightly prefer dropping the latter one > "In other words..." below, as "mapped" should mean the same to me (and the > wording "fully mapped" is a bit confusing too..). Fair enough, I had it that way at first but was worried the first sentence alone was too vague. I'm probably overthinking it. ;) I'll just drop the second sentence. > > > +In other words, not only did pages exist in the page cache, but page table > > +entries already existed for those pages and they were fully mapped. > > [...] > > Thanks, > > -- > Peter Xu >
