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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 10 May 2024 20:32:13 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Randy Dunlap <rdunlap@...radead.org>, <davem@...emloft.net>,
	<kuba@...nel.org>, <pabeni@...hat.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Alexander Duyck
	<alexander.duyck@...il.com>, Jonathan Corbet <corbet@....net>, Andrew Morton
	<akpm@...ux-foundation.org>, <linux-mm@...ck.org>,
	<linux-doc@...r.kernel.org>
Subject: Re: [PATCH net-next v3 12/13] mm: page_frag: update documentation for
 page_frag

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.