Received-SPF: pass (google.com: domain of linux-kernel+bounces-176103-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <463d6564-ea67-4f1f-92f2-196a1334cc3c@infradead.org>
Date: Fri, 10 May 2024 11:30:49 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v3 12/13] mm: page_frag: update documentation for
 page_frag
To: Yunsheng Lin <linyunsheng@huawei.com>, davem@davemloft.net,
 kuba@kernel.org, pabeni@redhat.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
 Alexander Duyck <alexander.duyck@gmail.com>, Jonathan Corbet
 <corbet@lwn.net>, Andrew Morton <akpm@linux-foundation.org>,
 linux-mm@kvack.org, linux-doc@vger.kernel.org
References: <20240508133408.54708-1-linyunsheng@huawei.com>
 <20240508133408.54708-13-linyunsheng@huawei.com>
 <0ac5219b-b756-4a8d-ba31-21601eb1e7f4@infradead.org>
 <ff1089c8-ad02-04bb-f715-ca97c118338b@huawei.com>
Content-Language: en-US
From: Randy Dunlap <rdunlap@infradead.org>
In-Reply-To: <ff1089c8-ad02-04bb-f715-ca97c118338b@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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