Re: [PATCH net-next v3 12/13] mm: page_frag: update documentation for page_frag

[Yunsheng Lin <linyunsheng@huawei.com>]

On 2024/5/9 8:44, Randy Dunlap wrote:
> 
> 

>>  
>> +/**
>> + * page_frag_cache_is_pfmemalloc() - Check for pfmemalloc.
>> + * @nc: page_frag cache from which to check
>> + *
>> + * Used to check if the current page in page_frag cache is pfmemalloc'ed.
>> + * It has the same calling context expection as the alloc API.
>> + *
>> + * Return:
>> + * Return true if the current page in page_frag cache is pfmemalloc'ed,
> 
> Drop the (second) word "Return"...

Did you mean something like below:

* Return:
* Return true if the current page in page_frag cache is pfmemalloc'ed,
* otherwise false.

Or:

* Return:
* true if the current page in page_frag cache is pfmemalloc'ed, otherwise
* return false.


> 
>> + * otherwise return false.
>> + */
>>  static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
>>  {
>>  	return encoded_page_pfmemalloc(nc->encoded_va);
>> @@ -92,6 +109,19 @@ void *__page_frag_alloc_va_align(struct page_frag_cache *nc,
>>  				 unsigned int fragsz, gfp_t gfp_mask,
>>  				 unsigned int align_mask);
>>  
>> +/**
>> + * page_frag_alloc_va_align() - Alloc a page fragment with aligning requirement.
>> + * @nc: page_frag cache from which to allocate
>> + * @fragsz: the requested fragment size
>> + * @gfp_mask: the allocation gfp to use when cache need to be refilled
> 
>                                                       needs
> 
>> + * @align: the requested aligning requirement for 'va'
> 
>                  or                                  @va

What does the 'or' means?

> 

...

> 
>                                                  needs
> 
>> + *
>> + * Prepare a page fragment with minimum size of ‘fragsz’, 'fragsz' is also used
> 
>                                                    'fragsz'. 'fragsz'
> (don't use fancy single quote marks)

You mean using @parameter to replace all the parameters marked with single
quote marks, right?

...

>>  
>> +/**
>> + * page_frag_alloc_prepare - Prepare allocing a page fragment.
>> + * @nc: page_frag cache from which to prepare
>> + * @offset: out as the offset of the page fragment
>> + * @fragsz: in as the requested size, out as the available size
>> + * @va: out as the virtual address of the returned page fragment
>> + * @gfp: the allocation gfp to use when cache need to be refilled
>> + *
>> + * Prepare a page fragment with minimum size of ‘fragsz’, 'fragsz' is also used
> 
>                                                    'fragsz'. 'fragsz'
> (don't use fancy single quote marks)
> 
> You could also (in several places) refer to the variables as
>                                                     @fragsz. @fragsz
> 
>> + * to report the maximum size of the page fragment. Return both 'page' and 'va'
>> + * of the fragment to the caller.
>> + *
>> + * Return:
>> + * Return the page fragment, otherwise return NULL.
> 
> Drop second "Return". But the paragraph above says that both @page and @va
> are returned. How is that done?

struct page *page_frag_alloc_prepare(struct page_frag_cache *nc,
				     unsigned int *offset,
				     unsigned int *fragsz,
				     void **va, gfp_t gfp);

As above, @page is returned through the function return, @va is returned
through double pointer.

Thanks for the detailed review.