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 11:30:49 -0700
From: Randy Dunlap <rdunlap@...radead.org>
To: Yunsheng Lin <linyunsheng@...wei.com>, 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

Hi.

On 5/10/24 5:32 AM, Yunsheng Lin wrote:
> 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.

This one ^^^^^^^^^^^^^^^^^^^^.

> 
>>
>>> + * 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?

I was just trying to say that you could use
     'va'
or
     @va
here.

> 
>>
> 
> ...
> 
>>
>>                                                  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?

That's what I would do, but there is also the issue of the first occurrence of
quoted fragsz that uses Unicode quote marks, but the second occurrence of
quoted fragsz uses plain ASCII quote marks. This happens in multiple places
(probably due to copy-paste).

> 
> ...

Thanks.

-- 
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html