Re: [PATCH next] mm/vmscan: __isolate_lru_page_prepare clean up

[Alex Shi]

在 2020/11/21 上午7:13, Andrew Morton 写道:
> On Fri, 20 Nov 2020 16:03:33 +0800 Alex Shi <alex.shi@xxxxxxxxxxxxxxxxx> wrote:
> 
>> The function just return 2 results, so use a 'switch' to deal with its
>> result is unnecessary, and simplify it to a bool func as Vlastimil
>> suggested.
>>
>> Also removed 'goto' in using by reusing list_move().
>>
>> ...
>>
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -1540,7 +1540,7 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone,
>>   */
>>  int __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode)
>>  {
>> -	int ret = -EBUSY;
>> +	int ret = false;
>>  
>>  	/* Only take pages on the LRU. */
>>  	if (!PageLRU(page))
>> @@ -1590,7 +1590,7 @@ int __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode)
>>  	if ((mode & ISOLATE_UNMAPPED) && page_mapped(page))
>>  		return ret;
>>  
>> -	return 0;
>> +	return true;
>>  }
> 
> The resulting __isolate_lru_page_prepare() is rather unpleasing.
> 
> - Why return an int and not a bool?
> 
> - `int ret = false' is a big hint that `ret' should have bool type!
> 
> - Why not just remove `ret' and do `return false' in all those `return
>   ret' places?
> 
> - The __isolate_lru_page_prepare() kerneldoc still says "returns 0 on
>   success, -ve errno on failure".  
> 

Hi Andrew,

Thanks a lot for caching and sorry for the bad patch.
It initially a 'int' version, and change it to bool in a hurry weekend.
I am sorry.