[PATCH v2] mm/swap.c: make functions and their kernel-doc agree

[PATCH v2] mm/swap.c: make functions and their kernel-doc agree

[Randy Dunlap]

From: Randy Dunlap <rdunlap@infradead.org>

Fix some basic kernel-doc notation in mm/swap.c:
- for function lru_cache_add_anon(), make its kernel-doc function name
  match its function name and change colon to hyphen following the
  function name
- for function pagevec_lookup_entries(), change the function parameter
  name from nr_pages to nr_entries since that is more descriptive of
  what the parameter actually is and then it matches the kernel-doc
  comments also

Fix function kernel-doc to match the change in commit 67fd707f4681:
- drop the kernel-doc notation for @nr_pages from pagevec_lookup_range()
  and correct the function description for that change

Fixes: 67fd707f4681
    ("mm: remove nr_pages argument from pagevec_lookup_{,range}_tag()")

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Matthew Wilcox <willy@infradead.org>
---
 mm/swap.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

v2 changes:
- leave kernel-doc for lru_cache_add_anon() where it is but fix its
  kernel-doc notation
- change pagevec_lookup_entries() parameter from nr_pages to nr_entries

--- lnx-415.orig/mm/swap.c
+++ lnx-415/mm/swap.c
@@ -411,7 +411,7 @@ static void __lru_cache_add(struct page
 }
 
 /**
- * lru_cache_add: add a page to the page lists
+ * lru_cache_add_anon - add a page to the page lists
  * @page: the page to add
  */
 void lru_cache_add_anon(struct page *page)
@@ -930,10 +930,10 @@ EXPORT_SYMBOL(__pagevec_lru_add);
  */
 unsigned pagevec_lookup_entries(struct pagevec *pvec,
 				struct address_space *mapping,
-				pgoff_t start, unsigned nr_pages,
+				pgoff_t start, unsigned nr_entries,
 				pgoff_t *indices)
 {
-	pvec->nr = find_get_entries(mapping, start, nr_pages,
+	pvec->nr = find_get_entries(mapping, start, nr_entries,
 				    pvec->pages, indices);
 	return pagevec_count(pvec);
 }
@@ -965,9 +965,8 @@ void pagevec_remove_exceptionals(struct
  * @mapping:	The address_space to search
  * @start:	The starting page index
  * @end:	The final page index
- * @nr_pages:	The maximum number of pages
  *
- * pagevec_lookup_range() will search for and return a group of up to @nr_pages
+ * pagevec_lookup_range() will search for & return a group of up to PAGEVEC_SIZE
  * pages in the mapping starting from index @start and upto index @end
  * (inclusive).  The pages are placed in @pvec.  pagevec_lookup() takes a
  * reference against the pages in @pvec.
@@ -977,7 +976,7 @@ void pagevec_remove_exceptionals(struct
  * also update @start to index the next page for the traversal.
  *
  * pagevec_lookup_range() returns the number of pages which were found. If this
- * number is smaller than @nr_pages, the end of specified range has been
+ * number is smaller than PAGEVEC_SIZE, the end of specified range has been
  * reached.
  */
 unsigned pagevec_lookup_range(struct pagevec *pvec,


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm/swap.c: make functions and their kernel-doc agree

[Michal Hocko]

On Mon 29-01-18 16:43:55, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> 
> Fix some basic kernel-doc notation in mm/swap.c:
> - for function lru_cache_add_anon(), make its kernel-doc function name
>   match its function name and change colon to hyphen following the
>   function name

This is pretty much an internal function to the MM. It shouldn't have
any external callers. Why do we need a kernel doc at all?

> - for function pagevec_lookup_entries(), change the function parameter
>   name from nr_pages to nr_entries since that is more descriptive of
>   what the parameter actually is and then it matches the kernel-doc
>   comments also

I know what is nr_pages because I do expect pages to be returned. What
are entries? Can it be something different from pages?
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm/swap.c: make functions and their kernel-doc agree

[Randy Dunlap]

On 01/30/2018 04:34 AM, Michal Hocko wrote:
> On Mon 29-01-18 16:43:55, Randy Dunlap wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> Fix some basic kernel-doc notation in mm/swap.c:
>> - for function lru_cache_add_anon(), make its kernel-doc function name
>>   match its function name and change colon to hyphen following the
>>   function name
> 
> This is pretty much an internal function to the MM. It shouldn't have
> any external callers. Why do we need a kernel doc at all?
> 
>> - for function pagevec_lookup_entries(), change the function parameter
>>   name from nr_pages to nr_entries since that is more descriptive of
>>   what the parameter actually is and then it matches the kernel-doc
>>   comments also
> 
> I know what is nr_pages because I do expect pages to be returned. What
> are entries? Can it be something different from pages?

OK, never mind.  I'll revisit this some other day.

later,
-- 
~Randy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm/swap.c: make functions and their kernel-doc agree

[Matthew Wilcox]

On Tue, Jan 30, 2018 at 01:34:00PM +0100, Michal Hocko wrote:
> On Mon 29-01-18 16:43:55, Randy Dunlap wrote:
> > - for function pagevec_lookup_entries(), change the function parameter
> >   name from nr_pages to nr_entries since that is more descriptive of
> >   what the parameter actually is and then it matches the kernel-doc
> >   comments also
> 
> I know what is nr_pages because I do expect pages to be returned. What
> are entries? Can it be something different from pages?

entries are any page cache entries -- pages or exceptional entries.
calling this parameter nr_pages tricks you into thinking that you'll
only get pages back.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm/swap.c: make functions and their kernel-doc agree

[Michal Hocko]

On Tue 30-01-18 23:58:48, Matthew Wilcox wrote:
> On Tue, Jan 30, 2018 at 01:34:00PM +0100, Michal Hocko wrote:
> > On Mon 29-01-18 16:43:55, Randy Dunlap wrote:
> > > - for function pagevec_lookup_entries(), change the function parameter
> > >   name from nr_pages to nr_entries since that is more descriptive of
> > >   what the parameter actually is and then it matches the kernel-doc
> > >   comments also
> > 
> > I know what is nr_pages because I do expect pages to be returned. What
> > are entries? Can it be something different from pages?
> 
> entries are any page cache entries -- pages or exceptional entries.

Fair point.

> calling this parameter nr_pages tricks you into thinking that you'll
> only get pages back.

Well, the data structure we are using is a pagevec and that operates on
top of struct pages. It is true that this is quite confusing especially
for those who are not familiar with exceptional entries. Hopefully the
associated documentation helps.

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>