netdev - 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]

Message-ID: <9b76e7c0-7b24-00e5-adba-214ce306ae96@huawei.com>
Date: Fri, 10 May 2024 17:48:55 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Mat Martineau <martineau@...nel.org>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<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/10 0:58, Mat Martineau wrote:

...

>>
>> +/**
>> + * page_frag_alloc_probe - Probe the avaiable page fragment.
>> + * @nc: page_frag cache from which to probe
>> + * @offset: out as the offset of the page fragment
>> + * @fragsz: in as the requested size, out as the available size
> 
> Hi Yunsheng -
> 
> fragsz is never used as an input in this function. I think it would be good to make the code consistent with this documentation by checking that *fragsz <= (nc)->remaining

Yes, you are right.
It is not used as input, will update the documentation according.

> 
>> + * @va: out as the virtual address of the returned page fragment
>> + *
>> + * Probe the current available memory to caller without doing cache refilling.
>> + * If the cache is empty, return NULL.
> 
> Instead of this line, is it more accurate to say "if no space is available in the page_frag cache, return NULL" ?
> 
> I also suggest adding some documentation here like:
> 
> "If the requested space is available, up to fragsz bytes may be added to the fragment using page_frag_alloc_commit()".

Ok.

> 
>> + *
>> + * Return:
>> + * Return the page fragment, otherwise return NULL.
>> + */
>> #define page_frag_alloc_probe(nc, offset, fragsz, va)            \
>> ({                                    \
>>     struct encoded_va *__encoded_va;                \
>> @@ -162,6 +241,13 @@ static inline struct encoded_va *__page_frag_alloc_probe(struct page_frag_cache
>>     __page;                                \
>> })
>>
>> +/**
>> + * page_frag_alloc_commit - Commit allocing a page fragment.
>> + * @nc: page_frag cache from which to commit
>> + * @fragsz: size of the page fragment has been used
>> + *
>> + * Commit the alloc preparing by passing the actual used size.
> 
> Rephrasing suggestion:
> 
> "Commit the actual used size for the allocation that was either prepared or probed"

Ok.

Thanks.

> 
> 
> Thanks,
> 
> Mat
>