[PATCH v2] mm: zsmalloc: Replace return type int with bool

[PATCH v2] mm: zsmalloc: Replace return type int with bool

[Souptick Joarder]

zs_register_migration() returns either 0 or 1.
So the return type int should be replaced with bool.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
---

v2: Returning false in zs_register_migration() as return
    type is bool

 mm/zsmalloc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index c301350..5fa2dfe 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -295,7 +295,7 @@ struct mapping_area {
 };

 #ifdef CONFIG_COMPACTION
-static int zs_register_migration(struct zs_pool *pool);
+static bool zs_register_migration(struct zs_pool *pool);
 static void zs_unregister_migration(struct zs_pool *pool);
 static void migrate_lock_init(struct zspage *zspage);
 static void migrate_read_lock(struct zspage *zspage);
@@ -306,7 +306,7 @@ struct mapping_area {
 #else
 static int zsmalloc_mount(void) { return 0; }
 static void zsmalloc_unmount(void) {}
-static int zs_register_migration(struct zs_pool *pool) { return 0; }
+static bool zs_register_migration(struct zs_pool *pool) { return false; }
 static void zs_unregister_migration(struct zs_pool *pool) {}
 static void migrate_lock_init(struct zspage *zspage) {}
 static void migrate_read_lock(struct zspage *zspage) {}
@@ -2101,17 +2101,17 @@ void zs_page_putback(struct page *page)
 	.putback_page = zs_page_putback,
 };

-static int zs_register_migration(struct zs_pool *pool)
+static bool zs_register_migration(struct zs_pool *pool)
 {
 	pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
 	if (IS_ERR(pool->inode)) {
 		pool->inode = NULL;
-		return 1;
+		return true;
 	}

 	pool->inode->i_mapping->private_data = pool;
 	pool->inode->i_mapping->a_ops = &zsmalloc_aops;
-	return 0;
+	return false;
 }

 static void zs_unregister_migration(struct zs_pool *pool)
--
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm: zsmalloc: Replace return type int with bool

[Michal Hocko]

On Tue 20-02-18 23:28:11, Souptick Joarder wrote:
[...]
> -static int zs_register_migration(struct zs_pool *pool)
> +static bool zs_register_migration(struct zs_pool *pool)
>  {
>  	pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
>  	if (IS_ERR(pool->inode)) {
>  		pool->inode = NULL;
> -		return 1;
> +		return true;
>  	}
> 
>  	pool->inode->i_mapping->private_data = pool;
>  	pool->inode->i_mapping->a_ops = &zsmalloc_aops;
> -	return 0;
> +	return false;
>  }

Don't you find it a bit strange that the function returns false on
success?
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm: zsmalloc: Replace return type int with bool

[Souptick Joarder]

On Wed, Feb 21, 2018 at 3:52 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Tue 20-02-18 23:28:11, Souptick Joarder wrote:
> [...]
>> -static int zs_register_migration(struct zs_pool *pool)
>> +static bool zs_register_migration(struct zs_pool *pool)
>>  {
>>       pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
>>       if (IS_ERR(pool->inode)) {
>>               pool->inode = NULL;
>> -             return 1;
>> +             return true;
>>       }
>>
>>       pool->inode->i_mapping->private_data = pool;
>>       pool->inode->i_mapping->a_ops = &zsmalloc_aops;
>> -     return 0;
>> +     return false;
>>  }
>
> Don't you find it a bit strange that the function returns false on
> success?

The original code was returning 0 on success  and return value was handled
accordingly in zs_create_pool(). So returning false on success.

Shall I change it ?
> --
> Michal Hocko
> SUSE Labs

-Souptick

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm: zsmalloc: Replace return type int with bool

[Michal Hocko]

On Wed 21-02-18 16:48:50, Souptick Joarder wrote:
> On Wed, Feb 21, 2018 at 3:52 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Tue 20-02-18 23:28:11, Souptick Joarder wrote:
> > [...]
> >> -static int zs_register_migration(struct zs_pool *pool)
> >> +static bool zs_register_migration(struct zs_pool *pool)
> >>  {
> >>       pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
> >>       if (IS_ERR(pool->inode)) {
> >>               pool->inode = NULL;
> >> -             return 1;
> >> +             return true;
> >>       }
> >>
> >>       pool->inode->i_mapping->private_data = pool;
> >>       pool->inode->i_mapping->a_ops = &zsmalloc_aops;
> >> -     return 0;
> >> +     return false;
> >>  }
> >
> > Don't you find it a bit strange that the function returns false on
> > success?
> 
> The original code was returning 0 on success  and return value was handled
> accordingly in zs_create_pool(). So returning false on success.

Returning 0 on success and an error code on failure is a standard
convention. Returning false is just weird. zs_register_migration is
somewhere in the middle because it doesn't really return an error code
on failure. Whether this is worth bothering and fixing is a question for
the maintainer but returning false on success is just not an improvement
IMHO.

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>