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

[Yunsheng Lin]

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
>