On Wed, Feb 20, 2019 at 04:25:07PM -0800, John Hubbard wrote: > On 1/29/19 8:54 AM, jglisse@xxxxxxxxxx wrote: > > From: Jérôme Glisse <jglisse@xxxxxxxxxx> > > > > Rename for consistency between code, comments and documentation. Also > > improves the comments on all the possible returns values. Improve the > > function by returning the number of populated entries in pfns array. > > > > Signed-off-by: Jérôme Glisse <jglisse@xxxxxxxxxx> > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > Cc: Ralph Campbell <rcampbell@xxxxxxxxxx> > > Cc: John Hubbard <jhubbard@xxxxxxxxxx> > > --- > > include/linux/hmm.h | 4 ++-- > > mm/hmm.c | 23 ++++++++++------------- > > 2 files changed, 12 insertions(+), 15 deletions(-) > > > > Hi Jerome, > > After applying the entire patchset, I still see a few hits of the old name, > in Documentation: > > $ git grep -n hmm_vma_get_pfns > Documentation/vm/hmm.rst:192: int hmm_vma_get_pfns(struct vm_area_struct *vma, > Documentation/vm/hmm.rst:205:The first one (hmm_vma_get_pfns()) will only > fetch present CPU page table > Documentation/vm/hmm.rst:224: ret = hmm_vma_get_pfns(vma, &range, > start, end, pfns); > include/linux/hmm.h:145: * HMM pfn value returned by hmm_vma_get_pfns() or > hmm_vma_fault() will be: > > > > diff --git a/include/linux/hmm.h b/include/linux/hmm.h > > index bd6e058597a6..ddf49c1b1f5e 100644 > > --- a/include/linux/hmm.h > > +++ b/include/linux/hmm.h > > @@ -365,11 +365,11 @@ void hmm_mirror_unregister(struct hmm_mirror *mirror); > > * table invalidation serializes on it. > > * > > * YOU MUST CALL hmm_vma_range_done() ONCE AND ONLY ONCE EACH TIME YOU CALL > > - * hmm_vma_get_pfns() WITHOUT ERROR ! > > + * hmm_range_snapshot() WITHOUT ERROR ! > > * > > * IF YOU DO NOT FOLLOW THE ABOVE RULE THE SNAPSHOT CONTENT MIGHT BE INVALID ! > > */ > > -int hmm_vma_get_pfns(struct hmm_range *range); > > +long hmm_range_snapshot(struct hmm_range *range); > > bool hmm_vma_range_done(struct hmm_range *range); > > diff --git a/mm/hmm.c b/mm/hmm.c > > index 74d69812d6be..0d9ecd3337e5 100644 > > --- a/mm/hmm.c > > +++ b/mm/hmm.c > > @@ -706,23 +706,19 @@ static void hmm_pfns_special(struct hmm_range *range) > > } > > /* > > - * hmm_vma_get_pfns() - snapshot CPU page table for a range of virtual addresses > > - * @range: range being snapshotted > > + * hmm_range_snapshot() - snapshot CPU page table for a range > > + * @range: range > > * Returns: -EINVAL if invalid argument, -ENOMEM out of memory, -EPERM invalid > > Channeling Mike Rapoport, that should be @Return: instead of Returns: , but... > > > > - * vma permission, 0 success > > + * permission (for instance asking for write and range is read only), > > + * -EAGAIN if you need to retry, -EFAULT invalid (ie either no valid > > + * vma or it is illegal to access that range), number of valid pages > > + * in range->pfns[] (from range start address). > > ...actually, that's a little hard to spot that we're returning number of > valid pages. How about: > > * @Returns: number of valid pages in range->pfns[] (from range start > * address). This may be zero. If the return value is negative, > * then one of the following values may be returned: > * > * -EINVAL range->invalid is set, or range->start or range->end > * are not valid. > * -EPERM For example, asking for write, when the range is > * read-only > * -EAGAIN Caller needs to retry > * -EFAULT Either no valid vma exists for this range, or it is > * illegal to access the range > > (caution: my white space might be wrong with respect to tabs) Will do a documentation patch to improve things and remove leftover. Cheers, Jérôme
