Re: [PATCH v3 1/4] mm: change to return bool for folio_isolate_lru()

From: Matthew Wilcox
Date: Wed Feb 15 2023 - 10:41:39 EST


On Wed, Feb 15, 2023 at 06:39:34PM +0800, Baolin Wang wrote:
> Now the folio_isolate_lru() did not return a boolean value to indicate
> isolation success or not, however below code checking the return value
> can make people think that it was a boolean success/failure thing, which
> makes people easy to make mistakes (see the fix patch[1]).
>
> if (folio_isolate_lru(folio))
> continue;
>
> Thus it's better to check the negative error value expilictly returned by
> folio_isolate_lru(), which makes code more clear per Linus's suggestion[2].
> Moreover Matthew suggested we can convert the isolation functions to return
> a boolean[3], since most users did not care about the negative error value,
> and can also remove the confusing of checking return value.
>
> So this patch converts the folio_isolate_lru() to return a boolean value,
> which means return 'true' to indicate the folio isolation is successful,
> and 'false' means a failure to isolation. Meanwhile changing all users'
> logic of checking the isolation state.
>
> No functional changes intended.
>
> [1] https://lore.kernel.org/all/20230131063206.28820-1-Kuan-Ying.Lee@xxxxxxxxxxxx/T/#u
> [2] https://lore.kernel.org/all/CAHk-=wiBrY+O-4=2mrbVyxR+hOqfdJ=Do6xoucfJ9_5az01L4Q@xxxxxxxxxxxxxx/
> [3] https://lore.kernel.org/all/Y+sTFqwMNAjDvxw3@xxxxxxxxxxxxxxxxxxxx/
>
> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> Reviewed-by: SeongJae Park <sj@xxxxxxxxxx>
> Acked-by: David Hildenbrand <david@xxxxxxxxxx>

Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>