[PATCH 0/3] Some cleanups for page isolation

Baolin Wang posted 3 patches 2 years, 6 months ago
mm/damon/paddr.c    | 2 +-
mm/gup.c            | 2 +-
mm/khugepaged.c     | 4 ++--
mm/memcontrol.c     | 2 +-
mm/mempolicy.c      | 2 +-
mm/migrate.c        | 4 ++--
mm/migrate_device.c | 2 +-
7 files changed, 9 insertions(+), 9 deletions(-)
[PATCH 0/3] Some cleanups for page isolation
Posted by Baolin Wang 2 years, 6 months ago
Hi,

The page isolation functions did not return a boolean to indicate
success or not, instead it will return a negative error when failed
to isolate a page. So it's better to check the negative error explicitly
for isolation to make the code more clear per Linus's suggestion in [1].

No functional changes intended in this patch series.

[1] https://lore.kernel.org/all/CAHk-=wiBrY+O-4=2mrbVyxR+hOqfdJ=Do6xoucfJ9_5az01L4Q@mail.gmail.com/

Baolin Wang (3):
  mm: check negative error of folio_isolate_lru() when failed to isolate
    a folio
  mm: check negative error of isolate_lru_page() when failed to isolate
    a page
  mm: mempolicy: check negative error of isolate_hugetlb() when failed
    to isolate a hugetlb

 mm/damon/paddr.c    | 2 +-
 mm/gup.c            | 2 +-
 mm/khugepaged.c     | 4 ++--
 mm/memcontrol.c     | 2 +-
 mm/mempolicy.c      | 2 +-
 mm/migrate.c        | 4 ++--
 mm/migrate_device.c | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.27.0
Re: [PATCH 0/3] Some cleanups for page isolation
Posted by Matthew Wilcox 2 years, 6 months ago
On Tue, Feb 14, 2023 at 11:18:05AM +0800, Baolin Wang wrote:
> The page isolation functions did not return a boolean to indicate
> success or not, instead it will return a negative error when failed
> to isolate a page. So it's better to check the negative error explicitly
> for isolation to make the code more clear per Linus's suggestion in [1].

Only one caller of isolate_lru_page() or folio_isolate_lru() actually
uses the errno.  And the errno can only be 0 or -EBUSY.  It'd be
better to change the three functions to return bool and fix
add_page_for_migration() to set the errno to -EBUSY itself.
Re: [PATCH 0/3] Some cleanups for page isolation
Posted by Baolin Wang 2 years, 6 months ago

On 2/14/2023 12:50 PM, Matthew Wilcox wrote:
> On Tue, Feb 14, 2023 at 11:18:05AM +0800, Baolin Wang wrote:
>> The page isolation functions did not return a boolean to indicate
>> success or not, instead it will return a negative error when failed
>> to isolate a page. So it's better to check the negative error explicitly
>> for isolation to make the code more clear per Linus's suggestion in [1].
> 
> Only one caller of isolate_lru_page() or folio_isolate_lru() actually
> uses the errno.  And the errno can only be 0 or -EBUSY.  It'd be
> better to change the three functions to return bool and fix
> add_page_for_migration() to set the errno to -EBUSY itself.

Sounds reasonable to me, and I can change them to return bool in next 
version. Thanks.